[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

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

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

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

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

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

56

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

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;
+  }
+
   /// 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/lib/ExtractAPI/ExtractAPIVisitor.cpp

 }
 
 template <class DeclTy>
 bool isInSystemHeader(const ASTContext &Context, const DeclTy *D) {
   return Context.getSourceManager().isInSystemHeader(D->getLocation());
 }
 
 } // namespace
 
 bool ExtractAPIVisitor::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())
     return true;
 
   // Skip local variables inside function or method.
   if (!Decl->isDefinedOutsideFunctionOrMethod())
     return true;
if (isa<ObjCTypeParamDecl>(Decl))
     return true;
 
   if (!Decl->isDefinedOutsideFunctionOrMethod())
     return true;
 
   if (!LocationChecker(Decl->getLocation()))
     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()) {
+          for (const auto &Struct : API.getStructs()) {
+            if (Decl->getName() == Struct.second.get()->Name) {
+              Struct.second.get()
+                  ->Declaration
+                  .appendFront(" ", DeclarationFragments::FragmentKind::Text)
+                  .appendFront("typedef",
+                               DeclarationFragments::FragmentKind::Keyword, "",
+                               nullptr);
+              break;
+            }
+          }
+        }
+        if (TagTy->getDecl()->isEnum()) {
+          for (const auto &Enum : API.getEnums()) {
+            if (Decl->getName() == Enum.second.get()->Name) {
+              Enum.second.get()
+                  ->Declaration
+                  .appendFront(" ", DeclarationFragments::FragmentKind::Text)
+                  .appendFront("typedef",
+                               DeclarationFragments::FragmentKind::Keyword, "",
+                               nullptr);
+              break;
+            }
+          }
+        }
+      }
+    }
+  }
+
   PresumedLoc Loc =
       Context.getSourceManager().getPresumedLoc(Decl->getLocation());
   StringRef Name = Decl->getName();
   StringRef USR = API.recordUSR(Decl);
   DocComment Comment;
   if (auto *RawComment = Context.getRawCommentForDeclNoCache(Decl))
     Comment = RawComment->getFormattedLines(Context.getSourceManager(),
                                             Context.getDiagnostics());

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;
+
+//--- reference.output.json.in
+{
+  "metadata": {
+    "formatVersion": {
+      "major": 0,
+      "minor": 5,
+      "patch": 3
+    },
+    "generator": "?"
+  },
+  "module": {
+    "name": "",
+    "platform": {
+      "architecture": "arm64",
+      "operatingSystem": {
+        "name": "macosx"
+      },
+      "vendor": "apple"
+    }
+  },
+  "relationships": [
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Test2@simple",
+      "target": "c:@E@Test2",
+      "targetFallback": "Test2"
+    }
+  ],
+  "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": ";"
+        }
+      ],
+      "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": ";"
+        }
+      ],
+      "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"
+      ]
+    }
+  ]
+}