From b7488d77414b000ce2506b520a6b29f845fb3950 Mon Sep 17 00:00:00 2001 From: Samuel Benzaquen Date: Tue, 29 Oct 2013 14:37:15 +0000 Subject: [PATCH] Resubmit "Refactor DynTypedMatcher into a value type class, just like Matcher." Summary: This resubmits r193100, plus a fix for a breakage with MSVC. Reviewers: klimek, rnk CC: cfe-commits, revane Differential Revision: http://llvm-reviews.chandlerc.com/D2005 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@193613 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/ASTMatchers/ASTMatchFinder.h | 2 +- .../clang/ASTMatchers/ASTMatchersInternal.h | 325 ++++++++++-------- 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, 305 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..3d078259e0 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,168 @@ 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 inline DynTypedMatcher(const Matcher &M); + + /// \brief Construct from a bindable \c Matcher. Copies the matcher. + /// + /// This version enables \c tryBind() on the \c DynTypedMatcher. + template inline DynTypedMatcher(const BindableMatcher &M); + + /// \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; + }; + + /// \brief Typed implementation of \c MatcherStorage. + template class TypedMatcherStorage; + + /// \brief Simple MatcherInterface wrapper around a DynTypedMatcher. + template class WrappedMatcher; + + IntrusiveRefCntPtr Storage; +}; + +template +class DynTypedMatcher::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; +}; + +template +inline DynTypedMatcher::DynTypedMatcher(const Matcher &M) + : Storage(new TypedMatcherStorage(M, false)) {} + +template +inline DynTypedMatcher::DynTypedMatcher(const BindableMatcher &M) + : Storage(new TypedMatcherStorage(M, true)) {} + +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 +477,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 +1031,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 +1123,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 +1133,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 +1145,7 @@ public: private: const VariadicOperatorFunction Func; - std::vector InnerMatchers; + std::vector InnerMatchers; }; /// \brief "No argument" placeholder to use as template paratemers. @@ -1196,24 +1239,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 +1275,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