I cleaned up the resulting clang -Wnon-virtual-dtor warnings with r217167 (feel free to fix otherwise/let me know if that wasn't the right approach).
On Thu, Sep 4, 2014 at 7:13 AM, Samuel Benzaquen <[email protected]> wrote: > 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, > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
