Remove VariadicOperatorMatcherInterface as it is redundant with logic from DynTypedMatcher.

[sbenza]

Summary

The generic variadic matcher is faster (one less virtual function call
per match) and doesn't require template instantiations which reduces
compile time and binary size.
Registry.cpp.o generates ~14% less symbols and compiles ~7.5% faster.
The change also speeds up our clang-tidy benchmark by ~2%.

[klimek]

Nice! LG

[sbenza]

Closed by commit rL222131 (authored by @sbenza).

include/clang/ASTMatchers/ASTMatchersInternal.h

 
 private:
   const Matcher<ChildT> ChildMatcher;
 };
 
 /// \brief VariadicOperatorMatcher related types.
 /// @{
 
-/// \brief Function signature for any variadic operator. It takes the inner
-///   matchers as an array of DynTypedMatcher.
-typedef bool (*VariadicOperatorFunction)(
-    const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder,
-    BoundNodesTreeBuilder *Builder, ArrayRef<DynTypedMatcher> InnerMatchers);
-
-/// \brief \c MatcherInterface<T> implementation for an variadic operator.
-template <typename T>
-class VariadicOperatorMatcherInterface : public MatcherInterface<T> {
-public:
-  VariadicOperatorMatcherInterface(VariadicOperatorFunction Func,
-                                   std::vector<DynTypedMatcher> InnerMatchers)
-      : Func(Func), InnerMatchers(std::move(InnerMatchers)) {}
-
-  bool matches(const T &Node, ASTMatchFinder *Finder,
-               BoundNodesTreeBuilder *Builder) const override {
-    return Func(ast_type_traits::DynTypedNode::create(Node), Finder, Builder,
-                InnerMatchers);
-  }
-
-private:
-  const VariadicOperatorFunction Func;
-  const std::vector<DynTypedMatcher> InnerMatchers;
-};
-
 /// \brief "No argument" placeholder to use as template paratemers.
 struct VariadicOperatorNoArg {};
 
-/// \brief Polymorphic matcher object that uses a \c VariadicOperatorFunction
-///   operator.
+/// \brief Polymorphic matcher object that uses a \c
+/// DynTypedMatcher::VariadicOperatorFunction operator.
 ///
 /// Input matchers can have any type (including other polymorphic matcher
 /// types), and the actual Matcher<T> is generated on demand with an implicit
 /// coversion operator.
 template <typename P1, typename P2 = VariadicOperatorNoArg,
           typename P3 = VariadicOperatorNoArg,
           typename P4 = VariadicOperatorNoArg,
           typename P5 = VariadicOperatorNoArg,
           typename P6 = VariadicOperatorNoArg,
           typename P7 = VariadicOperatorNoArg,
           typename P8 = VariadicOperatorNoArg,
           typename P9 = VariadicOperatorNoArg>
 class VariadicOperatorMatcher {
 public:
-  VariadicOperatorMatcher(VariadicOperatorFunction Func, const P1 &Param1,
+  VariadicOperatorMatcher(DynTypedMatcher::VariadicOperatorFunction Func,
+                          const P1 &Param1,
                           const P2 &Param2 = VariadicOperatorNoArg(),
                           const P3 &Param3 = VariadicOperatorNoArg(),
                           const P4 &Param4 = VariadicOperatorNoArg(),
                           const P5 &Param5 = VariadicOperatorNoArg(),
                           const P6 &Param6 = VariadicOperatorNoArg(),
                           const P7 &Param7 = VariadicOperatorNoArg(),
                           const P8 &Param8 = VariadicOperatorNoArg(),
                           const P9 &Param9 = VariadicOperatorNoArg())
       : Func(Func), Param1(Param1), Param2(Param2), Param3(Param3),
         Param4(Param4), Param5(Param5), Param6(Param6), Param7(Param7),
         Param8(Param8), Param9(Param9) {}
 
   template <typename T> operator Matcher<T>() const {
     std::vector<DynTypedMatcher> Matchers;
     addMatcher<T>(Param1, Matchers);
     addMatcher<T>(Param2, Matchers);
     addMatcher<T>(Param3, Matchers);
     addMatcher<T>(Param4, Matchers);
     addMatcher<T>(Param5, Matchers);
     addMatcher<T>(Param6, Matchers);
     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)));
+    return DynTypedMatcher::constructVariadic(Func, std::move(Matchers))
+        .template unconditionalConvertTo<T>();
   }
 
 private:
   template <typename T>
   static void addMatcher(const Matcher<T> &M,
                          std::vector<DynTypedMatcher> &Matchers) {
     Matchers.push_back(M);
   }
 
   /// \brief Overload to ignore \c VariadicOperatorNoArg arguments.
   template <typename T>
   static void addMatcher(VariadicOperatorNoArg,
                          std::vector<DynTypedMatcher> &Matchers) {}
 
-  const VariadicOperatorFunction Func;
+  const DynTypedMatcher::VariadicOperatorFunction Func;
   const P1 Param1;
   const P2 Param2;
   const P3 Param3;
   const P4 Param4;
   const P5 Param5;
   const P6 Param6;
   const P7 Param7;
   const P8 Param8;
   const P9 Param9;
 };
 
 /// \brief Overloaded function object to generate VariadicOperatorMatcher
 ///   objects from arbitrary matchers.
 ///
 /// It supports 1-9 argument overloaded operator(). More can be added if needed.
 template <unsigned MinCount, unsigned MaxCount>
 struct VariadicOperatorMatcherFunc {
-  VariadicOperatorFunction Func;
+  DynTypedMatcher::VariadicOperatorFunction Func;
 
   template <unsigned Count, typename T>
   struct EnableIfValidArity
       : public std::enable_if<MinCount <= Count && Count <= MaxCount, T> {};
 
   template <typename M1>
   typename EnableIfValidArity<1, VariadicOperatorMatcher<M1> >::type
   operator()(const M1 &P1) const {
if (InnerMatchers.size() == 1) {
     return BindableMatcher<T>(*InnerMatchers[0]);
   }
 
   std::vector<DynTypedMatcher> DynMatchers;
   DynMatchers.reserve(InnerMatchers.size());
   for (const auto *InnerMatcher : InnerMatchers) {
     DynMatchers.push_back(*InnerMatcher);
   }
-  // FIXME: Use DynTypedMatcher::constructVariadic() instead.
-  return BindableMatcher<T>(new VariadicOperatorMatcherInterface<T>(
-      AllOfVariadicOperator, std::move(DynMatchers)));
+  return BindableMatcher<T>(DynTypedMatcher::constructVariadic(
+                                AllOfVariadicOperator, std::move(DynMatchers))
+                                .template unconditionalConvertTo<T>());
 }
 
 /// \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.

include/clang/ASTMatchers/Dynamic/VariantValue.h

public:
     /// \brief Convert \p Matcher the destination type and return it as a new
     /// DynTypedMatcher.
     virtual DynTypedMatcher
     convertMatcher(const DynTypedMatcher &Matcher) const = 0;
 
     /// \brief Constructs a variadic typed matcher from \p InnerMatchers.
     /// Will try to convert each inner matcher to the destination type and
     /// return llvm::None if it fails to do so.
-    llvm::Optional<DynTypedMatcher> constructVariadicOperator(
-        ast_matchers::internal::VariadicOperatorFunction Func,
+    llvm::Optional<DynTypedMatcher>
+    constructVariadicOperator(DynTypedMatcher::VariadicOperatorFunction Func,
                               ArrayRef<VariantMatcher> InnerMatchers) const;
 
   protected:
     ~MatcherOps() {}
 
   private:
     ast_type_traits::ASTNodeKind NodeKind;
   };
 
public:
   ///
   /// They should be the result of a polymorphic matcher.
   static VariantMatcher
   PolymorphicMatcher(std::vector<DynTypedMatcher> Matchers);
 
   /// \brief Creates a 'variadic' operator matcher.
   ///
   /// It will bind to the appropriate type on getTypedMatcher<T>().
-  static VariantMatcher VariadicOperatorMatcher(
-      ast_matchers::internal::VariadicOperatorFunction Func,
+  static VariantMatcher
+  VariadicOperatorMatcher(DynTypedMatcher::VariadicOperatorFunction Func,
                           std::vector<VariantMatcher> Args);
 
   /// \brief Makes the matcher the "null" matcher.
   void reset();
 
   /// \brief Whether the matcher is null.
   bool isNull() const { return !Value; }
 
   /// \brief Return a single matcher, if there is no ambiguity.

lib/ASTMatchers/ASTMatchersInternal.cpp

for (BoundNodesMap &Binding : Bindings) {
     ResultVisitor->visitMatch(BoundNodes(Binding));
   }
 }
 
 namespace {
 
 class VariadicMatcher : public DynMatcherInterface {
  public:
-  VariadicMatcher(VariadicOperatorFunction Func,
+  VariadicMatcher(DynTypedMatcher::VariadicOperatorFunction Func,
                   std::vector<DynTypedMatcher> InnerMatchers)
       : Func(Func), InnerMatchers(std::move(InnerMatchers)) {}
 
   bool dynMatches(const ast_type_traits::DynTypedNode &DynNode,
                   ASTMatchFinder *Finder,
                   BoundNodesTreeBuilder *Builder) const override {
     return Func(DynNode, Finder, Builder, InnerMatchers);
   }
 
  private:
-  VariadicOperatorFunction Func;
+  DynTypedMatcher::VariadicOperatorFunction Func;
   std::vector<DynTypedMatcher> InnerMatchers;
 };
 
 class IdDynMatcher : public DynMatcherInterface {
  public:
   IdDynMatcher(StringRef ID,
                const IntrusiveRefCntPtr<DynMatcherInterface> &InnerMatcher)
       : ID(ID), InnerMatcher(InnerMatcher) {}
bool dynMatches(const ast_type_traits::DynTypedNode &, ASTMatchFinder *,
     return true;
   }
 };
 static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance;
 
 }  // namespace
 
 DynTypedMatcher DynTypedMatcher::constructVariadic(
-    VariadicOperatorFunction Func, std::vector<DynTypedMatcher> InnerMatchers) {
+    DynTypedMatcher::VariadicOperatorFunction Func,
+    std::vector<DynTypedMatcher> InnerMatchers) {
   assert(InnerMatchers.size() > 0 && "Array must not be empty.");
   assert(std::all_of(InnerMatchers.begin(), InnerMatchers.end(),
                      [&InnerMatchers](const DynTypedMatcher &M) {
            return InnerMatchers[0].SupportedKind.isSame(M.SupportedKind);
          }) &&
          "SupportedKind must match!");
 
   // We must relax the restrict kind here.

lib/ASTMatchers/Dynamic/Marshallers.h

 
 private:
   std::vector<std::unique_ptr<MatcherDescriptor>> Overloads;
 };
 
 /// \brief Variadic operator marshaller function.
 class VariadicOperatorMatcherDescriptor : public MatcherDescriptor {
 public:
-  typedef ast_matchers::internal::VariadicOperatorFunction VarFunc;
+  typedef DynTypedMatcher::VariadicOperatorFunction VarFunc;
   VariadicOperatorMatcherDescriptor(unsigned MinCount, unsigned MaxCount,
                                     VarFunc Func, StringRef MatcherName)
       : MinCount(MinCount), MaxCount(MaxCount), Func(Func),
         MatcherName(MatcherName) {}
 
   virtual VariantMatcher create(const SourceRange &NameRange,
                                 ArrayRef<ParserValue> Args,
                                 Diagnostics *Error) const override {

lib/ASTMatchers/Dynamic/VariantValue.cpp

 VariantMatcher::MatcherOps::canConstructFrom(const DynTypedMatcher &Matcher,
                                              bool &IsExactMatch) const {
   IsExactMatch = Matcher.getSupportedKind().isSame(NodeKind);
   return Matcher.canConvertTo(NodeKind);
 }
 
 llvm::Optional<DynTypedMatcher>
 VariantMatcher::MatcherOps::constructVariadicOperator(
-    ast_matchers::internal::VariadicOperatorFunction Func,
+    DynTypedMatcher::VariadicOperatorFunction Func,
     ArrayRef<VariantMatcher> InnerMatchers) const {
   std::vector<DynTypedMatcher> DynMatchers;
   for (const auto &InnerMatcher : InnerMatchers) {
     // Abort if any of the inner matchers can't be converted to
     // Matcher<T>.
     if (!InnerMatcher.Value)
       return llvm::None;
     llvm::Optional<DynTypedMatcher> Inner =
bool isConvertibleTo(ast_type_traits::ASTNodeKind Kind,
     return MaxSpecificity > 0;
   }
 
   const std::vector<DynTypedMatcher> Matchers;
 };
 
 class VariantMatcher::VariadicOpPayload : public VariantMatcher::Payload {
 public:
-  VariadicOpPayload(ast_matchers::internal::VariadicOperatorFunction Func,
+  VariadicOpPayload(DynTypedMatcher::VariadicOperatorFunction Func,
                     std::vector<VariantMatcher> Args)
       : Func(Func), Args(std::move(Args)) {}
 
   llvm::Optional<DynTypedMatcher> getSingleMatcher() const override {
     return llvm::Optional<DynTypedMatcher>();
   }
 
   std::string getTypeAsString() const override {
bool isConvertibleTo(ast_type_traits::ASTNodeKind Kind,
     for (const VariantMatcher &Matcher : Args) {
       if (!Matcher.isConvertibleTo(Kind, Specificity))
         return false;
     }
     return true;
   }
 
 private:
-  const ast_matchers::internal::VariadicOperatorFunction Func;
+  const DynTypedMatcher::VariadicOperatorFunction Func;
   const std::vector<VariantMatcher> Args;
 };
 
 VariantMatcher::VariantMatcher() {}
 
 VariantMatcher VariantMatcher::SingleMatcher(const DynTypedMatcher &Matcher) {
   return VariantMatcher(new SinglePayload(Matcher));
 }
 
 VariantMatcher
 VariantMatcher::PolymorphicMatcher(std::vector<DynTypedMatcher> Matchers) {
   return VariantMatcher(new PolymorphicPayload(std::move(Matchers)));
 }
 
 VariantMatcher VariantMatcher::VariadicOperatorMatcher(
-    ast_matchers::internal::VariadicOperatorFunction Func,
+    DynTypedMatcher::VariadicOperatorFunction Func,
     std::vector<VariantMatcher> Args) {
   return VariantMatcher(new VariadicOpPayload(Func, std::move(Args)));
 }
 
 llvm::Optional<DynTypedMatcher> VariantMatcher::getSingleMatcher() const {
   return Value ? Value->getSingleMatcher() : llvm::Optional<DynTypedMatcher>();
 }