Revert r218616, "Refactor Matcher<T> and DynTypedMatcher to reduce overhead of casts."

MSC17, aka VS2012, cannot compile it.

  clang/include/clang/ASTMatchers/ASTMatchersInternal.h(387) : error C4519: default template arguments are only allowed on a class template

  clang/include/clang/ASTMatchers/ASTMatchersInternal.h(443) : see reference to class template instantiation 'clang::ast_matchers::internal::Matcher<T>' being compiled

llvm-svn: 218648

clang/include/clang/AST/ASTTypeTraits.h

@@ -191,8 +191,6 @@ public:
     return BaseConverter<T>::get(NodeKind, Storage.buffer);
   }
 
-  ASTNodeKind getNodeKind() const { return NodeKind; }
-
   /// \brief Returns a pointer that identifies the stored AST node.
   ///
   /// Note that this is not supported by all AST nodes. For AST nodes

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

@@ -61,8 +61,9 @@ public:
   /// \brief Adds \c Node to the map with key \c ID.
   ///
   /// The node's base type should be in NodeBaseType or it will be unaccessible.
-  void addNode(StringRef ID, const ast_type_traits::DynTypedNode& DynNode) {
+  template <typename T>
-    NodeMap[ID] = DynNode;
+  void addNode(StringRef ID, const T* Node) {
+    NodeMap[ID] = ast_type_traits::DynTypedNode::create(*Node);
   }
 
   /// \brief Returns the AST node bound to \c ID.
@@ -135,12 +136,11 @@ public:
   };
 
   /// \brief Add a binding from an id to a node.
-  void setBinding(const std::string &Id,
+  template <typename T> void setBinding(const std::string &Id, const T *Node) {
-                  const ast_type_traits::DynTypedNode &DynNode) {
     if (Bindings.empty())
       Bindings.push_back(BoundNodesMap());
     for (unsigned i = 0, e = Bindings.size(); i != e; ++i)
-      Bindings[i].addNode(Id, DynNode);
+      Bindings[i].addNode(Id, Node);
   }
 
   /// \brief Adds a branch in the tree.
@@ -179,22 +179,6 @@ private:
 
 class ASTMatchFinder;
 
-/// \brief Generic interface for all matchers.
-///
-/// Used by the implementation of Matcher<T> and DynTypedMatcher.
-/// In general, implement MatcherInterface<T> or SingleNodeMatcherInterface<T>
-/// instead.
-class DynMatcherInterface : public RefCountedBaseVPTR {
-public:
-  /// \brief Returns true if \p DynNode can be matched.
-  ///
-  /// May bind \p DynNode to an ID via \p Builder, or recurse into
-  /// the AST via \p Finder.
- virtual bool matches(const ast_type_traits::DynTypedNode &DynNode,
-                      ASTMatchFinder *Finder,
-                      BoundNodesTreeBuilder *Builder) const = 0;
-};
-
 /// \brief Generic interface for matchers on an AST node of type T.
 ///
 /// Implement this if your matcher may need to inspect the children or
@@ -203,7 +187,7 @@ public:
 /// current node and doesn't care about its children or descendants,
 /// implement SingleNodeMatcherInterface instead.
 template <typename T>
-class MatcherInterface : public DynMatcherInterface {
+class MatcherInterface : public RefCountedBaseVPTR {
 public:
   virtual ~MatcherInterface() {}
 
@@ -214,15 +198,6 @@ public:
   virtual bool matches(const T &Node,
                        ASTMatchFinder *Finder,
                        BoundNodesTreeBuilder *Builder) const = 0;
-
-  bool matches(const ast_type_traits::DynTypedNode &DynNode,
-               ASTMatchFinder *Finder,
-               BoundNodesTreeBuilder *Builder) const override {
-    if (const T *Node = DynNode.get<T>()) {
-      return matches(*Node, Finder, Builder);
-    }
-    return false;
-  }
 };
 
 /// \brief Interface for matchers that only evaluate properties on a single
@@ -244,7 +219,111 @@ private:
   }
 };
 
-template <typename> class Matcher;
+/// \brief Wrapper of a MatcherInterface<T> *that allows copying.
+///
+/// A Matcher<Base> can be used anywhere a Matcher<Derived> is
+/// required. This establishes an is-a relationship which is reverse
+/// to the AST hierarchy. In other words, Matcher<T> is contravariant
+/// with respect to T. The relationship is built via a type conversion
+/// operator rather than a type hierarchy to be able to templatize the
+/// type hierarchy instead of spelling it out.
+template <typename T>
+class Matcher {
+public:
+  /// \brief Takes ownership of the provided implementation pointer.
+  explicit Matcher(MatcherInterface<T> *Implementation)
+      : Implementation(Implementation) {}
+
+  /// \brief Implicitly converts \c Other to a Matcher<T>.
+  ///
+  /// Requires \c T to be derived from \c From.
+  template <typename From>
+  Matcher(const Matcher<From> &Other,
+          typename std::enable_if<std::is_base_of<From, T>::value &&
+                                  !std::is_same<From, T>::value>::type * = 0)
+      : Implementation(new ImplicitCastMatcher<From>(Other)) {}
+
+  /// \brief Implicitly converts \c Matcher<Type> to \c Matcher<QualType>.
+  ///
+  /// The resulting matcher is not strict, i.e. ignores qualifiers.
+  template <typename TypeT>
+  Matcher(const Matcher<TypeT> &Other,
+          typename std::enable_if<
+            std::is_same<T, QualType>::value &&
+            std::is_same<TypeT, Type>::value>::type* = 0)
+      : Implementation(new TypeToQualType<TypeT>(Other)) {}
+
+  /// \brief Forwards the call to the underlying MatcherInterface<T> pointer.
+  bool matches(const T &Node,
+               ASTMatchFinder *Finder,
+               BoundNodesTreeBuilder *Builder) const {
+    if (Implementation->matches(Node, Finder, Builder))
+      return true;
+    // Delete all bindings when a matcher does not match.
+    // This prevents unexpected exposure of bound nodes in unmatches
+    // branches of the match tree.
+    Builder->removeBindings([](const BoundNodesMap &) { return true; });
+    return false;
+  }
+
+  /// \brief Returns an ID that uniquely identifies the matcher.
+  uint64_t getID() const {
+    /// FIXME: Document the requirements this imposes on matcher
+    /// implementations (no new() implementation_ during a Matches()).
+    return reinterpret_cast<uint64_t>(Implementation.get());
+  }
+
+  /// \brief Allows the conversion of a \c Matcher<Type> to a \c
+  /// Matcher<QualType>.
+  ///
+  /// Depending on the constructor argument, the matcher is either strict, i.e.
+  /// does only matches in the absence of qualifiers, or not, i.e. simply
+  /// ignores any qualifiers.
+  template <typename TypeT>
+  class TypeToQualType : public MatcherInterface<QualType> {
+   public:
+    TypeToQualType(const Matcher<TypeT> &InnerMatcher)
+        : InnerMatcher(InnerMatcher) {}
+
+    bool matches(const QualType &Node, ASTMatchFinder *Finder,
+                 BoundNodesTreeBuilder *Builder) const override {
+      if (Node.isNull())
+        return false;
+      return InnerMatcher.matches(*Node, Finder, Builder);
+    }
+   private:
+    const Matcher<TypeT> InnerMatcher;
+  };
+
+private:
+  /// \brief Allows conversion from Matcher<Base> to Matcher<T> if T
+  /// is derived from Base.
+  template <typename Base>
+  class ImplicitCastMatcher : public MatcherInterface<T> {
+  public:
+    explicit ImplicitCastMatcher(const Matcher<Base> &From)
+        : From(From) {}
+
+    bool matches(const T &Node, ASTMatchFinder *Finder,
+                 BoundNodesTreeBuilder *Builder) const override {
+      return From.matches(Node, Finder, Builder);
+    }
+
+  private:
+    const Matcher<Base> From;
+  };
+
+  IntrusiveRefCntPtr< MatcherInterface<T> > Implementation;
+};  // class Matcher
+
+/// \brief A convenient helper for creating a Matcher<T> without specifying
+/// the template type argument.
+template <typename T>
+inline Matcher<T> makeMatcher(MatcherInterface<T> *Implementation) {
+  return Matcher<T>(Implementation);
+}
+
+template <typename T> class BindableMatcher;
 
 /// \brief Matcher that works on a \c DynTypedNode.
 ///
@@ -255,12 +334,13 @@ template <typename> class Matcher;
 /// return false if it is not convertible.
 class DynTypedMatcher {
 public:
-  /// \brief Takes ownership of the provided implementation pointer.
+  /// \brief Construct from a \c Matcher<T>. Copies the matcher.
-  template <typename T>
+  template <typename T> inline DynTypedMatcher(const Matcher<T> &M);
-  DynTypedMatcher(MatcherInterface<T> *Implementation)
+
-      : AllowBind(false),
+  /// \brief Construct from a bindable \c Matcher<T>. Copies the matcher.
-        SupportedKind(ast_type_traits::ASTNodeKind::getFromNodeKind<T>()),
+  ///
-        RestrictKind(SupportedKind), Implementation(Implementation) {}
+  /// This version enables \c tryBind() on the \c DynTypedMatcher.
+  template <typename T> inline DynTypedMatcher(const BindableMatcher<T> &M);
 
   /// \brief Construct from a variadic function.
   typedef bool (*VariadicOperatorFunction)(
@@ -268,44 +348,33 @@ public:
       BoundNodesTreeBuilder *Builder, ArrayRef<DynTypedMatcher> InnerMatchers);
   static DynTypedMatcher
   constructVariadic(VariadicOperatorFunction Func,
-                    std::vector<DynTypedMatcher> InnerMatchers);
+                    ArrayRef<DynTypedMatcher> InnerMatchers) {
-
+    assert(InnerMatchers.size() > 0 && "Array must not be empty.");
-  void setAllowBind(bool AB) { AllowBind = AB; }
+    return DynTypedMatcher(new VariadicStorage(Func, InnerMatchers));
-
+  }
-  /// \brief Return a matcher that points to the same implementation, but
-  ///   restricts the node types for \p Kind.
-  DynTypedMatcher dynCastTo(const ast_type_traits::ASTNodeKind Kind) const;
 
   /// \brief Returns true if the matcher matches the given \c DynNode.
   bool matches(const ast_type_traits::DynTypedNode DynNode,
-               ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const;
+               ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const {
+    return Storage->matches(DynNode, Finder, Builder);
+  }
 
   /// \brief Bind the specified \p ID to the matcher.
   /// \return A new matcher with the \p ID bound to it if this matcher supports
   ///   binding. Otherwise, returns an empty \c Optional<>.
-  llvm::Optional<DynTypedMatcher> tryBind(StringRef ID) const;
+  llvm::Optional<DynTypedMatcher> tryBind(StringRef ID) const {
+    return Storage->tryBind(ID);
+  }
 
   /// \brief Returns a unique \p ID for the matcher.
-  ///
+  uint64_t getID() const { return Storage->getID(); }
-  /// Casting a Matcher<T> to Matcher<U> creates a matcher that has the
-  /// same \c Implementation pointer, but different \c RestrictKind. We need to
-  /// include both in the ID to make it unique.
-  ///
-  /// \c MatcherIDType supports operator< and provides strict weak ordering.
-  typedef std::pair<ast_type_traits::ASTNodeKind, uint64_t> MatcherIDType;
-  MatcherIDType getID() const {
-    /// FIXME: Document the requirements this imposes on matcher
-    /// implementations (no new() implementation_ during a Matches()).
-    return std::make_pair(RestrictKind,
-                          reinterpret_cast<uint64_t>(Implementation.get()));
-  }
 
   /// \brief Returns the type this matcher works on.
   ///
   /// \c matches() will always return false unless the node passed is of this
   /// or a derived type.
   ast_type_traits::ASTNodeKind getSupportedKind() const {
-    return SupportedKind;
+    return Storage->getSupportedKind();
   }
 
   /// \brief Returns \c true if the passed \c DynTypedMatcher can be converted
@@ -335,119 +404,96 @@ public:
   template <typename T> Matcher<T> unconditionalConvertTo() const;
 
 private:
-  bool AllowBind;
+  class MatcherStorage : public RefCountedBaseVPTR {
-  ast_type_traits::ASTNodeKind SupportedKind;
+  public:
-  /// \brief A potentially stricter node kind.
+    MatcherStorage(ast_type_traits::ASTNodeKind SupportedKind, uint64_t ID)
-  ///
+        : SupportedKind(SupportedKind), ID(ID) {}
-  /// It allows to perform implicit and dynamic cast of matchers without
+    virtual ~MatcherStorage();
-  /// needing to change \c Implementation.
-  ast_type_traits::ASTNodeKind RestrictKind;
-  IntrusiveRefCntPtr<DynMatcherInterface> Implementation;
-};
 
-/// \brief Wrapper of a MatcherInterface<T> *that allows copying.
+    virtual bool matches(const ast_type_traits::DynTypedNode DynNode,
-///
+                         ASTMatchFinder *Finder,
-/// A Matcher<Base> can be used anywhere a Matcher<Derived> is
+                         BoundNodesTreeBuilder *Builder) const = 0;
-/// required. This establishes an is-a relationship which is reverse
-/// to the AST hierarchy. In other words, Matcher<T> is contravariant
-/// with respect to T. The relationship is built via a type conversion
-/// operator rather than a type hierarchy to be able to templatize the
-/// type hierarchy instead of spelling it out.
-template <typename T>
-class Matcher {
-public:
-  /// \brief Takes ownership of the provided implementation pointer.
-  explicit Matcher(MatcherInterface<T> *Implementation)
-      : Implementation(Implementation) {}
 
-  /// \brief Implicitly converts \c Other to a Matcher<T>.
+    virtual llvm::Optional<DynTypedMatcher> tryBind(StringRef ID) const = 0;
-  ///
-  /// Requires \c T to be derived from \c From.
-  template <typename From>
-  Matcher(const Matcher<From> &Other,
-          typename std::enable_if<std::is_base_of<From, T>::value &&
-                                  !std::is_same<From, T>::value>::type * = 0)
-      : Matcher(Other.Implementation) {}
 
-  /// \brief Implicitly converts \c Matcher<Type> to \c Matcher<QualType>.
+    ast_type_traits::ASTNodeKind getSupportedKind() const {
-  ///
+      return SupportedKind;
-  /// The resulting matcher is not strict, i.e. ignores qualifiers.
-  template <typename TypeT>
-  Matcher(const Matcher<TypeT> &Other,
-          typename std::enable_if<
-            std::is_same<T, QualType>::value &&
-            std::is_same<TypeT, Type>::value>::type* = 0)
-      : Implementation(new TypeToQualType<TypeT>(Other)) {}
-
-  /// \brief Convert \c this into a \c Matcher<T> by applying dyn_cast<> to the
-  /// argument.
-  /// \c To must be a base class of \c T.
-  template <typename To, typename std::enable_if<
-                             std::is_base_of<To, T>::value>::type * = nullptr>
-  Matcher<To> dynCastTo() const {
-    return Matcher<To>(Implementation);
-  }
-
-  /// \brief Forwards the call to the underlying MatcherInterface<T> pointer.
-  bool matches(const T &Node,
-               ASTMatchFinder *Finder,
-               BoundNodesTreeBuilder *Builder) const {
-    return Implementation.matches(ast_type_traits::DynTypedNode::create(Node),
-                                  Finder, Builder);
-  }
-
-  /// \brief Returns an ID that uniquely identifies the matcher.
-  DynTypedMatcher::MatcherIDType getID() const {
-    return Implementation.getID();
-  }
-
-  /// \brief Extract the dynamic matcher.
-  ///
-  /// The returned matcher keeps the same restrictions as \c this and remembers
-  /// that it is meant to support nodes of type \c T.
-  operator DynTypedMatcher() const { return Implementation; }
-
-  /// \brief Allows the conversion of a \c Matcher<Type> to a \c
-  /// Matcher<QualType>.
-  ///
-  /// Depending on the constructor argument, the matcher is either strict, i.e.
-  /// does only matches in the absence of qualifiers, or not, i.e. simply
-  /// ignores any qualifiers.
-  template <typename TypeT>
-  class TypeToQualType : public MatcherInterface<QualType> {
-   public:
-    TypeToQualType(const Matcher<TypeT> &InnerMatcher)
-        : InnerMatcher(InnerMatcher) {}
-
-    bool matches(const QualType &Node, ASTMatchFinder *Finder,
-                 BoundNodesTreeBuilder *Builder) const override {
-      if (Node.isNull())
-        return false;
-      return InnerMatcher.matches(*Node, Finder, Builder);
     }
-   private:
+
-    const Matcher<TypeT> InnerMatcher;
+    uint64_t getID() const { return ID; }
+
+  private:
+    const ast_type_traits::ASTNodeKind SupportedKind;
+    const uint64_t ID;
   };
 
-private:
+  class VariadicStorage : public MatcherStorage {
-  template <typename U> friend class Matcher;
+  public:
+    VariadicStorage(VariadicOperatorFunction Func,
+                    ArrayRef<DynTypedMatcher> InnerMatchers)
+        : MatcherStorage(InnerMatchers[0].getSupportedKind(),
+                         reinterpret_cast<uint64_t>(this)),
+          Func(Func), InnerMatchers(InnerMatchers) {}
 
-  explicit Matcher(const DynTypedMatcher &Implementation)
+    bool matches(const ast_type_traits::DynTypedNode DynNode,
-      : Implementation(Implementation.dynCastTo(
+                 ASTMatchFinder *Finder,
-            ast_type_traits::ASTNodeKind::getFromNodeKind<T>())) {
+                 BoundNodesTreeBuilder *Builder) const override {
-    assert(this->Implementation.getSupportedKind()
+      return Func(DynNode, Finder, Builder, InnerMatchers);
-               .isSame(ast_type_traits::ASTNodeKind::getFromNodeKind<T>()));
+    }
+
+    llvm::Optional<DynTypedMatcher> tryBind(StringRef ID) const override {
+      return llvm::None;
+    }
+
+  private:
+    VariadicOperatorFunction Func;
+    std::vector<DynTypedMatcher> InnerMatchers;
+  };
+
+  /// \brief Typed implementation of \c MatcherStorage.
+  template <typename T> class TypedMatcherStorage;
+
+  /// \brief Internal constructor for \c constructVariadic.
+  DynTypedMatcher(MatcherStorage *Storage) : Storage(Storage) {}
+
+  IntrusiveRefCntPtr<const MatcherStorage> Storage;
+};
+
+template <typename T>
+class DynTypedMatcher::TypedMatcherStorage : public MatcherStorage {
+public:
+  TypedMatcherStorage(const Matcher<T> &Other, bool AllowBind)
+      : MatcherStorage(ast_type_traits::ASTNodeKind::getFromNodeKind<T>(),
+                       Other.getID()),
+        InnerMatcher(Other), AllowBind(AllowBind) {}
+
+  bool matches(const ast_type_traits::DynTypedNode DynNode,
+               ASTMatchFinder *Finder,
+               BoundNodesTreeBuilder *Builder) const override {
+    if (const T *Node = DynNode.get<T>()) {
+      return InnerMatcher.matches(*Node, Finder, Builder);
+    }
+    return false;
   }
 
-  DynTypedMatcher Implementation;
+  llvm::Optional<DynTypedMatcher> tryBind(StringRef ID) const override {
-};  // class Matcher
+    if (!AllowBind)
+      return llvm::Optional<DynTypedMatcher>();
+    return DynTypedMatcher(BindableMatcher<T>(InnerMatcher).bind(ID));
+  }
+
+private:
+  const Matcher<T> InnerMatcher;
+  const bool AllowBind;
+};
 
-/// \brief A convenient helper for creating a Matcher<T> without specifying
-/// the template type argument.
 template <typename T>
-inline Matcher<T> makeMatcher(MatcherInterface<T> *Implementation) {
+inline DynTypedMatcher::DynTypedMatcher(const Matcher<T> &M)
-  return Matcher<T>(Implementation);
+    : Storage(new TypedMatcherStorage<T>(M, false)) {}
-}
+
+template <typename T>
+inline DynTypedMatcher::DynTypedMatcher(const BindableMatcher<T> &M)
+    : Storage(new TypedMatcherStorage<T>(M, true)) {}
 
 /// \brief Specialization of the conversion functions for QualType.
 ///
@@ -1008,7 +1054,7 @@ public:
                BoundNodesTreeBuilder *Builder) const override {
     bool Result = InnerMatcher.matches(Node, Finder, Builder);
     if (Result) {
-      Builder->setBinding(ID, ast_type_traits::DynTypedNode::create(Node));
+      Builder->setBinding(ID, &Node);
     }
     return Result;
   }
@@ -1034,18 +1080,8 @@ public:
   /// The returned matcher is equivalent to this matcher, but will
   /// bind the matched node on a match.
   Matcher<T> bind(StringRef ID) const {
-    // FIXME: Use DynTypedMatcher's IdMatcher instead. No need for a template
-    // version anymore.
     return Matcher<T>(new IdMatcher<T>(ID, *this));
   }
-
-  /// \brief Same as Matcher<T>'s conversion operator, but enables binding on
-  /// the returned matcher.
-  operator DynTypedMatcher() const {
-    DynTypedMatcher Result = static_cast<const Matcher<T>&>(*this);
-    Result.setAllowBind(true);
-    return Result;
-  }
 };
 
 /// \brief Matches nodes of type T that have child nodes of type ChildT for
@@ -1170,7 +1206,6 @@ public:
     addMatcher<T>(Param7, Matchers);
     addMatcher<T>(Param8, Matchers);
     addMatcher<T>(Param9, Matchers);
-    // FIXME: Use DynTypedMatcher::constructVariadic() instead.
     return Matcher<T>(
         new VariadicOperatorMatcherInterface<T>(Func, std::move(Matchers)));
   }
@@ -1309,7 +1344,6 @@ bool AnyOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode,
 
 template <typename T>
 inline Matcher<T> DynTypedMatcher::unconditionalConvertTo() const {
-  // FIXME: Remove this extra indirection and connect directly to Matcher<T>().
   return Matcher<T>(new VariadicOperatorMatcherInterface<T>(
       AllOfVariadicOperator, llvm::makeArrayRef(*this)));
 }
@@ -1318,12 +1352,10 @@ inline Matcher<T> DynTypedMatcher::unconditionalConvertTo() const {
 template<typename T>
 BindableMatcher<T> makeAllOfComposite(
     ArrayRef<const Matcher<T> *> InnerMatchers) {
-  // FIXME: Optimize for the cases of size()==0 and size()==1
   std::vector<DynTypedMatcher> DynMatchers;
   for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) {
     DynMatchers.push_back(*InnerMatchers[i]);
   }
-  // FIXME: Use DynTypedMatcher::constructVariadic() instead.
   return BindableMatcher<T>(new VariadicOperatorMatcherInterface<T>(
       AllOfVariadicOperator, std::move(DynMatchers)));
 }
@@ -1337,8 +1369,8 @@ BindableMatcher<T> makeAllOfComposite(
 template<typename T, typename InnerT>
 BindableMatcher<T> makeDynCastAllOfComposite(
     ArrayRef<const Matcher<InnerT> *> InnerMatchers) {
-  return BindableMatcher<T>(
+  return BindableMatcher<T>(DynTypedMatcher(makeAllOfComposite(InnerMatchers))
-      makeAllOfComposite(InnerMatchers).template dynCastTo<T>());
+                                .unconditionalConvertTo<T>());
 }
 
 /// \brief Matches nodes of type T that have at least one descendant node of

clang/lib/ASTMatchers/ASTMatchFinder.cpp

@@ -53,7 +53,7 @@ static const unsigned MaxMemoizationEntries = 10000;
 // FIXME: Benchmark whether memoization of non-pointer typed nodes
 // provides enough benefit for the additional amount of code.
 struct MatchKey {
-  DynTypedMatcher::MatcherIDType MatcherID;
+  uint64_t MatcherID;
   ast_type_traits::DynTypedNode Node;
   BoundNodesTreeBuilder BoundNodes;
 

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

@@ -26,113 +26,6 @@ void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
   }
 }
 
-namespace {
-
-class VariadicMatcher : public DynMatcherInterface {
- public:
-  VariadicMatcher(VariadicOperatorFunction Func,
-                  std::vector<DynTypedMatcher> InnerMatchers)
-      : Func(Func), InnerMatchers(std::move(InnerMatchers)) {}
-
-  bool matches(const ast_type_traits::DynTypedNode &DynNode,
-               ASTMatchFinder *Finder,
-               BoundNodesTreeBuilder *Builder) const override {
-    return Func(DynNode, Finder, Builder, InnerMatchers);
-  }
-
- private:
-  VariadicOperatorFunction Func;
-  std::vector<DynTypedMatcher> InnerMatchers;
-};
-
-class IdDynMatcher : public DynMatcherInterface {
- public:
-  IdDynMatcher(StringRef ID,
-               const IntrusiveRefCntPtr<DynMatcherInterface> &InnerMatcher)
-      : ID(ID), InnerMatcher(InnerMatcher) {}
-
-  bool matches(const ast_type_traits::DynTypedNode &DynNode,
-               ASTMatchFinder *Finder,
-               BoundNodesTreeBuilder *Builder) const override {
-    bool Result = InnerMatcher->matches(DynNode, Finder, Builder);
-    if (Result) Builder->setBinding(ID, DynNode);
-    return Result;
-  }
-
- private:
-  const std::string ID;
-  const IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher;
-};
-
-/// \brief Return the most derived type between \p Kind1 and \p Kind2.
-///
-/// Return the null type if they are not related.
-ast_type_traits::ASTNodeKind getMostDerivedType(
-    const ast_type_traits::ASTNodeKind Kind1,
-    const ast_type_traits::ASTNodeKind Kind2) {
-  if (Kind1.isBaseOf(Kind2)) return Kind2;
-  if (Kind2.isBaseOf(Kind1)) return Kind1;
-  return ast_type_traits::ASTNodeKind();
-}
-
-/// \brief Return the least derived type between \p Kind1 and \p Kind2.
-///
-/// Return the null type if they are not related.
-static ast_type_traits::ASTNodeKind getLeastDerivedType(
-    const ast_type_traits::ASTNodeKind Kind1,
-    const ast_type_traits::ASTNodeKind Kind2) {
-  if (Kind1.isBaseOf(Kind2)) return Kind1;
-  if (Kind2.isBaseOf(Kind1)) return Kind2;
-  return ast_type_traits::ASTNodeKind();
-}
-
-}  // namespace
-
-DynTypedMatcher DynTypedMatcher::constructVariadic(
-    VariadicOperatorFunction Func, std::vector<DynTypedMatcher> InnerMatchers) {
-  assert(InnerMatchers.size() > 0 && "Array must not be empty.");
-  DynTypedMatcher Result = InnerMatchers[0];
-  // Use the least derived type as the restriction for the wrapper.
-  // This allows mismatches to be resolved on the inner matchers.
-  for (const DynTypedMatcher &M : InnerMatchers) {
-    assert(Result.SupportedKind.isSame(M.SupportedKind) &&
-           "SupportedKind must match!");
-    Result.RestrictKind =
-        getLeastDerivedType(Result.RestrictKind, M.RestrictKind);
-  }
-  Result.Implementation = new VariadicMatcher(Func, std::move(InnerMatchers));
-  return Result;
-}
-
-DynTypedMatcher DynTypedMatcher::dynCastTo(
-    const ast_type_traits::ASTNodeKind Kind) const {
-  auto Copy = *this;
-  Copy.SupportedKind = Kind;
-  Copy.RestrictKind = getMostDerivedType(Kind, RestrictKind);
-  return Copy;
-}
-
-bool DynTypedMatcher::matches(const ast_type_traits::DynTypedNode DynNode,
-                              ASTMatchFinder *Finder,
-                              BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-      Implementation->matches(DynNode, Finder, Builder)) {
-    return true;
-  }
-  // Delete all bindings when a matcher does not match.
-  // This prevents unexpected exposure of bound nodes in unmatches
-  // branches of the match tree.
-  Builder->removeBindings([](const BoundNodesMap &) { return true; });
-  return false;
-}
-
-llvm::Optional<DynTypedMatcher> DynTypedMatcher::tryBind(StringRef ID) const {
-  if (!AllowBind) return llvm::None;
-  auto Result = *this;
-  Result.Implementation = new IdDynMatcher(ID, Result.Implementation);
-  return Result;
-}
-
 bool DynTypedMatcher::canConvertTo(ast_type_traits::ASTNodeKind To) const {
   const auto From = getSupportedKind();
   auto QualKind = ast_type_traits::ASTNodeKind::getFromNodeKind<QualType>();
@@ -144,6 +37,8 @@ bool DynTypedMatcher::canConvertTo(ast_type_traits::ASTNodeKind To) const {
   return From.isBaseOf(To);
 }
 
+DynTypedMatcher::MatcherStorage::~MatcherStorage() {}
+
 void BoundNodesTreeBuilder::addMatch(const BoundNodesTreeBuilder &Other) {
   for (unsigned i = 0, e = Other.Bindings.size(); i != e; ++i) {
     Bindings.push_back(Other.Bindings[i]);

clang/unittests/ASTMatchers/ASTMatchersTest.cpp

@@ -654,20 +654,6 @@ TEST(DeclarationMatcher, HasDescendantMemoization) {
   EXPECT_TRUE(matches("void f() { int i; }", CannotMemoize));
 }
 
-TEST(DeclarationMatcher, HasDescendantMemoizationUsesRestrictKind) {
-  auto Name = hasName("i");
-  auto VD = internal::Matcher<VarDecl>(Name).dynCastTo<Decl>();
-  auto RD = internal::Matcher<RecordDecl>(Name).dynCastTo<Decl>();
-  // Matching VD first should not make a cache hit for RD.
-  EXPECT_TRUE(notMatches("void f() { int i; }",
-                         decl(hasDescendant(VD), hasDescendant(RD))));
-  EXPECT_TRUE(notMatches("void f() { int i; }",
-                         decl(hasDescendant(RD), hasDescendant(VD))));
-  // Not matching RD first should not make a cache hit for VD either.
-  EXPECT_TRUE(matches("void f() { int i; }",
-                      decl(anyOf(hasDescendant(RD), hasDescendant(VD)))));
-}
-
 TEST(DeclarationMatcher, HasAttr) {
   EXPECT_TRUE(matches("struct __attribute__((warn_unused)) X {};",
                       decl(hasAttr(clang::attr::WarnUnused))));

clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp

@@ -28,7 +28,7 @@ public:
   uint64_t expectMatcher(StringRef MatcherName) {
     ast_matchers::internal::Matcher<Stmt> M = stmt();
     ExpectedMatchers.insert(std::make_pair(MatcherName, M));
-    return M.getID().second;
+    return M.getID();
   }
 
   void parse(StringRef Code) {
@@ -125,12 +125,8 @@ TEST(ParserTest, ParseMatcher) {
     EXPECT_EQ("", Sema.Errors[i]);
   }
 
-  EXPECT_NE(ExpectedFoo, ExpectedBar);
-  EXPECT_NE(ExpectedFoo, ExpectedBaz);
-  EXPECT_NE(ExpectedBar, ExpectedBaz);
-
   EXPECT_EQ(1ULL, Sema.Values.size());
-  EXPECT_EQ(ExpectedFoo, getSingleMatcher(Sema.Values[0])->getID().second);
+  EXPECT_EQ(ExpectedFoo, getSingleMatcher(Sema.Values[0])->getID());
 
   EXPECT_EQ(3ULL, Sema.Matchers.size());
   const MockSema::MatcherInfo Bar = Sema.Matchers[0];
@@ -149,8 +145,8 @@ TEST(ParserTest, ParseMatcher) {
   EXPECT_EQ("Foo", Foo.MatcherName);
   EXPECT_TRUE(matchesRange(Foo.NameRange, 1, 2, 2, 12));
   EXPECT_EQ(2ULL, Foo.Args.size());
-  EXPECT_EQ(ExpectedBar, getSingleMatcher(Foo.Args[0].Value)->getID().second);
+  EXPECT_EQ(ExpectedBar, getSingleMatcher(Foo.Args[0].Value)->getID());
-  EXPECT_EQ(ExpectedBaz, getSingleMatcher(Foo.Args[1].Value)->getID().second);
+  EXPECT_EQ(ExpectedBaz, getSingleMatcher(Foo.Args[1].Value)->getID());
   EXPECT_EQ("Yo!", Foo.BoundID);
 }