[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In https://reviews.llvm.org/D27104#607161, @lukasza wrote:

> Forcing shallow matching means that unit test below will stop passing after this CL.
>
>   TEST(HasDeclaration, DeepTagType) {
>     std::string input =
>         "class Foo {};\n"
>         "using Bar = Foo;\n"
>         "void Function(Bar param) {}\n";
>   
>     // Matcher for declaration of the Foo class.
>     auto param_type_decl_matcher = cxxRecordDecl(hasName("Foo"));
>   
>     // hasDeclaration / qualType-flavour.
>     EXPECT_TRUE(matches(input, parmVarDecl(
>         hasName("param"),
>         hasType(qualType(hasDeclaration(decl(param_type_decl_matcher)))))));
>   }
>   
>
> I am working on a tool that renames methods in a specific namespace - the tool depends on hasDeclaration doing deep-matching to tell whether an UnresolvedUsingValueDecl and/or CXXDependentScopeMemberExpr end up pointing at a record or template from the namespace of interest.
>
> Q1: I assume that the breaking aspect of this CL is understood and accepted?


Yes. That this worked was by chance.

> Q2: Can you please provide an example code (here or in the CL description) for how to achieve deep matching (since AFAIU hasDeclaration would no longer do deep matching after the CL under review)?  Can deep matching be achieved by combining existing matchers with the now shallow hasDeclaration?  Or does deep matching require authoring a custom matcher?  How would the custom matcher look like (i.e. would it have to consider all possible types that can be desugared - e.g. requiring separate code for each possible type node - i.e. having separate code for hopping over a type alias and over other type nodes).

I think we should either:
a) add a hasAnyDeclaration matcher, or
b) add a desugarsTo() matcher that just triest to run getAs<> on the node.

I'm going to get you an implementation in a different CL.



================
Comment at: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:250
 }
 
 TEST(HasDeclaration, HasDeclarationOfCXXNewExpr) {
----------------
lukasza wrote:
> Could you please add a unit test that covers a scenario made possible by your CL and which involves an elaborated type?
> 
>     TEST(HasDeclaration, ElaboratedTypeAndTemplateSpecializationType) {
>       std::string input =
>           "namespace Namespace {\n"
>           "template<typename T>\n"
>           "class Template {\n"
>           " public:\n"
>           "  void Method() {}\n"
>           "};\n"
>           "}  // namespace Namespace\n"
>           "template <typename U>\n"
>           "void Function(Namespace::Template<U> param) {\n"
>           "  param.Method();\n"
>           "};\n";
>     
>       // Matcher for ::Namespace::Template<T> template decl.
>       auto param_type_decl_matcher = classTemplateDecl(
>           hasName("Template"),
>           hasParent(namespaceDecl(hasName("Namespace"))),
>           has(templateTypeParmDecl(hasName("T"))));
>     
>       // hasDeclaration / qualType-flavour.
>       EXPECT_TRUE(matches(input, parmVarDecl(
>           hasName("param"),
>           hasType(qualType(hasDeclaration(decl(param_type_decl_matcher)))))));
>     }
> 
> I was a little bit surprised that ElaboratedType is handled when going via qualType, but not when matching the type directly - the test code below (an extension of the unit test proposed above) fails to compile.  Is this expected (for this CL?  for long-term?)?
> 
>   // hasDeclaration / elaboratedType-flavour.
>   EXPECT_TRUE(matches(input, parmVarDecl(
>       hasName("param"),
>       hasType(elaboratedType(hasDeclaration(decl(param_type_decl_matcher)))))));
> 

Added unit test and added ElaboratedType to the list of allowed types for hasDeclaration.


https://reviews.llvm.org/D27104