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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
