[clang][ExtractAPI] Fix naming of typedef'd anonymous enums

[dang]

Summary

Anonymous enums that are typedef'd should take on the name of the typedef.

[zixuw]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

174–175

Aren't these two lines supposed to do this?

[dang]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

174–175

Qualified name is never empty, just contains some version of anonymous, whereas getName() is actually empty for anonymous types. The flow is now, try to populate with getName (which is unqualified and puts us in a better spot for the eventual support of c++ and nested types). and if that doesn't work fallback to the qualified name.

[zixuw]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

174–175

Sorry I meant getTypedefName(Decl), which also tries to get the typedef name for an anonymous tag.
The patch summary says "Anonymous enums that are typedef'd should take on the name of the typedef." and I was a bit confused of the change.

Now we have three (two fallbacks) for Enum:

1. straightforward Decl->getName(), and if that's empty
2. getTypedefName(Decl), which calls Decl->getTypedefNameForAnonDecl(), and if that fails,
3. Decl->printQualifiedName(OS)

My questions:

1. Do we need all three? We should be able to get rid of at least one right? The logic seems to be: if enum is named, use it. Otherwise get typedef name if it's part of a typedef. And finally leave something like (anonymous) if it's just an unattached anonymous enum.
2. For the printQualifiedName path, isn't Name referencing a local string inside the method?
3. structs are also tags that can be anonymous and inside a typedef, do we need similar changes there?

[dang]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

174–175

1. Yup that is exactly what I am trying to do here. If you can think of a way of removing a chunk then let's do it. The reason for the third fallback is to have a name for the container of an anonymous (without a typedef) top level enum (there is an example of that in the enum.c test case)
2. Not entirely sure what you mean. If you are referring to the fact that the backing storage for Name in this case is a local variable, this is fine because we copy the string into the APISet when constructing the record within this method a bit further down.
3. The struct case is a little different, It's not possible to have to have an anonymous struct without a typedef at the top level, whereas it is possible to do so with an enum (e.g. to introduce some constants). Currently for structs we only do Decl->getName() and falls back to getTypedefName(Decl). If that fails we ignore the struct because it must be a nested anonymous struct (that logic is flawed and we should aim to support anonymous structs properly, but that would be a follow up patch to this one).

[zixuw]

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

174–175

Ah I see. So before this patch we had getQualifiedNameAsString which would always bypass the following getTypedefName even if it's an anonymous within a typedef. This patch cleared out the logic and ordering.

we copy the string into the APISet when constructing the record within this method a bit further down.

😅 right, that part was folded in the diff view and I totally forgot we had that there...

LGTM. Thanks Daniel!

clang/lib/ExtractAPI/ExtractAPIVisitor.cpp

 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/AvailabilityInfo.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace extractapi;
 
 namespace {
 
 StringRef getTypedefName(const TagDecl *Decl) {
   if (const auto *TypedefDecl = Decl->getTypedefNameForAnonDecl())
bool ExtractAPIVisitor::VisitEnumDecl(const EnumDecl *Decl) {
 
   // Skip forward declaration.
   if (!Decl->isThisDeclarationADefinition())
     return true;
 
   if (!LocationChecker(Decl->getLocation()))
     return true;
 
+  SmallString<128> QualifiedNameBuffer;
   // Collect symbol information.
-  std::string NameString = Decl->getQualifiedNameAsString();
-  StringRef Name(NameString);
+  StringRef Name = Decl->getName();
   if (Name.empty())
     Name = getTypedefName(Decl);
+  if (Name.empty()) {
+    llvm::raw_svector_ostream OS(QualifiedNameBuffer);
+    Decl->printQualifiedName(OS);
+    Name = QualifiedNameBuffer.str();
+  }
 
   StringRef USR = API.recordUSR(Decl);
   PresumedLoc Loc =
       Context.getSourceManager().getPresumedLoc(Decl->getLocation());
   DocComment Comment;
   if (auto *RawComment = Context.getRawCommentForDeclNoCache(Decl))
     Comment = RawComment->getFormattedLines(Context.getSourceManager(),
                                             Context.getDiagnostics());

clang/test/ExtractAPI/typedef_anonymous_record.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 --product-name=TypedefChain -target arm64-apple-macosx \
-// RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+// RUN: %clang_cc1 -extract-api --product-name=TypedefChain -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
 
 // 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 { } MyStruct;
 typedef MyStruct MyStructStruct;
 typedef MyStructStruct MyStructStructStruct;
+typedef enum { Case } MyEnum;
+typedef MyEnum MyEnumEnum;
+typedef MyEnumEnum MyEnumEnumEnum;
+// expected-no-diagnostics
 
 //--- reference.output.json.in
 {
   "metadata": {
     "formatVersion": {
       "major": 0,
       "minor": 5,
       "patch": 3
"platform": {
           "minor": 0,
           "patch": 0
         },
         "name": "macosx"
       },
       "vendor": "apple"
     }
   },
-  "relationships": [],
+  "relationships": [
+    {
+      "kind": "memberOf",
+      "source": "c:@EA@MyEnum@Case",
+      "target": "c:@EA@MyEnum",
+      "targetFallback": "MyEnum"
+    }
+  ],
   "symbols": [
     {
       "accessLevel": "public",
       "declarationFragments": [
         {
           "kind": "keyword",
           "spelling": "typedef"
         },
         {
           "kind": "text",
           "spelling": " "
         },
         {
           "kind": "keyword",
+          "spelling": "enum"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "MyEnum"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@EA@MyEnum"
+      },
+      "kind": {
+        "displayName": "Enumeration",
+        "identifier": "c.enum"
+      },
+      "location": {
+        "position": {
+          "character": 9,
+          "line": 4
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "MyEnum"
+          }
+        ],
+        "title": "MyEnum"
+      },
+      "pathComponents": [
+        "MyEnum"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "Case"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@EA@MyEnum@Case"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "position": {
+          "character": 16,
+          "line": 4
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "Case"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Case"
+          }
+        ],
+        "title": "Case"
+      },
+      "pathComponents": [
+        "MyEnum",
+        "Case"
+      ]
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "typedef"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "keyword",
           "spelling": "struct"
         },
         {
           "kind": "text",
           "spelling": " "
         },
         {
           "kind": "identifier",
           "spelling": "MyStruct"
         }
       ],
       "identifier": {
-        "interfaceLanguage": "objective-c",
+        "interfaceLanguage": "c",
         "precise": "c:@SA@MyStruct"
       },
       "kind": {
         "displayName": "Structure",
-        "identifier": "objective-c.struct"
+        "identifier": "c.struct"
       },
       "location": {
         "position": {
           "character": 9,
           "line": 1
         },
         "uri": "file://INPUT_DIR/input.h"
       },
"symbols": [
           "spelling": " "
         },
         {
           "kind": "identifier",
           "spelling": "MyStructStruct"
         }
       ],
       "identifier": {
-        "interfaceLanguage": "objective-c",
+        "interfaceLanguage": "c",
         "precise": "c:input.h@T@MyStructStruct"
       },
       "kind": {
         "displayName": "Type Alias",
-        "identifier": "objective-c.typealias"
+        "identifier": "c.typealias"
       },
       "location": {
         "position": {
           "character": 18,
           "line": 2
         },
         "uri": "file://INPUT_DIR/input.h"
       },
"symbols": [
           "spelling": " "
         },
         {
           "kind": "identifier",
           "spelling": "MyStructStructStruct"
         }
       ],
       "identifier": {
-        "interfaceLanguage": "objective-c",
+        "interfaceLanguage": "c",
         "precise": "c:input.h@T@MyStructStructStruct"
       },
       "kind": {
         "displayName": "Type Alias",
-        "identifier": "objective-c.typealias"
+        "identifier": "c.typealias"
       },
       "location": {
         "position": {
           "character": 24,
           "line": 3
         },
         "uri": "file://INPUT_DIR/input.h"
       },
"symbols": [
           }
         ],
         "title": "MyStructStructStruct"
       },
       "pathComponents": [
         "MyStructStructStruct"
       ],
       "type": "c:input.h@T@MyStructStruct"
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "typedef"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:@EA@MyEnum",
+          "spelling": "MyEnum"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "MyEnumEnum"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:input.h@T@MyEnumEnum"
+      },
+      "kind": {
+        "displayName": "Type Alias",
+        "identifier": "c.typealias"
+      },
+      "location": {
+        "position": {
+          "character": 16,
+          "line": 5
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "MyEnumEnum"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "MyEnumEnum"
+          }
+        ],
+        "title": "MyEnumEnum"
+      },
+      "pathComponents": [
+        "MyEnumEnum"
+      ],
+      "type": "c:@EA@MyEnum"
+    },
+    {
+      "accessLevel": "public",
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "typedef"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:input.h@T@MyEnumEnum",
+          "spelling": "MyEnumEnum"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "MyEnumEnumEnum"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:input.h@T@MyEnumEnumEnum"
+      },
+      "kind": {
+        "displayName": "Type Alias",
+        "identifier": "c.typealias"
+      },
+      "location": {
+        "position": {
+          "character": 20,
+          "line": 6
+        },
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "navigator": [
+          {
+            "kind": "identifier",
+            "spelling": "MyEnumEnumEnum"
+          }
+        ],
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "MyEnumEnumEnum"
+          }
+        ],
+        "title": "MyEnumEnumEnum"
+      },
+      "pathComponents": [
+        "MyEnumEnumEnum"
+      ],
+      "type": "c:input.h@T@MyEnumEnum"
     }
   ]
 }