[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/include/clang/ExtractAPI/DeclarationFragments.h

struct Fragment {
     Fragment(StringRef Spelling, FragmentKind Kind, StringRef PreciseIdentifier,
              const Decl *Declaration)
         : Spelling(Spelling), Kind(Kind), PreciseIdentifier(PreciseIdentifier),
           Declaration(Declaration) {}
   };
 
   const std::vector<Fragment> &getFragments() const { return Fragments; }
 
+  // Add a new Fragment to the beginning of the Fragments.
+  DeclarationFragments &appendFront(StringRef Spelling, FragmentKind Kind,
+                                    StringRef PreciseIdentifier = "",
+                                    const Decl *Declaration = nullptr) {
+    Fragments.emplace(Fragments.begin(), Spelling, Kind, PreciseIdentifier,
+                      Declaration);
+    return *this;
+  }
+
+  DeclarationFragments &appendFront(DeclarationFragments &&Other) {
+    Fragments.insert(Fragments.begin(),
+                     std::make_move_iterator(Other.Fragments.begin()),
+                     std::make_move_iterator(Other.Fragments.end()));
+    Other.Fragments.clear();
+    return *this;
+  }
+
+  void removeLast() { Fragments.pop_back(); }
+
   /// Append a new Fragment to the end of the Fragments.
   ///
   /// \returns a reference to the DeclarationFragments object itself after
   /// appending to chain up consecutive appends.
   DeclarationFragments &append(StringRef Spelling, FragmentKind Kind,
                                StringRef PreciseIdentifier = "",
                                const Decl *Declaration = nullptr) {
     if (Kind == FragmentKind::Text && !Fragments.empty() &&

clang/include/clang/ExtractAPI/ExtractAPIVisitor.h

 #include "llvm/ADT/FunctionExtras.h"
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/TypedefUnderlyingTypeResolver.h"
+#include "llvm/ADT/StringRef.h"
 #include <type_traits>
 
 namespace clang {
 namespace extractapi {
 namespace impl {
 
 template <typename Derived>
 class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
protected:
   }
 
 private:
   Derived &getDerivedExtractAPIVisitor() {
     return *static_cast<Derived *>(this);
   }
 };
 
+template <typename T>
+static void modifyRecords(const T &Records, const StringRef &Name) {
+  for (const auto &Record : Records) {
+    if (Name == Record.second.get()->Name) {
+      Record.second.get()->Declaration.removeLast();
+      Record.second.get()
+          ->Declaration
+          .appendFront(" ", DeclarationFragments::FragmentKind::Text)
+          .appendFront("typedef", DeclarationFragments::FragmentKind::Keyword,
+                       "", nullptr)
+          .append(" { ... } ", DeclarationFragments::FragmentKind::Text)
+          .append(Name, DeclarationFragments::FragmentKind::Identifier)
+          .append(";", DeclarationFragments::FragmentKind::Text);
+      break;
+    }
+  }
+}
+
 template <typename Derived>
 bool ExtractAPIVisitorBase<Derived>::VisitVarDecl(const VarDecl *Decl) {
   // skip function parameters.
   if (isa<ParmVarDecl>(Decl))
     return true;
 
   // Skip non-global variables in records (struct/union/class).
   if (Decl->getDeclContext()->isRecord())
if (isa<ObjCTypeParamDecl>(Decl))
     return true;
 
   if (!Decl->isDefinedOutsideFunctionOrMethod())
     return true;
 
   if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl))
     return true;
 
+  // Add the notion of typedef for tag type (struct or enum) of the same name.
+  if (const ElaboratedType *ET =
+          dyn_cast<ElaboratedType>(Decl->getUnderlyingType())) {
+    if (const TagType *TagTy = dyn_cast<TagType>(ET->desugar())) {
+      if (Decl->getName() == TagTy->getDecl()->getName()) {
+        if (TagTy->getDecl()->isStruct()) {
+          modifyRecords(API.getStructs(), Decl->getName());
+        }
+        if (TagTy->getDecl()->isEnum()) {
+          modifyRecords(API.getEnums(), Decl->getName());
+        }
+      }
+    }
+  }
+
   PresumedLoc Loc =
       Context.getSourceManager().getPresumedLoc(Decl->getLocation());
   StringRef Name = Decl->getName();
   StringRef USR = API.recordUSR(Decl);
   DocComment Comment;
   if (auto *RawComment =
           getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl))
     Comment = RawComment->getFormattedLines(Context.getSourceManager(),

clang/test/ExtractAPI/typedef_struct_enum.c

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang -extract-api -target arm64-apple-macosx \
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- input.h
+typedef struct Test {
+} Test;
+
+typedef enum Test2 {
+  simple
+} Test2;
+
+struct Foo;
+typedef struct Foo TypedefedFoo;
+struct Foo {
+    int bar;
+};
+
+//--- reference.output.json.in
+{
+  "metadata": {
+    "formatVersion": {
+      "major": 0,
+      "minor": 5,
+      "patch": 3
+    },
+    "generator": "?"
+  },
+  "module": {
+    "name": "",
+    "platform": {
+      "architecture": "arm64",
+      "operatingSystem": {
+        "minimumVersion": {
+          "major": 11,
+          "minor": 0,
+          "patch": 0
+        },
+        "name": "macosx"
+      },
+      "vendor": "apple"
+    }
+  },
+  "relationships": [
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Test2@simple",
+      "target": "c:@E@Test2",
+      "targetFallback": "Test2"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@S@Foo@FI@bar",
+      "target": "c:@S@Foo",
+      "targetFallback": "Foo"
+    }
+  ],
+  "symbols": [
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "typedef"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "keyword",
+          "spelling": "enum"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "Test2"
+        },
+        {
+          "kind": "text",
+          "spelling": ": "
+        },
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:i",
+          "spelling": "unsigned int"
+        },
+        {
+          "kind": "text",
+          "spelling": " { ... } "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "Test2"
+        },
+        {
+          "kind": "text",
+          "spelling": ";"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Test2"
+      },
+      "kind": {
+        "displayName": "Enumeration",
+        "identifier": "c.enum"
+      },
+      "location": {
+        "position": {
+          "character": 14,
+          "line": 4
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "Test2"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Test2"
+          }
+        ],
+        "title": "Test2"
+      },
+      "pathComponents": [
+        "Test2"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "simple"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Test2@simple"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "position": {
+          "character": 3,
+          "line": 5
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "simple"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "simple"
+          }
+        ],
+        "title": "simple"
+      },
+      "pathComponents": [
+        "Test2",
+        "simple"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "typedef"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "keyword",
+          "spelling": "struct"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "Test"
+        },
+        {
+          "kind": "text",
+          "spelling": " { ... } "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "Test"
+        },
+        {
+          "kind": "text",
+          "spelling": ";"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@S@Test"
+      },
+      "kind": {
+        "displayName": "Structure",
+        "identifier": "c.struct"
+      },
+      "location": {
+        "position": {
+          "character": 16,
+          "line": 1
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "Test"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Test"
+          }
+        ],
+        "title": "Test"
+      },
+      "pathComponents": [
+        "Test"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "struct"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "Foo"
+        },
+        {
+          "kind": "text",
+          "spelling": ";"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@S@Foo"
+      },
+      "kind": {
+        "displayName": "Structure",
+        "identifier": "c.struct"
+      },
+      "location": {
+        "position": {
+          "character": 8,
+          "line": 10
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "Foo"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Foo"
+          }
+        ],
+        "title": "Foo"
+      },
+      "pathComponents": [
+        "Foo"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:I",
+          "spelling": "int"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "bar"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@S@Foo@FI@bar"
+      },
+      "kind": {
+        "displayName": "Instance Property",
+        "identifier": "c.property"
+      },
+      "location": {
+        "position": {
+          "character": 9,
+          "line": 11
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "bar"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "bar"
+          }
+        ],
+        "title": "bar"
+      },
+      "pathComponents": [
+        "Foo",
+        "bar"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "typedef"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "keyword",
+          "spelling": "struct"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:@S@Foo",
+          "spelling": "Foo"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "TypedefedFoo"
+        },
+        {
+          "kind": "text",
+          "spelling": ";"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:input.h@T@TypedefedFoo"
+      },
+      "kind": {
+        "displayName": "Type Alias",
+        "identifier": "c.typealias"
+      },
+      "location": {
+        "position": {
+          "character": 20,
+          "line": 9
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "TypedefedFoo"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "TypedefedFoo"
+          }
+        ],
+        "title": "TypedefedFoo"
+      },
+      "pathComponents": [
+        "TypedefedFoo"
+      ],
+      "type": "c:@S@Foo"
+    }
+  ]
+}