r217152 - Refactor VariantMatcher::MatcherOps to reduce the amount of generated code.

Samuel Benzaquen sbenza at google.com
Thu Sep 4 07:13:58 PDT 2014


Author: sbenza
Date: Thu Sep  4 09:13:58 2014
New Revision: 217152

URL: http://llvm.org/viewvc/llvm-project?rev=217152&view=rev
Log:
Refactor VariantMatcher::MatcherOps to reduce the amount of generated code.

Summary:
Refactor VariantMatcher::MatcherOps to reduce the amount of generated code.
 - Make some code type agnostic and move it to the cpp file.
 - Return a DynTypedMatcher instead of storing the object in MatcherOps.

This change reduces the number of symbols generated in Registry.cpp by
~19%, the object byte size by ~17% and the compilation time (in non-release mode) by ~20%.

Reviewers: klimek

Subscribers: klimek, cfe-commits

Differential Revision: http://reviews.llvm.org/D5124

Modified:
    cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
    cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
    cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
    cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=217152&r1=217151&r2=217152&view=diff
==============================================================================
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Thu Sep  4 09:13:58 2014
@@ -342,6 +342,17 @@ public:
   /// 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)(
+      const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder,
+      BoundNodesTreeBuilder *Builder, ArrayRef<DynTypedMatcher> InnerMatchers);
+  static DynTypedMatcher
+  constructVariadic(VariadicOperatorFunction Func,
+                    ArrayRef<DynTypedMatcher> InnerMatchers) {
+    assert(InnerMatchers.size() > 0 && "Array must not be empty.");
+    return DynTypedMatcher(new VariadicStorage(Func, InnerMatchers));
+  }
+
   /// \brief Returns true if the matcher matches the given \c DynNode.
   bool matches(const ast_type_traits::DynTypedNode DynNode,
                ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const {
@@ -372,9 +383,9 @@ public:
   /// This method verifies that the underlying matcher in \c Other can process
   /// nodes of types T.
   template <typename T> bool canConvertTo() const {
-    return getSupportedKind().isBaseOf(
-        ast_type_traits::ASTNodeKind::getFromNodeKind<T>());
+    return canConvertTo(ast_type_traits::ASTNodeKind::getFromNodeKind<T>());
   }
+  bool canConvertTo(ast_type_traits::ASTNodeKind To) const;
 
   /// \brief Construct a \c Matcher<T> interface around the dynamic matcher.
   ///
@@ -416,9 +427,35 @@ private:
     const uint64_t ID;
   };
 
+  class VariadicStorage : public MatcherStorage {
+  public:
+    VariadicStorage(VariadicOperatorFunction Func,
+                    ArrayRef<DynTypedMatcher> InnerMatchers)
+        : MatcherStorage(InnerMatchers[0].getSupportedKind(),
+                         reinterpret_cast<uint64_t>(this)),
+          Func(Func), InnerMatchers(InnerMatchers) {}
+
+    bool matches(const ast_type_traits::DynTypedNode DynNode,
+                 ASTMatchFinder *Finder,
+                 BoundNodesTreeBuilder *Builder) const override {
+      return Func(DynNode, Finder, Builder, InnerMatchers);
+    }
+
+    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;
 };
 
@@ -460,16 +497,8 @@ inline DynTypedMatcher::DynTypedMatcher(
 
 /// \brief Specialization of the conversion functions for QualType.
 ///
-/// These specializations provide the Matcher<Type>->Matcher<QualType>
+/// This specialization provides the Matcher<Type>->Matcher<QualType>
 /// conversion that the static API does.
-template <> inline bool DynTypedMatcher::canConvertTo<QualType>() const {
-  const ast_type_traits::ASTNodeKind SourceKind = getSupportedKind();
-  return SourceKind.isSame(
-             ast_type_traits::ASTNodeKind::getFromNodeKind<Type>()) ||
-         SourceKind.isSame(
-             ast_type_traits::ASTNodeKind::getFromNodeKind<QualType>());
-}
-
 template <>
 inline Matcher<QualType> DynTypedMatcher::convertTo<QualType>() const {
   assert(canConvertTo<QualType>());

Modified: cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h?rev=217152&r1=217151&r2=217152&view=diff
==============================================================================
--- cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h Thu Sep  4 09:13:58 2014
@@ -93,13 +93,25 @@ class VariantMatcher {
   /// \brief Methods that depend on T from hasTypedMatcher/getTypedMatcher.
   class MatcherOps {
   public:
-    virtual ~MatcherOps();
-    virtual bool canConstructFrom(const DynTypedMatcher &Matcher,
-                                  bool &IsExactMatch) const = 0;
-    virtual void constructFrom(const DynTypedMatcher &Matcher) = 0;
-    virtual void constructVariadicOperator(
+    MatcherOps(ast_type_traits::ASTNodeKind NodeKind) : NodeKind(NodeKind) {}
+
+    bool canConstructFrom(const DynTypedMatcher &Matcher,
+                          bool &IsExactMatch) const;
+
+    /// \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,
-        ArrayRef<VariantMatcher> InnerMatchers) = 0;
+        ArrayRef<VariantMatcher> InnerMatchers) const;
+
+  private:
+    ast_type_traits::ASTNodeKind NodeKind;
   };
 
   /// \brief Payload interface to be specialized by each matcher type.
@@ -110,7 +122,8 @@ class VariantMatcher {
     virtual ~Payload();
     virtual llvm::Optional<DynTypedMatcher> getSingleMatcher() const = 0;
     virtual std::string getTypeAsString() const = 0;
-    virtual void makeTypedMatcher(MatcherOps &Ops) const = 0;
+    virtual llvm::Optional<DynTypedMatcher>
+    getTypedMatcher(const MatcherOps &Ops) const = 0;
     virtual bool isConvertibleTo(ast_type_traits::ASTNodeKind Kind,
                                  unsigned *Specificity) const = 0;
   };
@@ -158,9 +171,8 @@ public:
   /// that can, the result would be ambiguous and false is returned.
   template <class T>
   bool hasTypedMatcher() const {
-    TypedMatcherOps<T> Ops;
-    if (Value) Value->makeTypedMatcher(Ops);
-    return Ops.hasMatcher();
+    if (!Value) return false;
+    return Value->getTypedMatcher(TypedMatcherOps<T>()).hasValue();
   }
 
   /// \brief Determines if the contained matcher can be converted to \p Kind.
@@ -182,10 +194,9 @@ public:
   /// Asserts that \c hasTypedMatcher<T>() is true.
   template <class T>
   ast_matchers::internal::Matcher<T> getTypedMatcher() const {
-    TypedMatcherOps<T> Ops;
-    Value->makeTypedMatcher(Ops);
-    assert(Ops.hasMatcher() && "hasTypedMatcher<T>() == false");
-    return Ops.matcher();
+    assert(hasTypedMatcher<T>() && "hasTypedMatcher<T>() == false");
+    return Value->getTypedMatcher(TypedMatcherOps<T>())
+        ->template convertTo<T>();
   }
 
   /// \brief String representation of the type of the value.
@@ -197,53 +208,27 @@ public:
 private:
   explicit VariantMatcher(Payload *Value) : Value(Value) {}
 
+  template <typename T> struct TypedMatcherOps;
+
   class SinglePayload;
   class PolymorphicPayload;
   class VariadicOpPayload;
 
-  template <typename T>
-  class TypedMatcherOps : public MatcherOps {
-  public:
-    typedef ast_matchers::internal::Matcher<T> MatcherT;
-
-    virtual bool canConstructFrom(const DynTypedMatcher &Matcher,
-                                  bool &IsExactMatch) const {
-      IsExactMatch = Matcher.getSupportedKind().isSame(
-          ast_type_traits::ASTNodeKind::getFromNodeKind<T>());
-      return Matcher.canConvertTo<T>();
-    }
-
-    virtual void constructFrom(const DynTypedMatcher& Matcher) {
-      Out.reset(new MatcherT(Matcher.convertTo<T>()));
-    }
-
-    virtual void constructVariadicOperator(
-        ast_matchers::internal::VariadicOperatorFunction Func,
-        ArrayRef<VariantMatcher> InnerMatchers) {
-      std::vector<DynTypedMatcher> DynMatchers;
-      for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) {
-        // Abort if any of the inner matchers can't be converted to
-        // Matcher<T>.
-        if (!InnerMatchers[i].hasTypedMatcher<T>()) {
-          return;
-        }
-        DynMatchers.push_back(InnerMatchers[i].getTypedMatcher<T>());
-      }
-      Out.reset(new MatcherT(
-          new ast_matchers::internal::VariadicOperatorMatcherInterface<T>(
-              Func, DynMatchers)));
-    }
-
-    bool hasMatcher() const { return Out.get() != nullptr; }
-    const MatcherT &matcher() const { return *Out; }
-
-  private:
-    std::unique_ptr<MatcherT> Out;
-  };
-
   IntrusiveRefCntPtr<const Payload> Value;
 };
 
+template <typename T>
+struct VariantMatcher::TypedMatcherOps : VariantMatcher::MatcherOps {
+  TypedMatcherOps()
+      : MatcherOps(ast_type_traits::ASTNodeKind::getFromNodeKind<T>()) {}
+  typedef ast_matchers::internal::Matcher<T> MatcherT;
+
+  DynTypedMatcher
+  convertMatcher(const DynTypedMatcher &Matcher) const override {
+    return DynTypedMatcher(Matcher.convertTo<T>());
+  }
+};
+
 /// \brief Variant value class.
 ///
 /// Basically, a tagged union with value type semantics.

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=217152&r1=217151&r2=217152&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Thu Sep  4 09:13:58 2014
@@ -26,6 +26,17 @@ void BoundNodesTreeBuilder::visitMatches
   }
 }
 
+bool DynTypedMatcher::canConvertTo(ast_type_traits::ASTNodeKind To) const {
+  const auto From = getSupportedKind();
+  auto QualKind = ast_type_traits::ASTNodeKind::getFromNodeKind<QualType>();
+  auto TypeKind = ast_type_traits::ASTNodeKind::getFromNodeKind<Type>();
+  /// Mimic the implicit conversions of Matcher<>.
+  /// - From Matcher<Type> to Matcher<QualType>
+  if (From.isSame(TypeKind) && To.isSame(QualKind)) return true;
+  /// - From Matcher<Base> to Matcher<Derived>
+  return From.isBaseOf(To);
+}
+
 DynTypedMatcher::MatcherStorage::~MatcherStorage() {}
 
 void BoundNodesTreeBuilder::addMatch(const BoundNodesTreeBuilder &Other) {

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp?rev=217152&r1=217151&r2=217152&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp Thu Sep  4 09:13:58 2014
@@ -49,7 +49,32 @@ bool ArgKind::isConvertibleTo(ArgKind To
   return true;
 }
 
-VariantMatcher::MatcherOps::~MatcherOps() {}
+bool
+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,
+    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 =
+        InnerMatcher.Value->getTypedMatcher(*this);
+    if (!Inner)
+      return llvm::None;
+    DynMatchers.push_back(*Inner);
+  }
+  return DynTypedMatcher::constructVariadic(Func, DynMatchers);
+}
+
 VariantMatcher::Payload::~Payload() {}
 
 class VariantMatcher::SinglePayload : public VariantMatcher::Payload {
@@ -65,10 +90,12 @@ public:
         .str();
   }
 
-  void makeTypedMatcher(MatcherOps &Ops) const override {
+  llvm::Optional<DynTypedMatcher>
+  getTypedMatcher(const MatcherOps &Ops) const override {
     bool Ignore;
     if (Ops.canConstructFrom(Matcher, Ignore))
-      Ops.constructFrom(Matcher);
+      return Matcher;
+    return llvm::None;
   }
 
   bool isConvertibleTo(ast_type_traits::ASTNodeKind Kind,
@@ -104,7 +131,8 @@ public:
     return (Twine("Matcher<") + Inner + ">").str();
   }
 
-  void makeTypedMatcher(MatcherOps &Ops) const override {
+  llvm::Optional<DynTypedMatcher>
+  getTypedMatcher(const MatcherOps &Ops) const override {
     bool FoundIsExact = false;
     const DynTypedMatcher *Found = nullptr;
     int NumFound = 0;
@@ -124,7 +152,8 @@ public:
     }
     // We only succeed if we found exactly one, or if we found an exact match.
     if (Found && (FoundIsExact || NumFound == 1))
-      Ops.constructFrom(*Found);
+      return *Found;
+    return llvm::None;
   }
 
   bool isConvertibleTo(ast_type_traits::ASTNodeKind Kind,
@@ -165,8 +194,9 @@ public:
     return Inner;
   }
 
-  void makeTypedMatcher(MatcherOps &Ops) const override {
-    Ops.constructVariadicOperator(Func, Args);
+  llvm::Optional<DynTypedMatcher>
+  getTypedMatcher(const MatcherOps &Ops) const override {
+    return Ops.constructVariadicOperator(Func, Args);
   }
 
   bool isConvertibleTo(ast_type_traits::ASTNodeKind Kind,





More information about the cfe-commits mailing list