[ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[mboehme]

Summary

Required for D22220

[mboehme]

Retracting from review for the moment. Tests fail to compile (embarrasingly enough, I ran the wrong test suite -- apologies).

[mboehme]

Updated test and documentation for hasUnderlyingDecl() to use using declarations instead of typedefs. (I made the mistaken assumption that getUnderlyingDecl() would also work on a typdef and didn't catch the mistake because I ran the wrong tests.)

[alexfh]

include/clang/ASTMatchers/ASTMatchers.h

2440 ↗

(On Diff #66291)

nit: Parentheses are superfluous here.

2807 ↗

(On Diff #66291)

Please add an unresolvedLookupExpr that this matcher doesn't match (maybe change the matcher to make it easier).

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

2809 ↗

(On Diff #66291)

I find the name of this matcher a little bit confusing. The documentation doesn't describe what the matcher does (can you please clarify the docs?). The implementation suggests that this is looking to see if the given decl exists in the overload expression set, which makes me wonder why this isn't implemented on the hasDeclaration() traversal matcher rather than adding a new matcher name?

[klimek]

include/clang/ASTMatchers/ASTMatchers.h

2809 ↗

(On Diff #66291)

I don't like hasDeclaration, as that's so far used when there is exactly one. Something like hasAnyDeclaration might also fit, but I think canReferToDecl makes it clear that there is not yet a specific decl, but a couple of candidates we'll want to select from.

[mboehme]

I've rebased the patch to a newer head revision, which seems to confuse Phabricator when diffing against earlier versions of the patch. Apologies. I assume this means I should avoid doing this (rebasing to a newer revision)?

[mboehme]

include/clang/ASTMatchers/ASTMatchers.h

2486 ↗

(On Diff #66808)

I was trying to match the style of the next matcher below -- or do you think I should establish a precedent / preference for emitting the superfluous parentheses?

2855 ↗

(On Diff #66808)

The documentation doesn't describe what the matcher does (can you please clarify the docs?).

I've rephrased the description of the matcher -- is it clearer now?

The implementation suggests that this is looking to see if the given decl exists in the overload expression set, which makes me wonder why this isn't implemented on the hasDeclaration() traversal matcher rather than adding a new matcher name?

As klimek notes, the difference is that hasDeclaration() is currently used only for nodes that have exactly one associated declaration.

My proposal would be to stay with canReferToDecl -- thoughts?

[alexfh]

In D23004#505854, @mboehme wrote:

I assume this means I should avoid doing this (rebasing to a newer revision)?

No, rebasing is totally fine. But if some files changed in the meantime, comparing with older revisions will contain unrelated changes, that's expected, I think.

[alexfh]

include/clang/ASTMatchers/ASTMatchers.h

2486 ↗

(On Diff #66808)

Yes, I consider parentheses with return are confusing - I'm immediately trying to find a reason, why they are needed.

2855 ↗

(On Diff #66808)

+1 to canReferToDecl

[mboehme]

Removed superfluous parentheses

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

2855 ↗

(On Diff #66808)

canReferToDecl doesn't make it any more clear that there could be multiple candidates, and I find its usage to read really strangely. However, it is a good point about hasDeclaration usage being singular currently. My preference is for hasAnyDeclaration, and the precedence is hasAnyName, hasAnyConstructorInitializer, hasAnyArgument, etc.

The documentation does read better now, thank you!

[mboehme]

• Rename canReferToDecl to hasAnyDeclaration

[aaron.ballman]

Assuming others are fine with hasAnyDeclaration() vs canReferToDecl(), LGTM! If others feel strongly about canReferToDecl() instead, it still LGTM.

[alexfh]

LG

cfe/trunk/docs/LibASTMatchersReference.html

 pointerType(pointee(isConstQualified(), isInteger()))
   matches "int const *b"
 
 Usable as: Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1BlockPointerType.html">BlockPointerType</a>&gt;, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1MemberPointerType.html">MemberPointerType</a>&gt;,
   Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1PointerType.html">PointerType</a>&gt;, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ReferenceType.html">ReferenceType</a>&gt;
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html">NamedDecl</a>&gt;</td><td class="name" onclick="toggle('hasUnderlyingDecl0')"><a name="hasUnderlyingDecl0Anchor">hasUnderlyingDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html">NamedDecl</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasUnderlyingDecl0"><pre>Matches a NamedDecl whose underlying declaration matches the given
+matcher.
+
+Given
+  namespace N { template&lt;class T&gt; void f(T t); }
+  template &lt;class T&gt; void g() { using N::f; f(T()); }
+unresolvedLookupExpr(hasAnyDeclaration(
+    namedDecl(hasUnderlyingDecl(hasName("::N::f")))))
+  matches the use of f in g() .
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1NestedNameSpecifierLoc.html">NestedNameSpecifierLoc</a>&gt;</td><td class="name" onclick="toggle('hasPrefix1')"><a name="hasPrefix1Anchor">hasPrefix</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1NestedNameSpecifierLoc.html">NestedNameSpecifierLoc</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasPrefix1"><pre>Matches on the prefix of a NestedNameSpecifierLoc.
 
 Given
   struct A { struct B { struct C {}; }; };
   A::B::C c;
 nestedNameSpecifierLoc(hasPrefix(loc(specifiesType(asString("struct A")))))
   matches "A::"
matches the [webView ...] message invocation.
   [webView stringByEvaluatingJavaScriptFromString:webViewJavascript];
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1OpaqueValueExpr.html">OpaqueValueExpr</a>&gt;</td><td class="name" onclick="toggle('hasSourceExpression1')"><a name="hasSourceExpression1Anchor">hasSourceExpression</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasSourceExpression1"><pre></pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1OverloadExpr.html">OverloadExpr</a>&gt;</td><td class="name" onclick="toggle('hasAnyDeclaration0')"><a name="hasAnyDeclaration0Anchor">hasAnyDeclaration</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasAnyDeclaration0"><pre>Matches an OverloadExpr if any of the declarations in the set of
+overloads matches the given matcher.
+
+Given
+  template &lt;typename T&gt; void foo(T);
+  template &lt;typename T&gt; void bar(T);
+  template &lt;typename T&gt; void baz(T t) {
+    foo(t);
+    bar(t);
+  }
+unresolvedLookupExpr(hasAnyDeclaration(
+    functionTemplateDecl(hasName("foo"))))
+  matches foo in foo(t); but not bar in bar(t);
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ParenType.html">ParenType</a>&gt;</td><td class="name" onclick="toggle('innerType0')"><a name="innerType0Anchor">innerType</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Type.html">Type</a>&gt;</td></tr>
 <tr><td colspan="4" class="doc" id="innerType0"><pre>Matches ParenType nodes where the inner type is a specific type.
 
 Given
   int (*ptr_to_array)[4];
   int (*ptr_to_func)(int);
 
 varDecl(hasType(pointsTo(parenType(innerType(functionType()))))) matches

cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

inline internal::PolymorphicMatcherWithParam1<
     internal::HasDeclarationMatcher, internal::Matcher<Decl>,
     void(internal::HasDeclarationSupportedTypes)>
 hasDeclaration(const internal::Matcher<Decl> &InnerMatcher) {
   return internal::PolymorphicMatcherWithParam1<
       internal::HasDeclarationMatcher, internal::Matcher<Decl>,
       void(internal::HasDeclarationSupportedTypes)>(InnerMatcher);
 }
 
+/// \brief Matches a \c NamedDecl whose underlying declaration matches the given
+/// matcher.
+///
+/// Given
+/// \code
+///   namespace N { template<class T> void f(T t); }
+///   template <class T> void g() { using N::f; f(T()); }
+/// \endcode
+/// \c unresolvedLookupExpr(hasAnyDeclaration(
+///     namedDecl(hasUnderlyingDecl(hasName("::N::f")))))
+///   matches the use of \c f in \c g() .
+AST_MATCHER_P(NamedDecl, hasUnderlyingDecl, internal::Matcher<NamedDecl>,
+              InnerMatcher) {
+  const NamedDecl *UnderlyingDecl = Node.getUnderlyingDecl();
+
+  return UnderlyingDecl != nullptr &&
+         InnerMatcher.matches(*UnderlyingDecl, Finder, Builder);
+}
+
 /// \brief Matches on the implicit object argument of a member call expression.
 ///
 /// Example matches y.x()
 ///   (matcher = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y"))))))
 /// \code
 ///   class Y { public: void x(); };
 ///   void z() { Y y; y.x(); }",
 /// \endcode

 AST_MATCHER_P(DeclRefExpr, throughUsingDecl,
               internal::Matcher<UsingShadowDecl>, InnerMatcher) {
   const NamedDecl *FoundDecl = Node.getFoundDecl();
   if (const UsingShadowDecl *UsingDecl = dyn_cast<UsingShadowDecl>(FoundDecl))
     return InnerMatcher.matches(*UsingDecl, Finder, Builder);
   return false;
 }
 
+/// \brief Matches an \c OverloadExpr if any of the declarations in the set of
+/// overloads matches the given matcher.
+///
+/// Given
+/// \code
+///   template <typename T> void foo(T);
+///   template <typename T> void bar(T);
+///   template <typename T> void baz(T t) {
+///     foo(t);
+///     bar(t);
+///   }
+/// \endcode
+/// unresolvedLookupExpr(hasAnyDeclaration(
+///     functionTemplateDecl(hasName("foo"))))
+///   matches \c foo in \c foo(t); but not \c bar in \c bar(t);
+AST_MATCHER_P(OverloadExpr, hasAnyDeclaration, internal::Matcher<Decl>,
+              InnerMatcher) {
+  return matchesFirstInPointerRange(InnerMatcher, Node.decls_begin(),
+                                    Node.decls_end(), Finder, Builder);
+}
+
 /// \brief Matches the Decl of a DeclStmt which has a single declaration.
 ///
 /// Given
 /// \code
 ///   int a, b;
 ///   int c;
 /// \endcode
 /// declStmt(hasSingleDecl(anything()))

cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(functionTemplateDecl);
   REGISTER_MATCHER(functionType);
   REGISTER_MATCHER(gnuNullExpr);
   REGISTER_MATCHER(gotoStmt);
   REGISTER_MATCHER(has);
   REGISTER_MATCHER(hasAncestor);
   REGISTER_MATCHER(hasAnyArgument);
   REGISTER_MATCHER(hasAnyConstructorInitializer);
+  REGISTER_MATCHER(hasAnyDeclaration);
   REGISTER_MATCHER(hasAnyName);
   REGISTER_MATCHER(hasAnyParameter);
   REGISTER_MATCHER(hasAnySubstatement);
   REGISTER_MATCHER(hasAnyTemplateArgument);
   REGISTER_MATCHER(hasAnyUsingShadowDecl);
   REGISTER_MATCHER(hasArgument);
   REGISTER_MATCHER(hasArgumentOfType);
   REGISTER_MATCHER(hasAttr);
RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
   REGISTER_MATCHER(hasThen);
   REGISTER_MATCHER(hasThreadStorageDuration);
   REGISTER_MATCHER(hasTrueExpression);
   REGISTER_MATCHER(hasTypeLoc);
   REGISTER_MATCHER(hasUnaryOperand);
   REGISTER_MATCHER(hasUnarySelector);
+  REGISTER_MATCHER(hasUnderlyingDecl);
   REGISTER_MATCHER(hasValueType);
   REGISTER_MATCHER(ifStmt);
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringImpCasts);
   REGISTER_MATCHER(ignoringParenCasts);
   REGISTER_MATCHER(ignoringParenImpCasts);
   REGISTER_MATCHER(ignoringParens);
   REGISTER_MATCHER(implicitCastExpr);

cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

 }
 
 TEST(HasDeclaration, HasDeclarationOfTemplateSpecializationType) {
   EXPECT_TRUE(matches("template <typename T> class A {}; A<int> a;",
                       varDecl(hasType(templateSpecializationType(
                         hasDeclaration(namedDecl(hasName("A"))))))));
 }
 
+TEST(HasUnderlyingDecl, Matches) {
+  EXPECT_TRUE(matches("namespace N { template <class T> void f(T t); }"
+                      "template <class T> void g() { using N::f; f(T()); }",
+                      unresolvedLookupExpr(hasAnyDeclaration(
+                          namedDecl(hasUnderlyingDecl(hasName("::N::f")))))));
+  EXPECT_TRUE(matches(
+      "namespace N { template <class T> void f(T t); }"
+      "template <class T> void g() { N::f(T()); }",
+      unresolvedLookupExpr(hasAnyDeclaration(namedDecl(hasName("::N::f"))))));
+  EXPECT_TRUE(notMatches(
+      "namespace N { template <class T> void f(T t); }"
+      "template <class T> void g() { using N::f; f(T()); }",
+      unresolvedLookupExpr(hasAnyDeclaration(namedDecl(hasName("::N::f"))))));
+}
+
 TEST(HasType, TakesQualTypeMatcherAndMatchesExpr) {
   TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
   EXPECT_TRUE(
     matches("class X {}; void y(X &x) { x; }", expr(hasType(ClassX))));
   EXPECT_TRUE(
     notMatches("class X {}; void y(X *x) { x; }",
                expr(hasType(ClassX))));
   EXPECT_TRUE(
TEST(Matcher, ForEachOverriden) {
   EXPECT_TRUE(matchAndVerifyResultTrue(
       Code2, ForEachOverriddenInClass("B"),
       llvm::make_unique<VerifyIdIsBoundTo<CXXMethodDecl>>("overridden", "f",
                                                           2)));
   // A1::f overrides nothing.
   EXPECT_TRUE(notMatches(Code2, ForEachOverriddenInClass("A1")));
 }
 
+TEST(Matcher, HasAnyDeclaration) {
+  std::string Fragment = "void foo(int p1);"
+                         "void foo(int *p2);"
+                         "void bar(int p3);"
+                         "template <typename T> void baz(T t) { foo(t); }";
+
+  EXPECT_TRUE(
+      matches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(functionDecl(
+                            hasParameter(0, parmVarDecl(hasName("p1"))))))));
+  EXPECT_TRUE(
+      matches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(functionDecl(
+                            hasParameter(0, parmVarDecl(hasName("p2"))))))));
+  EXPECT_TRUE(
+      notMatches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(functionDecl(
+                               hasParameter(0, parmVarDecl(hasName("p3"))))))));
+  EXPECT_TRUE(notMatches(Fragment, unresolvedLookupExpr(hasAnyDeclaration(
+                                       functionDecl(hasName("bar"))))));
+}
+
 } // namespace ast_matchers
 } // namespace clang