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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
