Introduces a new concept for binding results to matchers

as per Chandler's request:
- introduces a new matcher base type BindableMatcher that
  provides the bind() call
- makes all dynamic-cast matcher creation functions return
  BindableMatchers; the special case about dynamic-cast
  matchers is that the node they match on and the node
  their child matchers match on are the same node, just
  casted to a different type; thus, there is no ambiguity
  on what bind() matches on; additionally, those are the
  matchers that we name with nouns in the matcher language,
  so it's easy for users to intuitively know which matchers
  are bindable

To make this change possible, we got rid of a non-orthogonal
implementation of thisPointerType, which had an implicit
dynamic-cast matcher from CallExpr to CXXMemberCallExpr; as
alternative, we now provide a memberCall dynamic-cast matcher
and thisPointerType is a predicate on CXXMemberCallExpr.

Last, the ArgumentAdaptingMatcher is actually not required
for the implementation of makeDynCastAllOfComposite - this
simplification makes it more obvious where the bind() call
can be used based on the matcher creation function types.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@160673 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 74 insertions, 53 deletions

diff --git a/include/clang/ASTMatchers/ASTMatchers.h b/include/clang/ASTMatchers/ASTMatchers.h
index d2ff604946..61571e5fa0 100644
--- a/include/clang/ASTMatchers/ASTMatchers.h
+++ b/include/clang/ASTMatchers/ASTMatchers.h
@@ -104,8 +104,8 @@ private:
 /// FIXME: Add example for accessing it.
 template <typename T>
 internal::Matcher<T> id(const std::string &ID,
-                        const internal::Matcher<T> &InnerMatcher) {
-  return internal::Matcher<T>(new internal::IdMatcher<T>(ID, InnerMatcher));
+                        const internal::BindableMatcher<T> &InnerMatcher) {
+  return InnerMatcher.bind(ID);
 }
 
 /// \brief Types of matchers for the top-level classes in the AST class
@@ -338,11 +338,19 @@ const internal::VariadicDynCastAllOfMatcher<
 
 /// \brief Matches call expressions.
 ///
-/// Example matches x.y()
+/// Example matches x.y() and y()
 ///   X x;
 ///   x.y();
+///   y();
 const internal::VariadicDynCastAllOfMatcher<Stmt, CallExpr> call;
 
+/// \brief Matches member call expressions.
+///
+/// Example matches x.y()
+///   X x;
+///   x.y();
+const internal::VariadicDynCastAllOfMatcher<Stmt, CXXMemberCallExpr> memberCall;
+
 /// \brief Matches init list expressions.
 ///
 /// Given
@@ -1120,14 +1128,14 @@ AST_MATCHER_P(CXXMemberCallExpr, onImplicitObjectArgument,
 
 /// \brief Matches if the expression's type either matches the specified
 /// matcher, or is a pointer to a type that matches the InnerMatcher.
-inline internal::Matcher<CallExpr> thisPointerType(
+inline internal::Matcher<CXXMemberCallExpr> thisPointerType(
     const internal::Matcher<QualType> &InnerMatcher) {
   return onImplicitObjectArgument(
       anyOf(hasType(InnerMatcher), hasType(pointsTo(InnerMatcher))));
 }
 
 /// \brief Overloaded to match the type's declaration.
-inline internal::Matcher<CallExpr> thisPointerType(
+inline internal::Matcher<CXXMemberCallExpr> thisPointerType(
     const internal::Matcher<Decl> &InnerMatcher) {
   return onImplicitObjectArgument(
       anyOf(hasType(InnerMatcher), hasType(pointsTo(InnerMatcher))));
diff --git a/include/clang/ASTMatchers/ASTMatchersInternal.h b/include/clang/ASTMatchers/ASTMatchersInternal.h
index 4d044018a9..3f5568521c 100644
--- a/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -440,20 +440,19 @@ public:
                                    BindKind Bind) = 0;
 };
 
-/// \brief Converts a Matcher<T> to a matcher of desired type To by "adapting"
-/// a To into a T.
+/// \brief Converts a \c Matcher<T> to a matcher of desired type \c To by
+/// "adapting" a \c To into a \c T.
 ///
-/// The ArgumentAdapterT argument specifies how the adaptation is done.
+/// The \c ArgumentAdapterT argument specifies how the adaptation is done.
 ///
 /// For example:
-///   ArgumentAdaptingMatcher<DynCastMatcher, T>(InnerMatcher);
-/// returns a matcher that can be used where a Matcher<To> is required, if
-/// To and T are in the same type hierarchy, and thus dyn_cast can be
-/// called to convert a To to a T.
+///   \c ArgumentAdaptingMatcher<HasMatcher, T>(InnerMatcher);
+/// Given that \c InnerMatcher is of type \c Matcher<T>, this returns a matcher
+/// that is convertible into any matcher of type \c To by constructing
+/// \c HasMatcher<To, T>(InnerMatcher).
 ///
-/// FIXME: Make sure all our applications of this class actually require
-/// knowledge about the inner type. DynCastMatcher obviously does, but the
-/// Has *matchers require the inner type solely for COMPILE_ASSERT purposes.
+/// If a matcher does not need knowledge about the inner type, prefer to use
+/// PolymorphicMatcherWithParam1.
 template <template <typename ToArg, typename FromArg> class ArgumentAdapterT,
           typename T>
 class ArgumentAdaptingMatcher {
@@ -567,19 +566,6 @@ private:
   const Matcher<To> InnerMatcher;
 };
 
-/// \brief Enables the user to pass a Matcher<CXXMemberCallExpr> to
-/// Call().
-///
-/// FIXME: Alternatives are using more specific methods than Call, like
-/// MemberCall, or not using VariadicFunction for Call and overloading it.
-template <>
-template <>
-inline Matcher<CXXMemberCallExpr>::
-operator Matcher<CallExpr>() const {
-  return makeMatcher(
-    new DynCastMatcher<CallExpr, CXXMemberCallExpr>(*this));
-}
-
 /// \brief Matcher<T> that wraps an inner Matcher<T> and binds the matched node
 /// to an ID if the inner matcher matches on the node.
 template <typename T>
@@ -605,6 +591,28 @@ private:
   const Matcher<T> InnerMatcher;
 };
 
+/// \brief A Matcher that allows binding the node it matches to an id.
+///
+/// BindableMatcher provides a \a bind() method that allows binding the
+/// matched node to an id if the match was successful.
+template <typename T>
+class BindableMatcher : public Matcher<T> {
+public:
+  BindableMatcher(MatcherInterface<T> *Implementation)
+    : Matcher<T>(Implementation) {}
+
+  /// \brief Returns a matcher that will bind the matched node on a match.
+  ///
+  /// The returned matcher is equivalent to this matcher, but will
+  /// bind the matched node on a match.
+  Matcher<T> bind(StringRef ID) const {
+    TOOLING_COMPILE_ASSERT((llvm::is_base_of<Stmt, T>::value ||
+                            llvm::is_base_of<Decl, T>::value),
+      trying_to_bind_unsupported_node_type__only_decl_and_stmt_supported);
+    return Matcher<T>(new IdMatcher<T>(ID, *this));
+  }
+};
+
 /// \brief Matches nodes of type T that have child nodes of type ChildT for
 /// which a specified child matcher matches.
 ///
@@ -727,12 +735,16 @@ private:
 
 /// \brief Creates a Matcher<T> that matches if
 /// T is dyn_cast'able into InnerT and all inner matchers match.
+///
+/// Returns BindableMatcher, as matchers that use dyn_cast have
+/// the same object both to match on and to run submatchers on,
+/// so there is no ambiguity with what gets bound.
 template<typename T, typename InnerT>
-Matcher<T> makeDynCastAllOfComposite(
+BindableMatcher<T> makeDynCastAllOfComposite(
     ArrayRef<const Matcher<InnerT> *> InnerMatchers) {
   if (InnerMatchers.empty()) {
-    return ArgumentAdaptingMatcher<DynCastMatcher, InnerT>(
-        makeMatcher(new TrueMatcher<InnerT>));
+    Matcher<InnerT> InnerMatcher = makeMatcher(new TrueMatcher<InnerT>);
+    return BindableMatcher<T>(new DynCastMatcher<T, InnerT>(InnerMatcher));
   }
   Matcher<InnerT> InnerMatcher = *InnerMatchers.back();
   for (int i = InnerMatchers.size() - 2; i >= 0; --i) {
@@ -740,7 +752,7 @@ Matcher<T> makeDynCastAllOfComposite(
         new AllOfMatcher<InnerT, Matcher<InnerT>, Matcher<InnerT> >(
             *InnerMatchers[i], InnerMatcher));
   }
-  return ArgumentAdaptingMatcher<DynCastMatcher, InnerT>(InnerMatcher);
+  return BindableMatcher<T>(new DynCastMatcher<T, InnerT>(InnerMatcher));
 }
 
 /// \brief Matches nodes of type T that have at least one descendant node of
@@ -876,7 +888,7 @@ class IsConstQualifiedMatcher
 template <typename SourceT, typename TargetT>
 class VariadicDynCastAllOfMatcher
     : public llvm::VariadicFunction<
-        Matcher<SourceT>, Matcher<TargetT>,
+        BindableMatcher<SourceT>, Matcher<TargetT>,
         makeDynCastAllOfComposite<SourceT, TargetT> > {
 public:
   VariadicDynCastAllOfMatcher() {}
diff --git a/unittests/ASTMatchers/ASTMatchersTest.cpp b/unittests/ASTMatchers/ASTMatchersTest.cpp
index 5791932b17..f76a59696f 100644
--- a/unittests/ASTMatchers/ASTMatchersTest.cpp
+++ b/unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -283,7 +283,7 @@ TEST(DeclarationMatcher, ClassIsDerived) {
 
   EXPECT_TRUE(matches(
       "class X {}; class Y : public X {};",
-      record(isDerivedFrom(id("test", record(hasName("X")))))));
+      record(isDerivedFrom(record(hasName("X")).bind("test")))));
 }
 
 TEST(AllOf, AllOverloadsWork) {
@@ -620,7 +620,7 @@ private:
 };
 
 TEST(Matcher, BindMatchedNodes) {
-  DeclarationMatcher ClassX = has(id("x", record(hasName("X"))));
+  DeclarationMatcher ClassX = has(record(hasName("::X")).bind("x"));
 
   EXPECT_TRUE(matchAndVerifyResultTrue("class X {};",
       ClassX, new VerifyIdIsBoundToDecl<CXXRecordDecl>("x")));
@@ -629,13 +629,13 @@ TEST(Matcher, BindMatchedNodes) {
       ClassX, new VerifyIdIsBoundToDecl<CXXRecordDecl>("other-id")));
 
   TypeMatcher TypeAHasClassB = hasDeclaration(
-      record(hasName("A"), has(id("b", record(hasName("B"))))));
+      record(hasName("A"), has(record(hasName("B")).bind("b"))));
 
   EXPECT_TRUE(matchAndVerifyResultTrue("class A { public: A *a; class B {}; };",
       TypeAHasClassB,
       new VerifyIdIsBoundToDecl<Decl>("b")));
 
-  StatementMatcher MethodX = id("x", call(callee(method(hasName("x")))));
+  StatementMatcher MethodX = call(callee(method(hasName("x")))).bind("x");
 
   EXPECT_TRUE(matchAndVerifyResultTrue("class A { void x() { x(); } };",
       MethodX,
@@ -645,11 +645,11 @@ TEST(Matcher, BindMatchedNodes) {
 TEST(Matcher, BindTheSameNameInAlternatives) {
   StatementMatcher matcher = anyOf(
       binaryOperator(hasOperatorName("+"),
-                     hasLHS(id("x", expression())),
+                     hasLHS(expression().bind("x")),
                      hasRHS(integerLiteral(equals(0)))),
       binaryOperator(hasOperatorName("+"),
                      hasLHS(integerLiteral(equals(0))),
-                     hasRHS(id("x", expression()))));
+                     hasRHS(expression().bind("x"))));
 
   EXPECT_TRUE(matchAndVerifyResultTrue(
       // The first branch of the matcher binds x to 0 but then fails.
@@ -707,7 +707,7 @@ TEST(Matcher, Call) {
   EXPECT_TRUE(matches("class Y { void x() { x(); } };", MethodX));
   EXPECT_TRUE(notMatches("class Y { void x() {} };", MethodX));
 
-  StatementMatcher MethodOnY = call(on(hasType(record(hasName("Y")))));
+  StatementMatcher MethodOnY = memberCall(on(hasType(record(hasName("Y")))));
 
   EXPECT_TRUE(
       matches("class Y { public: void x(); }; void z() { Y y; y.x(); }",
@@ -726,7 +726,7 @@ TEST(Matcher, Call) {
                  MethodOnY));
 
   StatementMatcher MethodOnYPointer =
-      call(on(hasType(pointsTo(record(hasName("Y"))))));
+      memberCall(on(hasType(pointsTo(record(hasName("Y"))))));
 
   EXPECT_TRUE(
       matches("class Y { public: void x(); }; void z() { Y *y; y->x(); }",
@@ -748,7 +748,7 @@ TEST(Matcher, Call) {
 TEST(HasType, MatchesAsString) {
   EXPECT_TRUE(
       matches("class Y { public: void x(); }; void z() {Y* y; y->x(); }",
-              call(on(hasType(asString("class Y *"))))));
+              memberCall(on(hasType(asString("class Y *"))))));
   EXPECT_TRUE(matches("class X { void x(int x) {} };",
       method(hasParameter(0, hasType(asString("int"))))));
   EXPECT_TRUE(matches("namespace ns { struct A {}; }  struct B { ns::A a; };",
@@ -798,7 +798,8 @@ TEST(Matcher, HasOperatorNameForOverloadedOperatorCall) {
 }
 
 TEST(Matcher, ThisPointerType) {
-  StatementMatcher MethodOnY = call(thisPointerType(record(hasName("Y"))));
+  StatementMatcher MethodOnY =
+    memberCall(thisPointerType(record(hasName("Y"))));
 
   EXPECT_TRUE(
       matches("class Y { public: void x(); }; void z() { Y y; y.x(); }",
@@ -830,7 +831,7 @@ TEST(Matcher, VariableUsage) {
   StatementMatcher Reference =
       declarationReference(to(
           variable(hasInitializer(
-              call(thisPointerType(record(hasName("Y"))))))));
+              memberCall(thisPointerType(record(hasName("Y"))))))));
 
   EXPECT_TRUE(matches(
       "class Y {"
@@ -854,7 +855,7 @@ TEST(Matcher, VariableUsage) {
 
 TEST(Matcher, CalledVariable) {
   StatementMatcher CallOnVariableY = expression(
-      call(on(declarationReference(to(variable(hasName("y")))))));
+      memberCall(on(declarationReference(to(variable(hasName("y")))))));
 
   EXPECT_TRUE(matches(
       "class Y { public: void x() { Y y; y.x(); } };", CallOnVariableY));
@@ -1739,7 +1740,7 @@ AST_MATCHER_P(Decl, just, internal::Matcher<Decl>, AMatcher) {
 }
 
 TEST(AstMatcherPMacro, Works) {
-  DeclarationMatcher HasClassB = just(has(id("b", record(hasName("B")))));
+  DeclarationMatcher HasClassB = just(has(record(hasName("B")).bind("b")));
 
   EXPECT_TRUE(matchAndVerifyResultTrue("class A { class B {}; };",
       HasClassB, new VerifyIdIsBoundToDecl<Decl>("b")));
@@ -1764,7 +1765,7 @@ AST_POLYMORPHIC_MATCHER_P(
 }
 
 TEST(AstPolymorphicMatcherPMacro, Works) {
-  DeclarationMatcher HasClassB = polymorphicHas(id("b", record(hasName("B"))));
+  DeclarationMatcher HasClassB = polymorphicHas(record(hasName("B")).bind("b"));
 
   EXPECT_TRUE(matchAndVerifyResultTrue("class A { class B {}; };",
       HasClassB, new VerifyIdIsBoundToDecl<Decl>("b")));
@@ -2133,26 +2134,26 @@ TEST(HasConditionVariableStatement, MatchesConditionVariables) {
 
 TEST(ForEach, BindsOneNode) {
   EXPECT_TRUE(matchAndVerifyResultTrue("class C { int x; };",
-      record(hasName("C"), forEach(id("x", field(hasName("x"))))),
+      record(hasName("C"), forEach(field(hasName("x")).bind("x"))),
       new VerifyIdIsBoundToDecl<FieldDecl>("x", 1)));
 }
 
 TEST(ForEach, BindsMultipleNodes) {
   EXPECT_TRUE(matchAndVerifyResultTrue("class C { int x; int y; int z; };",
-      record(hasName("C"), forEach(id("f", field()))),
+      record(hasName("C"), forEach(field().bind("f"))),
       new VerifyIdIsBoundToDecl<FieldDecl>("f", 3)));
 }
 
 TEST(ForEach, BindsRecursiveCombinations) {
   EXPECT_TRUE(matchAndVerifyResultTrue(
       "class C { class D { int x; int y; }; class E { int y; int z; }; };",
-      record(hasName("C"), forEach(record(forEach(id("f", field()))))),
+      record(hasName("C"), forEach(record(forEach(field().bind("f"))))),
       new VerifyIdIsBoundToDecl<FieldDecl>("f", 4)));
 }
 
 TEST(ForEachDescendant, BindsOneNode) {
   EXPECT_TRUE(matchAndVerifyResultTrue("class C { class D { int x; }; };",
-      record(hasName("C"), forEachDescendant(id("x", field(hasName("x"))))),
+      record(hasName("C"), forEachDescendant(field(hasName("x")).bind("x"))),
       new VerifyIdIsBoundToDecl<FieldDecl>("x", 1)));
 }
 
@@ -2160,7 +2161,7 @@ TEST(ForEachDescendant, BindsMultipleNodes) {
   EXPECT_TRUE(matchAndVerifyResultTrue(
       "class C { class D { int x; int y; }; "
       "          class E { class F { int y; int z; }; }; };",
-      record(hasName("C"), forEachDescendant(id("f", field()))),
+      record(hasName("C"), forEachDescendant(field().bind("f"))),
       new VerifyIdIsBoundToDecl<FieldDecl>("f", 4)));
 }
 
@@ -2169,7 +2170,7 @@ TEST(ForEachDescendant, BindsRecursiveCombinations) {
       "class C { class D { "
       "          class E { class F { class G { int y; int z; }; }; }; }; };",
       record(hasName("C"), forEachDescendant(record(
-          forEachDescendant(id("f", field()))))),
+          forEachDescendant(field().bind("f"))))),
       new VerifyIdIsBoundToDecl<FieldDecl>("f", 8)));
 }