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

[Łukasz Anforowicz via cfe-commits]

lukasza added inline comments.

================
Comment at: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:2119
@@ +2118,3 @@
+      "template <typename U>\n"
+      "void Function(Namespace::Template<U> param) {\n"
+      "  param.Method();\n"
----------------
klimek wrote:
> Given your use case: why do we need hasDeclaration here at all?
> I'd have expected this working with just matching on the nested name specifier of the type instead of saying hasDeclaration on the template type.
> Btw, if you add a type alias for a class not in the namespace into the namespace (typedef / using), do you wan that to rename or not? :)
> 
> I'd personally probably have expected (2), but I'm never sure in these cases without playing around with more test cases...
> Given your use case: why do we need hasDeclaration here at all?
> I'd have expected this working with just matching on the nested name specifier of the type instead of saying hasDeclaration on the template type.

Because I want "namespace-of-user-provided-declaration" matching to work both for ElaboratedType nodes (with explicit nested name specifier) and for other kinds of nodes (where there might be no nested name specifier).  I was hoping that I could do this with a single hasDeclaration matcher, rather than listing all possible type nodes myself (when building my own matcher) like I sort of do in a workaround.  In particular, after this CL a single, simple hasDeclaration-based matcher can be used in
    //    auto blink_qual_type_base_matcher =
    //        qualType(hasDeclaration(in_blink_namespace));
inside https://codereview.chromium.org/2256913002/patch/180001/190001.

> Btw, if you add a type alias for a class not in the namespace into the namespace (typedef / using), do you wan that to rename or not? :)

Good question.  I want a rename to happen if I have ::SomeOtherNamespace::Typedef resolving to ::NamespaceWithRenamedMethods::Class, but I do not want rename to happen if I have ::NamespaceWithRenamtedMethods::Typedef resolving to ::SomeOtherNamespace::Class.  I guess my current hasDeclaration-based matcher will match both cases :-(  One way to fix this would be to exclude typedefs in |decl_under_blink_namespace| at https://chromium.googlesource.com/chromium/src/+/14d095b4df6754fa4e6959220b2b332db0b4f504/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp#646

But... this question+answer should have no impact on the CL under review, right?

> I'd personally probably have expected (2), but I'm never sure in these cases without playing around with more test cases...

Ok.  This (#2) is what the current patch results in.


https://reviews.llvm.org/D24361