Made the Payload be ref counted. They are const by design, so there is no 
problem sharing them.
  This change reduces copies and simplifies the implementation of 
VariantMatcher.

Hi klimek,

http://llvm-reviews.chandlerc.com/D1446

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1446?vs=3630&id=3671#toc

BRANCH
  variant_matcher

ARCANIST PROJECT
  clang

Files:
  include/clang/ASTMatchers/Dynamic/VariantValue.h
  lib/ASTMatchers/Dynamic/VariantValue.cpp
Index: include/clang/ASTMatchers/Dynamic/VariantValue.h
===================================================================
--- include/clang/ASTMatchers/Dynamic/VariantValue.h
+++ include/clang/ASTMatchers/Dynamic/VariantValue.h
@@ -21,6 +21,7 @@
 
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/type_traits.h"
 
@@ -44,13 +45,30 @@
 ///  - hasTypedMatcher<T>()/getTypedMatcher<T>(): These calls will determine if
 ///    the underlying matcher(s) can unambiguously return a Matcher<T>.
 class VariantMatcher {
+  /// \brief Methods that depend on T from hasTypedMatcher/getTypedMatcher.
+  class MatcherOps {
+  public:
+    virtual ~MatcherOps();
+    virtual bool canConstructFrom(const DynTypedMatcher &Matcher) const = 0;
+  };
+
+  /// \brief Payload interface to be specialized by each matcher type.
+  ///
+  /// It follows a similar interface as VariantMatcher itself.
+  class Payload : public RefCountedBaseVPTR {
+  public:
+    virtual ~Payload();
+    virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const = 0;
+    virtual std::string getTypeAsString() const = 0;
+    virtual bool hasTypedMatcher(const MatcherOps &Ops) const = 0;
+    virtual const DynTypedMatcher *
+    getTypedMatcher(const MatcherOps &Ops) const = 0;
+  };
+
 public:
   /// \brief A null matcher.
   VariantMatcher();
 
-  /// \brief Clones the matcher objects.
-  VariantMatcher(const VariantMatcher &Other);
-
   /// \brief Clones the provided matcher.
   static VariantMatcher SingleMatcher(const DynTypedMatcher &Matcher);
 
@@ -60,46 +78,42 @@
   static VariantMatcher
   PolymorphicMatcher(ArrayRef<const DynTypedMatcher *> Matchers);
 
-  ~VariantMatcher();
-
-  /// \brief Copy the \c VariantMatcher, by making a copy of its representation.
-  VariantMatcher &operator=(const VariantMatcher &Other);
-
   /// \brief Makes the matcher the "null" matcher.
   void reset();
 
   /// \brief Whether the matcher is null.
-  bool isNull() const { return List.empty(); }
+  bool isNull() const { return !Value; }
 
   /// \brief Return a single matcher, if there is no ambiguity.
   ///
-  /// \returns True, and set Out to the matcher, if there is only one matcher.
-  /// False, if the underlying matcher is a polymorphic matcher with
+  /// \returns \c true, and set Out to the matcher, if there is only one
+  /// matcher. \c false, if the underlying matcher is a polymorphic matcher with
   /// more than one representation.
   bool getSingleMatcher(const DynTypedMatcher *&Out) const;
 
-  /// \brief Determines if any of the contained matchers can be converted
-  ///   to \c Matcher<T>.
+  /// \brief Determines if the contained matcher can be converted to
+  ///   \c Matcher<T>.
   ///
-  /// Returns true if one, and only one, of the contained matchers can be
-  /// converted to \c Matcher<T>. If there are more than one that can, the
-  /// result would be ambigous and false is returned.
+  /// For the Single case, it returns true if it can be converted to
+  /// \c Matcher<T>.
+  /// For the Polymorphic case, it returns true if one, and only one, of the
+  /// overloads can be converted to \c Matcher<T>. If there are more than one
+  /// that can, the result would be ambiguous and false is returned.
   template <class T>
   bool hasTypedMatcher() const {
-    return getTypedMatcher(
-        &ast_matchers::internal::Matcher<T>::canConstructFrom) != NULL;
+    if (Value) return Value->hasTypedMatcher(TypedMatcherOps<T>());
+    return false;
   }
 
-  /// \brief Wrap the correct matcher as a \c Matcher<T>.
+  /// \brief Return this matcher as a \c Matcher<T>.
   ///
-  /// Selects the appropriate matcher from the wrapped matchers and returns it
-  /// as a \c Matcher<T>.
+  /// Handles the different types (Single, Polymorphic) accordingly.
   /// Asserts that \c hasTypedMatcher<T>() is true.
   template <class T>
   ast_matchers::internal::Matcher<T> getTypedMatcher() const {
     assert(hasTypedMatcher<T>());
-    return ast_matchers::internal::Matcher<T>::constructFrom(*getTypedMatcher(
-        &ast_matchers::internal::Matcher<T>::canConstructFrom));
+    return ast_matchers::internal::Matcher<T>::constructFrom(
+        *Value->getTypedMatcher(TypedMatcherOps<T>()));
   }
 
   /// \brief String representation of the type of the value.
@@ -109,13 +123,20 @@
   std::string getTypeAsString() const;
 
 private:
-  /// \brief Returns the matcher that passes the callback.
-  ///
-  /// Returns NULL if no matcher passes the test, or if more than one do.
-  const DynTypedMatcher *
-  getTypedMatcher(bool (*CanConstructCallback)(const DynTypedMatcher &)) const;
+  explicit VariantMatcher(Payload *Value) : Value(Value) {}
+
+  class SinglePayload;
+  class PolymorphicPayload;
+
+  template <typename T>
+  class TypedMatcherOps : public MatcherOps {
+  public:
+    virtual bool canConstructFrom(const DynTypedMatcher &Matcher) const {
+      return ast_matchers::internal::Matcher<T>::canConstructFrom(Matcher);
+    }
+  };
 
-  std::vector<const DynTypedMatcher *> List;
+  IntrusiveRefCntPtr<const Payload> Value;
 };
 
 /// \brief Variant value class.
Index: lib/ASTMatchers/Dynamic/VariantValue.cpp
===================================================================
--- lib/ASTMatchers/Dynamic/VariantValue.cpp
+++ lib/ASTMatchers/Dynamic/VariantValue.cpp
@@ -21,69 +21,105 @@
 namespace ast_matchers {
 namespace dynamic {
 
-VariantMatcher::VariantMatcher() : List() {}
+VariantMatcher::MatcherOps::~MatcherOps() {}
+VariantMatcher::Payload::~Payload() {}
 
-VariantMatcher::VariantMatcher(const VariantMatcher& Other) {
-  *this = Other;
-}
+class VariantMatcher::SinglePayload : public VariantMatcher::Payload {
+public:
+  SinglePayload(const DynTypedMatcher &Matcher) : Matcher(Matcher.clone()) {}
 
-VariantMatcher VariantMatcher::SingleMatcher(const DynTypedMatcher &Matcher) {
-  VariantMatcher Out;
-  Out.List.push_back(Matcher.clone());
-  return Out;
-}
+  virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const {
+    Out = Matcher.get();
+    return true;
+  }
 
-VariantMatcher
-VariantMatcher::PolymorphicMatcher(ArrayRef<const DynTypedMatcher *> Matchers) {
-  VariantMatcher Out;
-  for (size_t i = 0, e = Matchers.size(); i != e; ++i) {
-    Out.List.push_back(Matchers[i]->clone());
+  virtual std::string getTypeAsString() const {
+    return (Twine("Matcher<") + Matcher->getSupportedKind().asStringRef() + ">")
+        .str();
   }
-  return Out;
-}
 
-VariantMatcher::~VariantMatcher() {
-  reset();
-}
+  virtual bool hasTypedMatcher(const MatcherOps &Ops) const {
+    return Ops.canConstructFrom(*Matcher);
+  }
 
-VariantMatcher &VariantMatcher::operator=(const VariantMatcher &Other) {
-  if (this == &Other) return *this;
-  reset();
-  for (size_t i = 0, e = Other.List.size(); i != e; ++i) {
-    List.push_back(Other.List[i]->clone());
+  virtual const DynTypedMatcher *getTypedMatcher(const MatcherOps &Ops) const {
+    assert(hasTypedMatcher(Ops));
+    return Matcher.get();
   }
-  return *this;
-}
 
-bool VariantMatcher::getSingleMatcher(const DynTypedMatcher *&Out) const {
-  if (List.size() != 1) return false;
-  Out = List[0];
-  return true;
-}
+private:
+  OwningPtr<const DynTypedMatcher> Matcher;
+};
 
-void VariantMatcher::reset() {
-  llvm::DeleteContainerPointers(List);
-}
+class VariantMatcher::PolymorphicPayload : public VariantMatcher::Payload {
+public:
+  PolymorphicPayload(ArrayRef<const DynTypedMatcher *> MatchersIn) {
+    for (size_t i = 0, e = MatchersIn.size(); i != e; ++i) {
+      Matchers.push_back(MatchersIn[i]->clone());
+    }
+  }
 
-std::string VariantMatcher::getTypeAsString() const {
-  std::string Inner;
-  for (size_t I = 0, E = List.size(); I != E; ++I) {
-    if (I != 0) Inner += "|";
-    Inner += List[I]->getSupportedKind().asStringRef();
+  virtual ~PolymorphicPayload() {
+    llvm::DeleteContainerPointers(Matchers);
   }
-  return (Twine("Matcher<") + Inner + ">").str();
-}
 
-const DynTypedMatcher *VariantMatcher::getTypedMatcher(
-    bool (*CanConstructCallback)(const DynTypedMatcher &)) const {
-  const DynTypedMatcher *Out = NULL;
-  for (size_t i = 0, e = List.size(); i != e; ++i) {
-    if (CanConstructCallback(*List[i])) {
-      if (Out) return NULL;
-      Out = List[i];
+  virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const {
+    if (Matchers.size() != 1)
+      return false;
+    Out = Matchers[0];
+    return true;
+  }
+
+  virtual std::string getTypeAsString() const {
+    std::string Inner;
+    for (size_t i = 0, e = Matchers.size(); i != e; ++i) {
+      if (i != 0)
+        Inner += "|";
+      Inner += Matchers[i]->getSupportedKind().asStringRef();
     }
+    return (Twine("Matcher<") + Inner + ">").str();
+  }
+
+  virtual bool hasTypedMatcher(const MatcherOps &Ops) const {
+    return getTypedMatcher(Ops) != NULL;
   }
-  return Out;
+
+  virtual const DynTypedMatcher *getTypedMatcher(const MatcherOps &Ops) const {
+    const DynTypedMatcher* Found = NULL;
+    for (size_t i = 0, e = Matchers.size(); i != e; ++i) {
+      if (Ops.canConstructFrom(*Matchers[i])) {
+        if (Found) return NULL;
+        Found = Matchers[i];
+      }
+    }
+    return Found;
+  }
+
+private:
+  std::vector<const DynTypedMatcher *> Matchers;
+};
+
+VariantMatcher::VariantMatcher() {}
+
+VariantMatcher VariantMatcher::SingleMatcher(const DynTypedMatcher &Matcher) {
+  return VariantMatcher(new SinglePayload(Matcher));
+}
+
+VariantMatcher
+VariantMatcher::PolymorphicMatcher(ArrayRef<const DynTypedMatcher *> Matchers) {
+  return VariantMatcher(new PolymorphicPayload(Matchers));
+}
+
+bool VariantMatcher::getSingleMatcher(const DynTypedMatcher *&Out) const {
+  if (Value) return Value->getSingleMatcher(Out);
+  return false;
+}
+
+void VariantMatcher::reset() { Value.reset(); }
+
+std::string VariantMatcher::getTypeAsString() const {
+  if (Value) return Value->getTypeAsString();
+  return "<Nothing>";
 }
 
 VariantValue::VariantValue(const VariantValue &Other) : Type(VT_Nothing) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to