From 341b5df7f859e640c2ea2f35c0fff553ec55ada4 Mon Sep 17 00:00:00 2001 From: Samuel Benzaquen Date: Mon, 21 Oct 2013 18:40:51 +0000 Subject: [PATCH] Refactor DynTypedMatcher into a value type class, just like Matcher. Summary: Refactor DynTypedMatcher into a value type class, just like Matcher. This simplifies its usage and removes the virtual hierarchy from Matcher. It also enables planned changes to replace MatcherInteface. Too many instantiaions of this class hierarchy has been causing Registry.cpp.o to bloat in size and number of symbols. Reviewers: klimek CC: cfe-commits, revane Differential Revision: http://llvm-reviews.chandlerc.com/D1661 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@193100 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/ASTMatchers/ASTMatchFinder.h | 2 +- .../clang/ASTMatchers/ASTMatchersInternal.h | 317 ++++++++++-------- include/clang/ASTMatchers/Dynamic/Parser.h | 19 +- .../clang/ASTMatchers/Dynamic/VariantValue.h | 22 +- lib/ASTMatchers/ASTMatchFinder.cpp | 72 ++-- lib/ASTMatchers/ASTMatchersInternal.cpp | 18 +- lib/ASTMatchers/Dynamic/Marshallers.h | 11 +- lib/ASTMatchers/Dynamic/Parser.cpp | 22 +- lib/ASTMatchers/Dynamic/Registry.cpp | 8 +- lib/ASTMatchers/Dynamic/VariantValue.cpp | 52 ++- unittests/ASTMatchers/Dynamic/ParserTest.cpp | 75 ++--- 11 files changed, 297 insertions(+), 321 deletions(-) diff --git a/include/clang/ASTMatchers/ASTMatchFinder.h b/include/clang/ASTMatchers/ASTMatchFinder.h index d2b71d30df..8817d3eb0c 100644 --- a/include/clang/ASTMatchers/ASTMatchFinder.h +++ b/include/clang/ASTMatchers/ASTMatchFinder.h @@ -162,7 +162,7 @@ public: private: /// \brief For each \c DynTypedMatcher a \c MatchCallback that will be called /// when it matches. - std::vector > + std::vector > MatcherCallbackPairs; /// \brief Called when parsing is done. diff --git a/include/clang/ASTMatchers/ASTMatchersInternal.h b/include/clang/ASTMatchers/ASTMatchersInternal.h index 26ebc97c7d..079337d4ce 100644 --- a/include/clang/ASTMatchers/ASTMatchersInternal.h +++ b/include/clang/ASTMatchers/ASTMatchersInternal.h @@ -36,12 +36,13 @@ #define LLVM_CLANG_AST_MATCHERS_AST_MATCHERS_INTERNAL_H #include "clang/AST/ASTTypeTraits.h" -#include "clang/AST/DeclCXX.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/ExprCXX.h" -#include "clang/AST/StmtCXX.h" #include "clang/AST/Stmt.h" +#include "clang/AST/StmtCXX.h" #include "clang/AST/Type.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/VariadicFunction.h" #include "llvm/Support/type_traits.h" #include @@ -200,38 +201,6 @@ private: } }; -/// \brief Base class for all matchers that works on a \c DynTypedNode. -/// -/// Matcher implementations will check whether the \c DynTypedNode is -/// convertible into the respecitve types and then do the actual match -/// on the actual node, or return false if it is not convertible. -class DynTypedMatcher { -public: - virtual ~DynTypedMatcher(); - - /// \brief Returns true if the matcher matches the given \c DynNode. - virtual bool matches(const ast_type_traits::DynTypedNode DynNode, - ASTMatchFinder *Finder, - BoundNodesTreeBuilder *Builder) const = 0; - - /// \brief Makes a copy of this matcher object. - virtual DynTypedMatcher *clone() const = 0; - - /// \brief Returns a unique ID for the matcher. - virtual uint64_t getID() const = 0; - - /// \brief Bind the specified \p ID to the matcher. - /// \return A new matcher with the \p ID bound to it if this matcher supports - /// binding. Otherwise, returns NULL. Returns NULL by default. - virtual DynTypedMatcher* tryBind(StringRef ID) const; - - /// \brief Returns the type this matcher works on. - /// - /// \c matches() will always return false unless the node passed is of this - /// or a derived type. - virtual ast_type_traits::ASTNodeKind getSupportedKind() const = 0; -}; - /// \brief Wrapper of a MatcherInterface *that allows copying. /// /// A Matcher can be used anywhere a Matcher is @@ -241,7 +210,7 @@ public: /// operator rather than a type hierarchy to be able to templatize the /// type hierarchy instead of spelling it out. template -class Matcher : public DynTypedMatcher { +class Matcher { public: /// \brief Takes ownership of the provided implementation pointer. explicit Matcher(MatcherInterface *Implementation) @@ -267,35 +236,6 @@ public: llvm::is_same::value >::type* = 0) : Implementation(new TypeToQualType(Other)) {} - /// \brief Returns \c true if the passed DynTypedMatcher can be converted - /// to a \c Matcher. - /// - /// This method verifies that the underlying matcher in \c Other can process - /// nodes of types T. - static bool canConstructFrom(const DynTypedMatcher &Other) { - return Other.getSupportedKind() - .isBaseOf(ast_type_traits::ASTNodeKind::getFromNodeKind()); - } - - /// \brief Construct a Matcher interface around the dynamic matcher - /// \c Other. - /// - /// This method asserts that canConstructFrom(Other) is \c true. Callers - /// should call canConstructFrom(Other) first to make sure that Other is - /// compatible with T. - static Matcher constructFrom(const DynTypedMatcher &Other) { - assert(canConstructFrom(Other)); - return constructFromUnsafe(Other); - } - - /// \brief Same as constructFrom(), but does not check that the underlying - /// matcher can handle a value of T. - /// - /// If it is not compatible, then this matcher will never match anything. - static Matcher constructFromUnsafe(const DynTypedMatcher &Other) { - return Matcher(new WrappedMatcher(Other)); - } - /// \brief Forwards the call to the underlying MatcherInterface pointer. bool matches(const T &Node, ASTMatchFinder *Finder, @@ -316,23 +256,6 @@ public: return reinterpret_cast(Implementation.getPtr()); } - /// \brief Returns the type this matcher works on. - ast_type_traits::ASTNodeKind getSupportedKind() const { - return ast_type_traits::ASTNodeKind::getFromNodeKind(); - } - - /// \brief Returns whether the matcher matches on the given \c DynNode. - virtual bool matches(const ast_type_traits::DynTypedNode DynNode, - ASTMatchFinder *Finder, - BoundNodesTreeBuilder *Builder) const { - const T *Node = DynNode.get(); - if (!Node) return false; - return matches(*Node, Finder, Builder); - } - - /// \brief Makes a copy of this matcher object. - virtual Matcher *clone() const { return new Matcher(*this); } - /// \brief Allows the conversion of a \c Matcher to a \c /// Matcher. /// @@ -375,23 +298,6 @@ private: const Matcher From; }; - /// \brief Simple MatcherInterface wrapper around a DynTypedMatcher. - class WrappedMatcher : public MatcherInterface { - public: - explicit WrappedMatcher(const DynTypedMatcher &Matcher) - : Inner(Matcher.clone()) {} - virtual ~WrappedMatcher() {} - - bool matches(const T &Node, ASTMatchFinder *Finder, - BoundNodesTreeBuilder *Builder) const { - return Inner->matches(ast_type_traits::DynTypedNode::create(Node), Finder, - Builder); - } - - private: - const OwningPtr Inner; - }; - IntrusiveRefCntPtr< MatcherInterface > Implementation; }; // class Matcher @@ -402,15 +308,160 @@ inline Matcher makeMatcher(MatcherInterface *Implementation) { return Matcher(Implementation); } +template class BindableMatcher; + +/// \brief Matcher that works on a \c DynTypedNode. +/// +/// It is constructed from a \c Matcher object and redirects most calls to +/// underlying matcher. +/// It checks whether the \c DynTypedNode is convertible into the type of the +/// underlying matcher and then do the actual match on the actual node, or +/// return false if it is not convertible. +class DynTypedMatcher { +public: + /// \brief Construct from a \c Matcher. Copies the matcher. + template + DynTypedMatcher(const Matcher &M) + : Storage(new TypedMatcherStorage(M, false)) {} + + /// \brief Construct from a bindable \c Matcher. Copies the matcher. + /// + /// This version enables \c tryBind() on the \c DynTypedMatcher. + template + DynTypedMatcher(const BindableMatcher &M) + : Storage(new TypedMatcherStorage(M, true)) {} + + /// \brief Returns true if the matcher matches the given \c DynNode. + bool matches(const ast_type_traits::DynTypedNode DynNode, + ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const { + return Storage->matches(DynNode, Finder, Builder); + } + + /// \brief Bind the specified \p ID to the matcher. + /// \return A new matcher with the \p ID bound to it if this matcher supports + /// binding. Otherwise, returns an empty \c Optional<>. + llvm::Optional tryBind(StringRef ID) const { + return Storage->tryBind(ID); + } + + /// \brief Returns a unique \p ID for the matcher. + uint64_t getID() const { return Storage->getID(); } + + /// \brief Returns the type this matcher works on. + /// + /// \c matches() will always return false unless the node passed is of this + /// or a derived type. + ast_type_traits::ASTNodeKind getSupportedKind() const { + return Storage->getSupportedKind(); + } + + /// \brief Returns \c true if the passed \c DynTypedMatcher can be converted + /// to a \c Matcher. + /// + /// This method verifies that the underlying matcher in \c Other can process + /// nodes of types T. + template bool canConvertTo() const { + return getSupportedKind().isBaseOf( + ast_type_traits::ASTNodeKind::getFromNodeKind()); + } + + /// \brief Construct a \c Matcher interface around the dynamic matcher. + /// + /// This method asserts that \c canConvertTo() is \c true. Callers + /// should call \c canConvertTo() first to make sure that \c this is + /// compatible with T. + template Matcher convertTo() const { + assert(canConvertTo()); + return unconditionalConvertTo(); + } + + /// \brief Same as \c convertTo(), but does not check that the underlying + /// matcher can handle a value of T. + /// + /// If it is not compatible, then this matcher will never match anything. + template Matcher unconditionalConvertTo() const { + return Matcher(new WrappedMatcher(*this)); + } + +private: + class MatcherStorage : public RefCountedBaseVPTR { + public: + MatcherStorage(ast_type_traits::ASTNodeKind SupportedKind, uint64_t ID) + : SupportedKind(SupportedKind), ID(ID) {} + virtual ~MatcherStorage(); + + virtual bool matches(const ast_type_traits::DynTypedNode DynNode, + ASTMatchFinder *Finder, + BoundNodesTreeBuilder *Builder) const = 0; + + virtual llvm::Optional tryBind(StringRef ID) const = 0; + + ast_type_traits::ASTNodeKind getSupportedKind() const { + return SupportedKind; + } + + uint64_t getID() const { return ID; } + + private: + const ast_type_traits::ASTNodeKind SupportedKind; + const uint64_t ID; + }; + + template class TypedMatcherStorage : public MatcherStorage { + public: + TypedMatcherStorage(const Matcher &Other, bool AllowBind) + : MatcherStorage(ast_type_traits::ASTNodeKind::getFromNodeKind(), + Other.getID()), + InnerMatcher(Other), AllowBind(AllowBind) {} + + bool matches(const ast_type_traits::DynTypedNode DynNode, + ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const + LLVM_OVERRIDE { + if (const T *Node = DynNode.get()) { + return InnerMatcher.matches(*Node, Finder, Builder); + } + return false; + } + + llvm::Optional tryBind(StringRef ID) const LLVM_OVERRIDE { + if (!AllowBind) + return llvm::Optional(); + return DynTypedMatcher(BindableMatcher(InnerMatcher).bind(ID)); + } + + private: + const Matcher InnerMatcher; + const bool AllowBind; + }; + + /// \brief Simple MatcherInterface wrapper around a DynTypedMatcher. + template class WrappedMatcher; + + IntrusiveRefCntPtr Storage; +}; + +template +class DynTypedMatcher::WrappedMatcher : public MatcherInterface { +public: + explicit WrappedMatcher(const DynTypedMatcher &Matcher) : Inner(Matcher) {} + virtual ~WrappedMatcher() {} + + bool matches(const T &Node, ASTMatchFinder *Finder, + BoundNodesTreeBuilder *Builder) const { + return Inner.matches(ast_type_traits::DynTypedNode::create(Node), Finder, + Builder); + } + +private: + const DynTypedMatcher Inner; +}; + /// \brief Specialization of the conversion functions for QualType. /// /// These specializations provide the Matcher->Matcher /// conversion that the static API does. -template <> -inline bool -Matcher::canConstructFrom(const DynTypedMatcher &Other) { - ast_type_traits::ASTNodeKind SourceKind = Other.getSupportedKind(); - // We support implicit conversion from Matcher to Matcher +template <> inline bool DynTypedMatcher::canConvertTo() const { + const ast_type_traits::ASTNodeKind SourceKind = getSupportedKind(); return SourceKind.isSame( ast_type_traits::ASTNodeKind::getFromNodeKind()) || SourceKind.isSame( @@ -418,16 +469,15 @@ Matcher::canConstructFrom(const DynTypedMatcher &Other) { } template <> -inline Matcher -Matcher::constructFrom(const DynTypedMatcher &Other) { - assert(canConstructFrom(Other)); - ast_type_traits::ASTNodeKind SourceKind = Other.getSupportedKind(); +inline Matcher DynTypedMatcher::convertTo() const { + assert(canConvertTo()); + const ast_type_traits::ASTNodeKind SourceKind = getSupportedKind(); if (SourceKind.isSame( ast_type_traits::ASTNodeKind::getFromNodeKind())) { // We support implicit conversion from Matcher to Matcher - return Matcher::constructFrom(Other); + return unconditionalConvertTo(); } - return makeMatcher(new WrappedMatcher(Other)); + return unconditionalConvertTo(); } /// \brief Finds the first node in a range that matches the given matcher. @@ -973,16 +1023,6 @@ public: Matcher bind(StringRef ID) const { return Matcher(new IdMatcher(ID, *this)); } - - /// \brief Makes a copy of this matcher object. - virtual BindableMatcher* clone() const { - return new BindableMatcher(*this); - } - - /// \brief Bind the specified \c ID to the matcher. - virtual Matcher* tryBind(StringRef ID) const { - return new Matcher(bind(ID)); - } }; /// \brief Matches nodes of type T that have child nodes of type ChildT for @@ -1075,8 +1115,7 @@ private: /// matchers as an array of DynTypedMatcher. typedef bool (*VariadicOperatorFunction)( const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder, - BoundNodesTreeBuilder *Builder, - ArrayRef InnerMatchers); + BoundNodesTreeBuilder *Builder, ArrayRef InnerMatchers); /// \brief \c MatcherInterface implementation for an variadic operator. template @@ -1086,14 +1125,10 @@ public: ArrayRef *> InputMatchers) : Func(Func) { for (size_t i = 0, e = InputMatchers.size(); i != e; ++i) { - InnerMatchers.push_back(new Matcher(*InputMatchers[i])); + InnerMatchers.push_back(*InputMatchers[i]); } } - ~VariadicOperatorMatcherInterface() { - llvm::DeleteContainerPointers(InnerMatchers); - } - virtual bool matches(const T &Node, ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const { return Func(ast_type_traits::DynTypedNode::create(Node), Finder, Builder, @@ -1102,7 +1137,7 @@ public: private: const VariadicOperatorFunction Func; - std::vector InnerMatchers; + std::vector InnerMatchers; }; /// \brief "No argument" placeholder to use as template paratemers. @@ -1196,24 +1231,24 @@ struct VariadicOperatorMatcherFunc { /// @} /// \brief Matches nodes for which all provided matchers match. -bool -AllOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, - ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder, - ArrayRef InnerMatchers); +bool AllOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, + ASTMatchFinder *Finder, + BoundNodesTreeBuilder *Builder, + ArrayRef InnerMatchers); /// \brief Matches nodes for which at least one of the provided matchers /// matches, but doesn't stop at the first match. -bool -EachOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, - ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder, - ArrayRef InnerMatchers); +bool EachOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, + ASTMatchFinder *Finder, + BoundNodesTreeBuilder *Builder, + ArrayRef InnerMatchers); /// \brief Matches nodes for which at least one of the provided matchers /// matches. -bool -AnyOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, - ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder, - ArrayRef InnerMatchers); +bool AnyOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, + ASTMatchFinder *Finder, + BoundNodesTreeBuilder *Builder, + ArrayRef InnerMatchers); /// \brief Creates a Matcher that matches if all inner matchers match. template @@ -1232,8 +1267,8 @@ BindableMatcher makeAllOfComposite( template BindableMatcher makeDynCastAllOfComposite( ArrayRef *> InnerMatchers) { - return BindableMatcher( - Matcher::constructFromUnsafe(makeAllOfComposite(InnerMatchers))); + return BindableMatcher(DynTypedMatcher(makeAllOfComposite(InnerMatchers)) + .unconditionalConvertTo()); } /// \brief Matches nodes of type T that have at least one descendant node of diff --git a/include/clang/ASTMatchers/Dynamic/Parser.h b/include/clang/ASTMatchers/Dynamic/Parser.h index 3759f631b3..bb6ac76989 100644 --- a/include/clang/ASTMatchers/Dynamic/Parser.h +++ b/include/clang/ASTMatchers/Dynamic/Parser.h @@ -37,6 +37,7 @@ #include "clang/ASTMatchers/Dynamic/VariantValue.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" namespace clang { @@ -92,11 +93,12 @@ public: /// /// \param MatcherCode The matcher expression to parse. /// - /// \return The matcher object constructed, or NULL if an error occurred. - // In that case, \c Error will contain a description of the error. + /// \return The matcher object constructed, or an empty Optional if an error + /// occurred. + /// In that case, \c Error will contain a description of the error. /// The caller takes ownership of the DynTypedMatcher object returned. - static DynTypedMatcher *parseMatcherExpression(StringRef MatcherCode, - Diagnostics *Error); + static llvm::Optional + parseMatcherExpression(StringRef MatcherCode, Diagnostics *Error); /// \brief Parse a matcher expression. /// @@ -104,13 +106,12 @@ public: /// /// \param S The Sema instance that will help the parser /// construct the matchers. - /// \return The matcher object constructed by the processor, or NULL - /// if an error occurred. In that case, \c Error will contain a + /// \return The matcher object constructed by the processor, or an empty + /// Optional if an error occurred. In that case, \c Error will contain a /// description of the error. /// The caller takes ownership of the DynTypedMatcher object returned. - static DynTypedMatcher *parseMatcherExpression(StringRef MatcherCode, - Sema *S, - Diagnostics *Error); + static llvm::Optional + parseMatcherExpression(StringRef MatcherCode, Sema *S, Diagnostics *Error); /// \brief Parse an expression, creating matchers from the registry. /// diff --git a/include/clang/ASTMatchers/Dynamic/VariantValue.h b/include/clang/ASTMatchers/Dynamic/VariantValue.h index 9d7e4e13ff..b9bc017a48 100644 --- a/include/clang/ASTMatchers/Dynamic/VariantValue.h +++ b/include/clang/ASTMatchers/Dynamic/VariantValue.h @@ -22,6 +22,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchersInternal.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/type_traits.h" @@ -62,7 +63,7 @@ class VariantMatcher { class Payload : public RefCountedBaseVPTR { public: virtual ~Payload(); - virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const = 0; + virtual llvm::Optional getSingleMatcher() const = 0; virtual std::string getTypeAsString() const = 0; virtual void makeTypedMatcher(MatcherOps &Ops) const = 0; }; @@ -77,8 +78,7 @@ public: /// \brief Clones the provided matchers. /// /// They should be the result of a polymorphic matcher. - static VariantMatcher - PolymorphicMatcher(ArrayRef Matchers); + static VariantMatcher PolymorphicMatcher(ArrayRef Matchers); /// \brief Creates a 'variadic' operator matcher. /// @@ -95,10 +95,10 @@ public: /// \brief Return a single matcher, if there is no ambiguity. /// - /// \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; + /// \returns the matcher, if there is only one matcher. An empty Optional, if + /// the underlying matcher is a polymorphic matcher with more than one + /// representation. + llvm::Optional getSingleMatcher() const; /// \brief Determines if the contained matcher can be converted to /// \c Matcher. @@ -146,11 +146,11 @@ private: typedef ast_matchers::internal::Matcher MatcherT; virtual bool canConstructFrom(const DynTypedMatcher &Matcher) const { - return MatcherT::canConstructFrom(Matcher); + return Matcher.canConvertTo(); } virtual void constructFrom(const DynTypedMatcher& Matcher) { - Out.reset(new MatcherT(MatcherT::constructFrom(Matcher))); + Out.reset(new MatcherT(Matcher.convertTo())); } virtual void constructVariadicOperator( @@ -173,7 +173,9 @@ private: new ast_matchers::internal::VariadicOperatorMatcherInterface( Func, ArrayRef(InnerArgs, NumArgs)))); } - std::for_each(InnerArgs, InnerArgs + NumArgs, llvm::deleter); + for (size_t i = 0; i != NumArgs; ++i) { + delete InnerArgs[i]; + } delete[] InnerArgs; } diff --git a/lib/ASTMatchers/ASTMatchFinder.cpp b/lib/ASTMatchers/ASTMatchFinder.cpp index 78bd9be935..fbb696f464 100644 --- a/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/lib/ASTMatchers/ASTMatchFinder.cpp @@ -295,25 +295,26 @@ private: class MatchASTVisitor : public RecursiveASTVisitor, public ASTMatchFinder { public: - MatchASTVisitor(std::vector > *MatcherCallbackPairs) - : MatcherCallbackPairs(MatcherCallbackPairs), - ActiveASTContext(NULL) { - } + MatchASTVisitor( + std::vector > * + MatcherCallbackPairs) + : MatcherCallbackPairs(MatcherCallbackPairs), ActiveASTContext(NULL) {} void onStartOfTranslationUnit() { - for (std::vector >::const_iterator - I = MatcherCallbackPairs->begin(), E = MatcherCallbackPairs->end(); + for (std::vector >::const_iterator + I = MatcherCallbackPairs->begin(), + E = MatcherCallbackPairs->end(); I != E; ++I) { I->second->onStartOfTranslationUnit(); } } void onEndOfTranslationUnit() { - for (std::vector >::const_iterator - I = MatcherCallbackPairs->begin(), E = MatcherCallbackPairs->end(); + for (std::vector >::const_iterator + I = MatcherCallbackPairs->begin(), + E = MatcherCallbackPairs->end(); I != E; ++I) { I->second->onEndOfTranslationUnit(); } @@ -450,12 +451,13 @@ public: // Matches all registered matchers on the given node and calls the // result callback for every node that matches. void match(const ast_type_traits::DynTypedNode& Node) { - for (std::vector >::const_iterator - I = MatcherCallbackPairs->begin(), E = MatcherCallbackPairs->end(); + for (std::vector >::const_iterator + I = MatcherCallbackPairs->begin(), + E = MatcherCallbackPairs->end(); I != E; ++I) { BoundNodesTreeBuilder Builder; - if (I->first->matches(Node, this, &Builder)) { + if (I->first.matches(Node, this, &Builder)) { MatchVisitor Visitor(ActiveASTContext, I->second); Builder.visitMatches(&Visitor); } @@ -603,8 +605,8 @@ private: return false; } - std::vector > *const MatcherCallbackPairs; + std::vector > *const + MatcherCallbackPairs; ASTContext *ActiveASTContext; // Maps a canonical type to its TypedefDecls. @@ -743,11 +745,10 @@ bool MatchASTVisitor::TraverseNestedNameSpecifierLoc( class MatchASTConsumer : public ASTConsumer { public: MatchASTConsumer( - std::vector > *MatcherCallbackPairs, - MatchFinder::ParsingDoneTestCallback *ParsingDone) - : Visitor(MatcherCallbackPairs), - ParsingDone(ParsingDone) {} + std::vector > * + MatcherCallbackPairs, + MatchFinder::ParsingDoneTestCallback *ParsingDone) + : Visitor(MatcherCallbackPairs), ParsingDone(ParsingDone) {} private: virtual void HandleTranslationUnit(ASTContext &Context) { @@ -778,49 +779,36 @@ MatchFinder::ParsingDoneTestCallback::~ParsingDoneTestCallback() {} MatchFinder::MatchFinder() : ParsingDone(NULL) {} -MatchFinder::~MatchFinder() { - for (std::vector >::const_iterator - It = MatcherCallbackPairs.begin(), End = MatcherCallbackPairs.end(); - It != End; ++It) { - delete It->first; - } -} +MatchFinder::~MatchFinder() {} void MatchFinder::addMatcher(const DeclarationMatcher &NodeMatch, MatchCallback *Action) { - MatcherCallbackPairs.push_back(std::make_pair( - new internal::Matcher(NodeMatch), Action)); + MatcherCallbackPairs.push_back(std::make_pair(NodeMatch, Action)); } void MatchFinder::addMatcher(const TypeMatcher &NodeMatch, MatchCallback *Action) { - MatcherCallbackPairs.push_back(std::make_pair( - new internal::Matcher(NodeMatch), Action)); + MatcherCallbackPairs.push_back(std::make_pair(NodeMatch, Action)); } void MatchFinder::addMatcher(const StatementMatcher &NodeMatch, MatchCallback *Action) { - MatcherCallbackPairs.push_back(std::make_pair( - new internal::Matcher(NodeMatch), Action)); + MatcherCallbackPairs.push_back(std::make_pair(NodeMatch, Action)); } void MatchFinder::addMatcher(const NestedNameSpecifierMatcher &NodeMatch, MatchCallback *Action) { - MatcherCallbackPairs.push_back(std::make_pair( - new NestedNameSpecifierMatcher(NodeMatch), Action)); + MatcherCallbackPairs.push_back(std::make_pair(NodeMatch, Action)); } void MatchFinder::addMatcher(const NestedNameSpecifierLocMatcher &NodeMatch, MatchCallback *Action) { - MatcherCallbackPairs.push_back(std::make_pair( - new NestedNameSpecifierLocMatcher(NodeMatch), Action)); + MatcherCallbackPairs.push_back(std::make_pair(NodeMatch, Action)); } void MatchFinder::addMatcher(const TypeLocMatcher &NodeMatch, MatchCallback *Action) { - MatcherCallbackPairs.push_back(std::make_pair( - new TypeLocMatcher(NodeMatch), Action)); + MatcherCallbackPairs.push_back(std::make_pair(NodeMatch, Action)); } ASTConsumer *MatchFinder::newASTConsumer() { diff --git a/lib/ASTMatchers/ASTMatchersInternal.cpp b/lib/ASTMatchers/ASTMatchersInternal.cpp index e7465167da..d15eb54002 100644 --- a/lib/ASTMatchers/ASTMatchersInternal.cpp +++ b/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -26,25 +26,23 @@ void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) { } } +DynTypedMatcher::MatcherStorage::~MatcherStorage() {} + void BoundNodesTreeBuilder::addMatch(const BoundNodesTreeBuilder &Other) { for (unsigned i = 0, e = Other.Bindings.size(); i != e; ++i) { Bindings.push_back(Other.Bindings[i]); } } -DynTypedMatcher::~DynTypedMatcher() {} - -DynTypedMatcher *DynTypedMatcher::tryBind(StringRef ID) const { return NULL; } - bool AllOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder, - ArrayRef InnerMatchers) { + ArrayRef InnerMatchers) { // allOf leads to one matcher for each alternative in the first // matcher combined with each alternative in the second matcher. // Thus, we can reuse the same Builder. for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) { - if (!InnerMatchers[i]->matches(DynNode, Finder, Builder)) + if (!InnerMatchers[i].matches(DynNode, Finder, Builder)) return false; } return true; @@ -53,12 +51,12 @@ bool AllOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, bool EachOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder, - ArrayRef InnerMatchers) { + ArrayRef InnerMatchers) { BoundNodesTreeBuilder Result; bool Matched = false; for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) { BoundNodesTreeBuilder BuilderInner(*Builder); - if (InnerMatchers[i]->matches(DynNode, Finder, &BuilderInner)) { + if (InnerMatchers[i].matches(DynNode, Finder, &BuilderInner)) { Matched = true; Result.addMatch(BuilderInner); } @@ -70,10 +68,10 @@ bool EachOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, bool AnyOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder, - ArrayRef InnerMatchers) { + ArrayRef InnerMatchers) { for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) { BoundNodesTreeBuilder Result = *Builder; - if (InnerMatchers[i]->matches(DynNode, Finder, &Result)) { + if (InnerMatchers[i].matches(DynNode, Finder, &Result)) { *Builder = Result; return true; } diff --git a/lib/ASTMatchers/Dynamic/Marshallers.h b/lib/ASTMatchers/Dynamic/Marshallers.h index 0df9c8cd60..ae0c300d09 100644 --- a/lib/ASTMatchers/Dynamic/Marshallers.h +++ b/lib/ASTMatchers/Dynamic/Marshallers.h @@ -167,15 +167,13 @@ private: /// out of the polymorphic object. template static void mergePolyMatchers(const PolyMatcher &Poly, - std::vector &Out, + std::vector &Out, ast_matchers::internal::EmptyTypeList) {} template static void mergePolyMatchers(const PolyMatcher &Poly, - std::vector &Out, - TypeList) { - Out.push_back(ast_matchers::internal::Matcher(Poly) - .clone()); + std::vector &Out, TypeList) { + Out.push_back(ast_matchers::internal::Matcher(Poly)); mergePolyMatchers(Poly, Out, typename TypeList::tail()); } @@ -193,10 +191,9 @@ template static VariantMatcher outvalueToVariantMatcher(const T &PolyMatcher, typename T::ReturnTypes * = NULL) { - std::vector Matchers; + std::vector Matchers; mergePolyMatchers(PolyMatcher, Matchers, typename T::ReturnTypes()); VariantMatcher Out = VariantMatcher::PolymorphicMatcher(Matchers); - llvm::DeleteContainerPointers(Matchers); return Out; } diff --git a/lib/ASTMatchers/Dynamic/Parser.cpp b/lib/ASTMatchers/Dynamic/Parser.cpp index 37e6c10478..df9596e9b9 100644 --- a/lib/ASTMatchers/Dynamic/Parser.cpp +++ b/lib/ASTMatchers/Dynamic/Parser.cpp @@ -390,29 +390,29 @@ bool Parser::parseExpression(StringRef Code, Sema *S, return true; } -DynTypedMatcher *Parser::parseMatcherExpression(StringRef Code, - Diagnostics *Error) { +llvm::Optional +Parser::parseMatcherExpression(StringRef Code, Diagnostics *Error) { RegistrySema S; return parseMatcherExpression(Code, &S, Error); } -DynTypedMatcher *Parser::parseMatcherExpression(StringRef Code, - Parser::Sema *S, - Diagnostics *Error) { +llvm::Optional +Parser::parseMatcherExpression(StringRef Code, Parser::Sema *S, + Diagnostics *Error) { VariantValue Value; if (!parseExpression(Code, S, &Value, Error)) - return NULL; + return llvm::Optional(); if (!Value.isMatcher()) { Error->addError(SourceRange(), Error->ET_ParserNotAMatcher); - return NULL; + return llvm::Optional(); } - const DynTypedMatcher *Result; - if (!Value.getMatcher().getSingleMatcher(Result)) { + llvm::Optional Result = + Value.getMatcher().getSingleMatcher(); + if (!Result.hasValue()) { Error->addError(SourceRange(), Error->ET_ParserOverloadedType) << Value.getTypeAsString(); - return NULL; } - return Result->clone(); + return Result; } } // namespace dynamic diff --git a/lib/ASTMatchers/Dynamic/Registry.cpp b/lib/ASTMatchers/Dynamic/Registry.cpp index 3f8e7d4cd9..3a17038d98 100644 --- a/lib/ASTMatchers/Dynamic/Registry.cpp +++ b/lib/ASTMatchers/Dynamic/Registry.cpp @@ -329,10 +329,10 @@ VariantMatcher Registry::constructBoundMatcher(StringRef MatcherName, VariantMatcher Out = constructMatcher(MatcherName, NameRange, Args, Error); if (Out.isNull()) return Out; - const DynTypedMatcher *Result; - if (Out.getSingleMatcher(Result)) { - OwningPtr Bound(Result->tryBind(BindID)); - if (Bound) { + llvm::Optional Result = Out.getSingleMatcher(); + if (Result.hasValue()) { + llvm::Optional Bound = Result->tryBind(BindID); + if (Bound.hasValue()) { return VariantMatcher::SingleMatcher(*Bound); } } diff --git a/lib/ASTMatchers/Dynamic/VariantValue.cpp b/lib/ASTMatchers/Dynamic/VariantValue.cpp index c350d78aa1..3e49e1b8ef 100644 --- a/lib/ASTMatchers/Dynamic/VariantValue.cpp +++ b/lib/ASTMatchers/Dynamic/VariantValue.cpp @@ -26,44 +26,37 @@ VariantMatcher::Payload::~Payload() {} class VariantMatcher::SinglePayload : public VariantMatcher::Payload { public: - SinglePayload(const DynTypedMatcher &Matcher) : Matcher(Matcher.clone()) {} + SinglePayload(const DynTypedMatcher &Matcher) : Matcher(Matcher) {} - virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const { - Out = Matcher.get(); - return true; + virtual llvm::Optional getSingleMatcher() const { + return Matcher; } virtual std::string getTypeAsString() const { - return (Twine("Matcher<") + Matcher->getSupportedKind().asStringRef() + ">") + return (Twine("Matcher<") + Matcher.getSupportedKind().asStringRef() + ">") .str(); } virtual void makeTypedMatcher(MatcherOps &Ops) const { - if (Ops.canConstructFrom(*Matcher)) - Ops.constructFrom(*Matcher); + if (Ops.canConstructFrom(Matcher)) + Ops.constructFrom(Matcher); } private: - OwningPtr Matcher; + const DynTypedMatcher Matcher; }; class VariantMatcher::PolymorphicPayload : public VariantMatcher::Payload { public: - PolymorphicPayload(ArrayRef MatchersIn) { - for (size_t i = 0, e = MatchersIn.size(); i != e; ++i) { - Matchers.push_back(MatchersIn[i]->clone()); - } - } + PolymorphicPayload(ArrayRef MatchersIn) + : Matchers(MatchersIn) {} - virtual ~PolymorphicPayload() { - llvm::DeleteContainerPointers(Matchers); - } + virtual ~PolymorphicPayload() {} - virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const { + virtual llvm::Optional getSingleMatcher() const { if (Matchers.size() != 1) - return false; - Out = Matchers[0]; - return true; + return llvm::Optional(); + return Matchers[0]; } virtual std::string getTypeAsString() const { @@ -71,7 +64,7 @@ public: for (size_t i = 0, e = Matchers.size(); i != e; ++i) { if (i != 0) Inner += "|"; - Inner += Matchers[i]->getSupportedKind().asStringRef(); + Inner += Matchers[i].getSupportedKind().asStringRef(); } return (Twine("Matcher<") + Inner + ">").str(); } @@ -79,17 +72,17 @@ public: virtual void makeTypedMatcher(MatcherOps &Ops) const { const DynTypedMatcher *Found = NULL; for (size_t i = 0, e = Matchers.size(); i != e; ++i) { - if (Ops.canConstructFrom(*Matchers[i])) { + if (Ops.canConstructFrom(Matchers[i])) { if (Found) return; - Found = Matchers[i]; + Found = &Matchers[i]; } } if (Found) Ops.constructFrom(*Found); } - std::vector Matchers; + const std::vector Matchers; }; class VariantMatcher::VariadicOpPayload : public VariantMatcher::Payload { @@ -98,8 +91,8 @@ public: ArrayRef Args) : Func(Func), Args(Args) {} - virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const { - return false; + virtual llvm::Optional getSingleMatcher() const { + return llvm::Optional(); } virtual std::string getTypeAsString() const { @@ -128,7 +121,7 @@ VariantMatcher VariantMatcher::SingleMatcher(const DynTypedMatcher &Matcher) { } VariantMatcher -VariantMatcher::PolymorphicMatcher(ArrayRef Matchers) { +VariantMatcher::PolymorphicMatcher(ArrayRef Matchers) { return VariantMatcher(new PolymorphicPayload(Matchers)); } @@ -138,9 +131,8 @@ VariantMatcher VariantMatcher::VariadicOperatorMatcher( return VariantMatcher(new VariadicOpPayload(Func, Args)); } -bool VariantMatcher::getSingleMatcher(const DynTypedMatcher *&Out) const { - if (Value) return Value->getSingleMatcher(Out); - return false; +llvm::Optional VariantMatcher::getSingleMatcher() const { + return Value ? Value->getSingleMatcher() : llvm::Optional(); } void VariantMatcher::reset() { Value.reset(); } diff --git a/unittests/ASTMatchers/Dynamic/ParserTest.cpp b/unittests/ASTMatchers/Dynamic/ParserTest.cpp index 9116ab8a9f..f19ec51ad7 100644 --- a/unittests/ASTMatchers/Dynamic/ParserTest.cpp +++ b/unittests/ASTMatchers/Dynamic/ParserTest.cpp @@ -21,51 +21,14 @@ namespace ast_matchers { namespace dynamic { namespace { -class DummyDynTypedMatcher : public DynTypedMatcher { -public: - DummyDynTypedMatcher(uint64_t ID) : ID(ID) {} - DummyDynTypedMatcher(uint64_t ID, StringRef BoundID) - : ID(ID), BoundID(BoundID) {} - - typedef ast_matchers::internal::ASTMatchFinder ASTMatchFinder; - typedef ast_matchers::internal::BoundNodesTreeBuilder BoundNodesTreeBuilder; - virtual bool matches(const ast_type_traits::DynTypedNode DynNode, - ASTMatchFinder *Finder, - BoundNodesTreeBuilder *Builder) const { - return false; - } - - /// \brief Makes a copy of this matcher object. - virtual DynTypedMatcher *clone() const { - return new DummyDynTypedMatcher(*this); - } - - /// \brief Returns a unique ID for the matcher. - virtual uint64_t getID() const { return ID; } - - virtual DynTypedMatcher* tryBind(StringRef BoundID) const { - return new DummyDynTypedMatcher(ID, BoundID); - } - - StringRef boundID() const { return BoundID; } - - virtual ast_type_traits::ASTNodeKind getSupportedKind() const { - return ast_type_traits::ASTNodeKind(); - } - -private: - uint64_t ID; - std::string BoundID; -}; - class MockSema : public Parser::Sema { public: virtual ~MockSema() {} uint64_t expectMatcher(StringRef MatcherName) { - uint64_t ID = ExpectedMatchers.size() + 1; - ExpectedMatchers[MatcherName] = ID; - return ID; + ast_matchers::internal::Matcher M = stmt(); + ExpectedMatchers.insert(std::make_pair(MatcherName, M)); + return M.getID(); } void parse(StringRef Code) { @@ -83,9 +46,8 @@ public: Diagnostics *Error) { MatcherInfo ToStore = { MatcherName, NameRange, Args, BindID }; Matchers.push_back(ToStore); - DummyDynTypedMatcher Matcher(ExpectedMatchers[MatcherName]); - OwningPtr Out(Matcher.tryBind(BindID)); - return VariantMatcher::SingleMatcher(*Out); + return VariantMatcher::SingleMatcher( + ExpectedMatchers.find(MatcherName)->second); } struct MatcherInfo { @@ -98,7 +60,8 @@ public: std::vector Errors; std::vector Values; std::vector Matchers; - llvm::StringMap ExpectedMatchers; + std::map > + ExpectedMatchers; }; TEST(ParserTest, ParseUnsigned) { @@ -137,10 +100,11 @@ bool matchesRange(const SourceRange &Range, unsigned StartLine, Range.Start.Column == StartColumn && Range.End.Column == EndColumn; } -const DynTypedMatcher *getSingleMatcher(const VariantValue &Value) { - const DynTypedMatcher *Out; - EXPECT_TRUE(Value.getMatcher().getSingleMatcher(Out)); - return Out; +llvm::Optional getSingleMatcher(const VariantValue &Value) { + llvm::Optional Result = + Value.getMatcher().getSingleMatcher(); + EXPECT_TRUE(Result.hasValue()); + return Result; } TEST(ParserTest, ParseMatcher) { @@ -155,8 +119,6 @@ TEST(ParserTest, ParseMatcher) { EXPECT_EQ(1ULL, Sema.Values.size()); EXPECT_EQ(ExpectedFoo, getSingleMatcher(Sema.Values[0])->getID()); - EXPECT_EQ("Yo!", static_cast( - getSingleMatcher(Sema.Values[0]))->boundID()); EXPECT_EQ(3ULL, Sema.Matchers.size()); const MockSema::MatcherInfo Bar = Sema.Matchers[0]; @@ -184,27 +146,28 @@ using ast_matchers::internal::Matcher; TEST(ParserTest, FullParserTest) { Diagnostics Error; - OwningPtr VarDecl(Parser::parseMatcherExpression( + llvm::Optional VarDecl(Parser::parseMatcherExpression( "varDecl(hasInitializer(binaryOperator(hasLHS(integerLiteral())," " hasOperatorName(\"+\"))))", &Error)); EXPECT_EQ("", Error.toStringFull()); - Matcher M = Matcher::constructFrom(*VarDecl); + Matcher M = VarDecl->unconditionalConvertTo(); EXPECT_TRUE(matches("int x = 1 + false;", M)); EXPECT_FALSE(matches("int x = true + 1;", M)); EXPECT_FALSE(matches("int x = 1 - false;", M)); EXPECT_FALSE(matches("int x = true - 1;", M)); - OwningPtr HasParameter(Parser::parseMatcherExpression( + llvm::Optional HasParameter(Parser::parseMatcherExpression( "functionDecl(hasParameter(1, hasName(\"x\")))", &Error)); EXPECT_EQ("", Error.toStringFull()); - M = Matcher::constructFrom(*HasParameter); + M = HasParameter->unconditionalConvertTo(); EXPECT_TRUE(matches("void f(int a, int x);", M)); EXPECT_FALSE(matches("void f(int x, int a);", M)); - EXPECT_TRUE(Parser::parseMatcherExpression( - "hasInitializer(\n binaryOperator(hasLHS(\"A\")))", &Error) == NULL); + EXPECT_TRUE(!Parser::parseMatcherExpression( + "hasInitializer(\n binaryOperator(hasLHS(\"A\")))", + &Error).hasValue()); EXPECT_EQ("1:1: Error parsing argument 1 for matcher hasInitializer.\n" "2:5: Error parsing argument 1 for matcher binaryOperator.\n" "2:20: Error building matcher hasLHS.\n" -- 2.40.0