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

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.


================
Comment at: clang/lib/ExtractAPI/ExtractAPIVisitor.cpp:174-175
+  StringRef Name = Decl->getName();
   if (Name.empty())
     Name = getTypedefName(Decl);
+  if (Name.empty()) {
----------------
dang wrote:
> zixuw wrote:
> > Aren't these two lines supposed to do this?
> 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.
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?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140010/new/

https://reviews.llvm.org/D140010