r217152 - Refactor VariantMatcher::MatcherOps to reduce the amount of generated code.
David Blaikie
dblaikie at gmail.com
Thu Sep 4 09:11:57 PDT 2014
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 <sbenza at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140904/5042385a/attachment.html>
More information about the cfe-commits
mailing list