davrec accepted this revision.
davrec added a comment.
This revision is now accepted and ready to land.
Looks great, thanks for identifying the need and banging this out so quickly.
Hope you had some time to enjoy the holiday with your family!
Dave
================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2106
+ TRY_TO(TraverseDecl(Child));
+ }
}
----------------
So the idea is, the UsingDecl which introduced the shadows will have already
been traversed within its DeclStmt via the body traversal, but not the
UsingShadowDecls it introduced? Nicely spotted.
================
Comment at: clang/include/clang/AST/Type.h:4381
+public:
+ NamedDecl *getFoundDecl() const { return Found; }
+ QualType getUnderlyingType() const { return UnderlyingTy; }
----------------
sammccall wrote:
> davrec wrote:
> > I would rename this to `getDecl()`, to match the interface for other types
> > that just wrap a decl. E.g. if something is a RecordType I know I can call
> > getDecl() to get the RecordDecl; likewise a TypedefType::getDecl() returns
> > a TypedefNameDecl; I think it would follow this pattern for UsingType to
> > have a getDecl() method that returns a UsingShadowDecl (or whatever else it
> > can be, per other question).
> >
> I do prefer `getFoundDecl()` for a few reasons:
> - the parallel with `NamedDecl::getFoundDecl()` is closer/more important
> than with `TypedefDecl` I think
> - there are always two decls here: the invisible UsingShadowDecl and the
> underlying one. Saying "decl" without a hint seems error-prone to me.
> (Compare with TypedefType where in general there's no underlying decl).
> - I **do** find TypedefType::getDecl() confusing, because wherever I see it
> called I have to verify that it's TypedefType::getDecl() rather than some
> Type::getDecl() to be sure I understand the semantics.
>
> Would be happy to hear a third opinion here though.
I'm persuaded by your reasoning, particularly that UsingShadowDecl is invisible
and thus returning one already introduces some confusion -- i.e. a user might
reasonably expect getDecl() to return a UsingDecl, so better to call this
something with other parallels.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114251/new/
https://reviews.llvm.org/D114251
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
