[AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

[sammccall]

Summary

Semantically they're the same thing, and it's important when the underlying
struct is anonymous.

There doesn't seem to be a problem attaching the same comment to multiple things
as it already happens with /** doc */ int a, b;

This affects an Index test but the results look better (name present, USR points
to the typedef).

Fixes https://github.com/clangd/clangd/issues/189

[merge_guards_bot]

Build result: pass - 59972 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt, CMakeCache.txt

[kadircet]

clang/test/Index/annotate-comments-typedef.m

20

sorry for being dense, what exactly is the change here ? it looks like comment was already being attributed to MyEnum.
As for the USR I agree that this looks a whole lot better, apparently for anon decls USR contains the name of the typedef decl instead.

What is the extra text that we don't care in between?

clang/test/Sema/warn-documentation.cpp

871

what is the + for ?

[sammccall]

clang/test/Index/annotate-comments-typedef.m

20

Yeah, most of this doesn't matter AFAIK and the test is just brittle, as lit tests are wont to be.

The {{.*}} is because the output now includes the RawComment= clause, that previously wasn't included. (Which I think is to do with the comment being immediately before the location?)

Previously the Name was <anonymous>, and now it's MyEnum. And the USR has changed as you noted.

clang/test/Sema/warn-documentation.cpp

871

The warning is now emitted twice, once for the embedded struct definition and once for the typedef.
This isn't perfect but I don't think it's worth fixing. (Again, it's the existing behavior for a broken comment before int a,b,c)

[kadircet]

LGTM, thanks!

clang/test/Index/annotate-comments-typedef.m

20

oh missed the change in Name thanks for pointing it out!

[bjope]

Hi @sammccall .
Just a heads up. Looks like this might have caused: https://bugs.llvm.org/show_bug.cgi?id=44143 .

[chrish_ericsson_atx]

In D70203#1760299, @bjope wrote:

Hi @sammccall .
Just a heads up. Looks like this might have caused: https://bugs.llvm.org/show_bug.cgi?id=44143 .

Yes indeed. I locally reverted commit a433e714 and it resolved my issue. It's odd, though. The unittest Sam added is nearly identical with the reproducer I posted. Only obvious difference I see at first blush is that my reproducer uses the same identifier for the struct name and the typedef name, whereas Sam's unittest uses a unique identifier for each. Semantically, there is no difference, though.

clang-tools-extra/clangd/unittests/XRefsTests.cpp

OneTest Tests[] = {
           )cpp",
           "text[Declared in]code[global namespace]\n"
           "codeblock(cpp) [\n"
           "typedef int Foo\n"
           "]\n"
           "text[Typedef]",
       },
       {
+          R"cpp(// Typedef with embedded definition
+            typedef struct Bar {} Foo;
+            int main() {
+              ^Foo bar;
+            }
+          )cpp",
+          "text[Declared in]code[global namespace]\n"
+          "codeblock(cpp) [\n"
+          "typedef struct Bar Foo\n"
+          "]\n"
+          "text[Typedef with embedded definition]",
+      },
+      {
           R"cpp(// Namespace
             namespace ns {
             struct Foo { static void bar(); }
             } // namespace ns
             int main() { ^ns::Foo::bar(); }
           )cpp",
           "text[Declared in]code[global namespace]\n"
           "codeblock(cpp) [\n"

clang/lib/AST/ASTContext.cpp

static SourceLocation getDeclLocForCommentSearch(const Decl *D,
   // For Objective-C declarations we generally don't expect to have multiple
   // declarators, thus use declaration starting location as the "declaration
   // location".
   // For all other declarations multiple declarators are used quite frequently,
   // so we use the location of the identifier as the "declaration location".
   if (isa<ObjCMethodDecl>(D) || isa<ObjCContainerDecl>(D) ||
       isa<ObjCPropertyDecl>(D) ||
       isa<RedeclarableTemplateDecl>(D) ||
-      isa<ClassTemplateSpecializationDecl>(D))
+      isa<ClassTemplateSpecializationDecl>(D) ||
+      // Allow association with Y across {} in `typedef struct X {} Y`.
+      isa<TypedefDecl>(D))
     return D->getBeginLoc();
   else {
     const SourceLocation DeclLoc = D->getLocation();
     if (DeclLoc.isMacroID()) {
       if (isa<TypedefDecl>(D)) {
         // If location of the typedef name is in a macro, it is because being
         // declared via a macro. Try using declaration's starting location as
         // the "declaration location".

clang/test/Index/annotate-comments-typedef.m

 typedef unsigned int NSUInteger;
 
 /** Documentation for MyEnum */
 typedef enum : NSUInteger {
         MyEnumFoo, /**< value Foo */
         MyEnumBar, /**< value Bar */
         MyEnumBaz, /**< value Baz */
 } MyEnum;
-// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) FullCommentAsHTML=[<p class="para-brief"> Documentation for MyEnum </p>] FullCommentAsXML=[<Typedef file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-1]]" column="3"><Name>&lt;anonymous&gt;</Name><USR>c:@EA@MyEnum</USR><Declaration>typedef enum MyEnum MyEnum</Declaration><Abstract><Para> Documentation for MyEnum </Para></Abstract></Typedef>]
+// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) {{.*}} FullCommentAsHTML=[<p class="para-brief"> Documentation for MyEnum </p>] FullCommentAsXML=[<Typedef file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-1]]" column="3"><Name>MyEnum</Name><USR>c:@T@MyEnum</USR><Declaration>typedef enum MyEnum MyEnum</Declaration><Abstract><Para> Documentation for MyEnum </Para></Abstract></Typedef>]
 
 
 /** Documentation for E */
 enum E {
         E_MyEnumFoo, /**< value Foo */
         E_MyEnumBar, /**< value Bar */
         E_MyEnumBaz, /**< value Baz */
 };
 typedef enum E E_T;
 // CHECK: EnumDecl=E:[[@LINE-6]]:6 (Definition) {{.*}} BriefComment=[Documentation for E] FullCommentAsHTML=[<p class="para-brief"> Documentation for E </p>] FullCommentAsXML=[<Enum file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-6]]" column="6"><Name>E</Name><USR>c:@E@E</USR><Declaration>enum E{{( : int)?}} {}</Declaration><Abstract><Para> Documentation for E </Para></Abstract></Enum>]
 // CHECK: TypedefDecl=E_T:[[@LINE-2]]:16 (Definition) FullCommentAsHTML=[<p class="para-brief"> Documentation for E </p>] FullCommentAsXML=[<Typedef file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-2]]" column="16"><Name>E</Name><USR>c:@E@E</USR><Declaration>typedef enum E E_T</Declaration><Abstract><Para> Documentation for E </Para></Abstract></Typedef>]
 
 
 /** Comment about Foo */
 typedef struct {
          int iii;
         } Foo;
-// CHECK: TypedefDecl=Foo:[[@LINE-1]]:11 (Definition) FullCommentAsHTML=[<p class="para-brief"> Comment about Foo </p>] FullCommentAsXML=[<Typedef file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-1]]" column="11"><Name>&lt;anonymous&gt;</Name><USR>c:@SA@Foo</USR><Declaration>typedef struct Foo Foo</Declaration><Abstract><Para> Comment about Foo </Para></Abstract></Typedef>]
+// CHECK: TypedefDecl=Foo:[[@LINE-1]]:11 (Definition) {{.*}} FullCommentAsHTML=[<p class="para-brief"> Comment about Foo </p>] FullCommentAsXML=[<Typedef file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-1]]" column="11"><Name>Foo</Name><USR>c:@T@Foo</USR><Declaration>typedef struct Foo Foo</Declaration><Abstract><Para> Comment about Foo </Para></Abstract></Typedef>]
 // CHECK: StructDecl=:[[@LINE-4]]:9 (Definition) {{.*}} BriefComment=[Comment about Foo] FullCommentAsHTML=[<p class="para-brief"> Comment about Foo </p>] FullCommentAsXML=[<Class file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-4]]" column="9"><Name>&lt;anonymous&gt;</Name><USR>c:@SA@Foo</USR><Declaration>struct {}</Declaration><Abstract><Para> Comment about Foo </Para></Abstract></Class>]
 
 
 struct Foo1 {
   int iii;
 };
 /** About Foo1T */
 typedef struct Foo1 Foo1T;
 // FIXME: we don't attach this comment to 'struct Foo1'
 // CHECK: TypedefDecl=Foo1T:[[@LINE-2]]:21 (Definition) {{.*}} FullCommentAsHTML=[<p class="para-brief"> About Foo1T </p>] FullCommentAsXML=[<Typedef file="{{[^"]+}}annotate-comments-typedef.m" line="[[@LINE-2]]" column="21"><Name>Foo1T</Name><USR>c:annotate-comments-typedef.m@T@Foo1T</USR><Declaration>typedef struct Foo1 Foo1T</Declaration><Abstract><Para> About Foo1T </Para></Abstract></Typedef>]
 

clang/test/Sema/warn-documentation.cpp

 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 struct test_noattach12 *test_attach13;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_noattach14 *test_attach15;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach16 { int a; } test_attach17;
 
 struct S { int a; };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 struct S *test_attach18;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct S *test_attach19;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 struct test_attach20;
 
-// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+// expected-warning@+1 + {{empty paragraph passed to '\brief' command}}
 /// \brief\author Aaa
 typedef struct test_attach21 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
   /// \brief\author Aaa
   int test_attach22;
 } test_attach23;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}