davrec added a comment.
Looks good, a few notes.
================
Comment at: clang/include/clang/AST/ASTContext.h:1559
+ QualType getUsingType(const NamedDecl *Found, QualType Underlying) const;
+
----------------
I believe you mentioned ObjC considerations might require expanding beyond
UsingShadowDecl as the type of `Found` -- is that why this is a generic
NamedDecl? I.e. can `Found` indeed be other things than a UsingShadowDecl?
================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:984
+DEF_TRAVERSE_TYPE(UsingType, { TRY_TO(TraverseType(T->getUnderlyingType())); })
DEF_TRAVERSE_TYPE(UnresolvedUsingType, {})
----------------
This should just be DEF_TRAVERSE_TYPE(UsingType, {}), i.e. same as for
TypedefType -- RecursiveASTVisitor only visits the child nodes, to avoid
retraversal of any nodes; by traversing the desugared node this would jump back
to a node it probably already traversed.
================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1256
+DEF_TRAVERSE_TYPELOC(UsingType,
+ { TRY_TO(TraverseTypeLoc(TL.getUnderlyingLoc())); })
----------------
DEF_TRAVERSE_TYPELOC(UsingType, {})
================
Comment at: clang/include/clang/AST/Type.h:4381
+public:
+ NamedDecl *getFoundDecl() const { return Found; }
+ QualType getUnderlyingType() const { return UnderlyingTy; }
----------------
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).
================
Comment at: clang/include/clang/AST/TypeLoc.h:671
+/// Wrapper for source info for types used via transparent aliases.
+class UsingTypeLoc : public ConcreteTypeLoc<UnqualTypeLoc, UsingTypeLoc,
+ UsingType, UsingLocInfo> {
----------------
Should this be analogous to TypedefTypeLoc, with base as
```
InheritingConcreteTypeLoc<TypeSpecTypeLoc,
UsingTypeLoc,
UsingType>
```
?
================
Comment at: clang/include/clang/AST/TypeProperties.td:366
+let Class = UsingType in {
+ def : Property<"foundDeclaration", NamedDeclRef> {
+ let Read = [{ node->getFoundDecl() }];
----------------
Rename to "declaration" iff renaming UsingType::getFoundDecl to
UsingType::getDecl
================
Comment at: clang/lib/AST/ASTContext.cpp:4612
+ Canon = getCanonicalType(Underlying);
+ UsingType *NewType = new UsingType(Found, Underlying, Canon);
+ Types.push_back(NewType);
----------------
UsingType *NewType = new (*this, TypeAlignment) UsingType(Found, Underlying,
Canon)
================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:952
+ return false;
+ if (!IsStructurallyEquivalent(Context,
+ cast<UsingType>(T1)->getUnderlyingType(),
----------------
Is there a good reason this checks the underlying type in addition to
getFoundDecl()? I'm looking at the Typedef case below and it only checks the
decl, does this need to be different from that?
================
Comment at: clang/lib/AST/Type.cpp:1819
+ Type *VisitUsingType(const UsingType *T) {
+ return Visit(T->getUnderlyingType());
----------------
Do we need this here? I ask because I mainly because don't see a
VisitTypedefType, not 100% sure what this class does though.
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
