[clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef

[Ruturaj4]

Summary

enums and structs declared inside typedefs have incorrect declaration fragments, where the typedef keyword and other syntax is missing.

For the following struct:

typedef struct Test {

int hello;

} Test;
The produced declaration is:

"declarationFragments": [

{
  "kind": "keyword",
  "spelling": "struct"
},
{
  "kind": "text",
  "spelling": " "
},
{
  "kind": "identifier",
  "spelling": "Test"
}

],
instead the declaration fragments should represent the following

typedef struct Test {

…

} Test;

This patch removes the condition in SymbolGraphSerializer.cpp file and completes declaration fragments

[dang]

I think there might be some code missing here. Also can you add a test?

[Ruturaj4]

1. Updating D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Updated previous diff with requested changes and added new test.

[dang]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

342 ↗

(On Diff #507958)

This doesn't quite produce the output we want, this would generate typedef struct Foo; instead of the expected typedef struct Foo { ... } Bar; where Foo is the name of the struct and Bar is the name of the typedef. This should be easy enough to fix.

347 ↗

(On Diff #507958)

What happens if the visitation hasn't already encountered the definition of the struct? We wouldn't find it in the the APISet and this doesn't work. The approach I would suggest here is to generate the fragments for the underlying Decl directly. For example what happens for the following code:

c
struct Foo;
typedef struct Foo TypedefedFoo;
struct Foo {
    int bar;
};

349–354 ↗

(On Diff #507958)

This logic is very similar to the one below we could use the same code.

[Ruturaj4]

1. Updating D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Made all updates as you requested, though I didn't need to generate fragments for the underlying Decl. I added an example as you have recommended.

[Ruturaj4]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

347 ↗

(On Diff #507958)

Actually added an example for this. Our code works fine. Please take a look.

[Ruturaj4]

1. Updating D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Update main to avoid build failures.

[dang]

You will need to rebase this as I made some changes recently to how ExtractAPIVisitor is structured. We can either set up a time to talk about it and do it together or I can handle doing this work once we are happy with this.

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

52 ↗

(On Diff #509004)

this should be a marked static or put in an anonymous namespace.

56 ↗

(On Diff #509004)

I think this warrants adding methods to declaration fragments to streamline this sort of operation (merging fragments at arbitrary offsets. This should probably be done as a follow up patch.

[Ruturaj4]

1. Updating D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

declare modifyRecord function "static"

[Ruturaj4]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

56 ↗

(On Diff #509004)

Ok I will add this in a subsequent patch.

[Ruturaj4]

1. Updating D146385: [clang][ExtractAPI] Complete declaration fragments for TagDecl types defined in a typedef #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

rebase and update patch.

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

default:
     break;
   }
 }
 
 void SymbolGraphSerializer::serializeTypedefRecord(
     const TypedefRecord &Record) {
   // Typedefs of anonymous types have their entries unified with the underlying
   // type.
-  bool ShouldDrop = Record.UnderlyingType.Name.empty();
-  // enums declared with `NS_OPTION` have a named enum and a named typedef, with
-  // the same name
-  ShouldDrop |= (Record.UnderlyingType.Name == Record.Name);
-  if (ShouldDrop)
+  if (Record.UnderlyingType.Name.empty())
     return;
 
   auto Typedef = serializeAPIRecord(Record);
   if (!Typedef)
     return;
 
   (*Typedef)["type"] = Record.UnderlyingType.USR;