[PATCH] D24361: hasDeclaration(qualType(...)) matcher should unwrap ElaboratedType and TemplateSpecializationType

[Łukasz Anforowicz via cfe-commits]

lukasza added a comment.

Richard, could you please take a look?


================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:750
@@ +749,3 @@
+    else if (auto *ET = Node->getAs<ElaboratedType>())
+      return matchesSpecialized(ET->getNamedType(), Finder, Builder);
+    else if (auto *TST = Node->getAs<TemplateSpecializationType>())
----------------
There is also a question whether I should not only start supporting not only qualType(hasDeclaration(...)) when qualType wraps an ElaboratedType node, but also start supporting elaboratedType(hasDeclaration(...)).

Snippets of a patch that could achieve this (but then again - I am not sure if this is desirable and/or necessary):

    +++ include/clang/ASTMatchers/ASTMatchersInternal.h	(working copy)
    ...
    +  /// \brief Gets the QualType from ElaboratedType
    +  /// and returns whether the inner matches on it.
    +  bool matchesSpecialized(const ElaboratedType &Node,
    +                          ASTMatchFinder *Finder,
    +                          BoundNodesTreeBuilder *Builder) const {
    +    return matchesSpecialized(Node.getNamedType(), Finder, Builder);
    +  }
    +
    ...
     /// \brief All types that are supported by HasDeclarationMatcher above.
    -typedef TypeList<CallExpr, CXXConstructExpr, DeclRefExpr, EnumType,
    -                 InjectedClassNameType, LabelStmt, AddrLabelExpr, MemberExpr,
    -                 QualType, RecordType, TagType, TemplateSpecializationType,
    -                 TemplateTypeParmType, TypedefType,
    +typedef TypeList<AutoType, CallExpr, CXXConstructExpr, DeclRefExpr, ElaboratedType,
    +                 EnumType, InjectedClassNameType, LabelStmt, AddrLabelExpr,
    +                 MemberExpr, QualType, RecordType, TagType,
    +                 TemplateSpecializationType, TemplateTypeParmType, TypedefType,
                      UnresolvedUsingType> HasDeclarationSupportedTypes;

================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:753
@@ -749,2 +752,3 @@
+      return matchesSpecialized(*TST, Finder, Builder);
     else if (auto *TT = Node->getAs<TypedefType>())
       return matchesDecl(TT->getDecl(), Finder, Builder);
----------------
I do notice that there are quite a few similar Node->getAs<T> if statements here.  They could be replaced with something "clever" (and probably unreadable):

    +++ include/clang/ASTMatchers/ASTMatchersInternal.h	(working copy)
    ...
    +  /// \brief Terminating case for recursion over template parameters
    +  /// performed by matchesQualTypeAsAnyOf template.
    +  template <typename U>
    +  bool matchesQualTypeAsAnyOf(const QualType &Node, ASTMatchFinder *Finder,
    +                              BoundNodesTreeBuilder *Builder) const {
    +    return false;
    +  }
    +
    +  /// \brief Returns true if Node->getAs<U> is not null and matches inner
    +  /// matcher for any type U listed in template parameters.
    +  template <typename U, typename... UTail,
    +           typename std::enable_if<!std::is_void<U>::value, int>::type = 0>
    +  bool matchesQualTypeAsAnyOf(const QualType &Node, ASTMatchFinder *Finder,
    +                              BoundNodesTreeBuilder *Builder) const {
    +    if (const U *u = Node->getAs<U>())
    +      return matchesSpecialized(*u, Finder, Builder);
    +
    +    return matchesQualTypeAsAnyOf<UTail...>(Node, Finder, Builder);
    +  }
    +
    ...
    -    else if (auto *OCIT = Node->getAs<ObjCInterfaceType>())
    -      return matchesDecl(OCIT->getDecl(), Finder, Builder);
    -    else if (auto *UUT = Node->getAs<UnresolvedUsingType>())
    -      return matchesDecl(UUT->getDecl(), Finder, Builder);
    -    else if (auto *ICNT = Node->getAs<InjectedClassNameType>())
    -      return matchesDecl(ICNT->getDecl(), Finder, Builder);
    -    return false;
    +    else if (auto *TD = Node->getAsTagDecl())
    +      return matchesDecl(TD, Finder, Builder);
    +
    +    return matchesQualTypeAsAnyOf<
    +        ArrayType, InjectedClassNameType, ObjCInterfaceType, TemplateSpecializationType,
    +        TypedefType, UnresolvedUsingType, void>(
    +            Node, Finder, Builder);

================
Comment at: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:2124
@@ +2123,3 @@
+  // hasDeclaration should see through:
+  // 1. from ElaboratedType (Namespace::MyTemplate<U>) to
+  //    TemplateSpecializationType (MyTemplate<U>)
----------------
FWIW, I couldn't repro a similar issue (i.e. VarDecl or ParmVarDecl of a type with user-provided declaration, but where hasDeclaration doesn't match *anything*) for other types (i.e. AutoType or ArrayType) - only for ElaboratedType and TemplateSpecializationType.


https://reviews.llvm.org/D24361