Index: include/clang/ASTMatchers/ASTMatchers.h =================================================================== --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -140,10 +140,7 @@ /// \endcode /// /// Usable as: Any Matcher -inline internal::PolymorphicMatcherWithParam0 -anything() { - return internal::PolymorphicMatcherWithParam0(); -} +inline internal::TrueMatcher anything() { return internal::TrueMatcher(); } /// \brief Matches declarations. /// Index: include/clang/ASTMatchers/ASTMatchersInternal.h =================================================================== --- include/clang/ASTMatchers/ASTMatchersInternal.h +++ include/clang/ASTMatchers/ASTMatchersInternal.h @@ -270,6 +270,11 @@ constructVariadic(VariadicOperatorFunction Func, std::vector InnerMatchers); + /// \brief Get a "true" matcher for \p NodeKind. + /// + /// It only checks that the node is of the right kind. + static DynTypedMatcher trueMatcher(ast_type_traits::ASTNodeKind NodeKind); + void setAllowBind(bool AB) { AllowBind = AB; } /// \brief Return a matcher that points to the same implementation, but @@ -335,6 +340,14 @@ template Matcher unconditionalConvertTo() const; private: + DynTypedMatcher(ast_type_traits::ASTNodeKind SupportedKind, + ast_type_traits::ASTNodeKind RestrictKind, + IntrusiveRefCntPtr Implementation) + : AllowBind(false), + SupportedKind(SupportedKind), + RestrictKind(RestrictKind), + Implementation(std::move(Implementation)) {} + bool AllowBind; ast_type_traits::ASTNodeKind SupportedKind; /// \brief A potentially stricter node kind. @@ -433,7 +446,10 @@ }; private: + // For Matcher <=> Matcher conversions. template friend class Matcher; + // For DynTypedMatcher::unconditionalConvertTo. + friend class DynTypedMatcher; static DynTypedMatcher restrictMatcher(const DynTypedMatcher &Other) { return Other.dynCastTo(ast_type_traits::ASTNodeKind::getFromNodeKind()); @@ -982,46 +998,16 @@ /// /// This is useful when a matcher syntactically requires a child matcher, /// but the context doesn't care. See for example: anything(). -/// -/// FIXME: Alternatively we could also create a IsAMatcher or something -/// that checks that a dyn_cast is possible. This is purely needed for the -/// difference between calling for example: -/// record() -/// and -/// record(SomeMatcher) -/// In the second case we need the correct type we were dyn_cast'ed to in order -/// to get the right type for the inner matcher. In the first case we don't need -/// that, but we use the type conversion anyway and insert a TrueMatcher. -template -class TrueMatcher : public SingleNodeMatcherInterface { -public: - bool matchesNode(const T &Node) const override { - return true; - } -}; - -/// \brief Matcher that wraps an inner Matcher and binds the matched node -/// to an ID if the inner matcher matches on the node. -template -class IdMatcher : public MatcherInterface { -public: - /// \brief Creates an IdMatcher that binds to 'ID' if 'InnerMatcher' matches - /// the node. - IdMatcher(StringRef ID, const Matcher &InnerMatcher) - : ID(ID), InnerMatcher(InnerMatcher) {} +class TrueMatcher { + public: + typedef AllNodeBaseTypes ReturnTypes; - bool matches(const T &Node, ASTMatchFinder *Finder, - BoundNodesTreeBuilder *Builder) const override { - bool Result = InnerMatcher.matches(Node, Finder, Builder); - if (Result) { - Builder->setBinding(ID, ast_type_traits::DynTypedNode::create(Node)); - } - return Result; + template + operator Matcher() const { + return DynTypedMatcher::trueMatcher( + ast_type_traits::ASTNodeKind::getFromNodeKind()) + .template unconditionalConvertTo(); } - -private: - const std::string ID; - const Matcher InnerMatcher; }; /// \brief A Matcher that allows binding the node it matches to an id. @@ -1040,9 +1026,9 @@ /// The returned matcher is equivalent to this matcher, but will /// bind the matched node on a match. Matcher bind(StringRef ID) const { - // FIXME: Use DynTypedMatcher's IdMatcher instead. No need for a template - // version anymore. - return Matcher(new IdMatcher(ID, *this)); + return DynTypedMatcher(*this) + .tryBind(ID) + ->template unconditionalConvertTo(); } /// \brief Same as Matcher's conversion operator, but enables binding on @@ -1315,16 +1301,23 @@ template inline Matcher DynTypedMatcher::unconditionalConvertTo() const { - // FIXME: Remove this extra indirection and connect directly to Matcher(). - return Matcher(new VariadicOperatorMatcherInterface( - AllOfVariadicOperator, llvm::makeArrayRef(*this))); + return Matcher(*this); } /// \brief Creates a Matcher that matches if all inner matchers match. template BindableMatcher makeAllOfComposite( ArrayRef *> InnerMatchers) { - // FIXME: Optimize for the cases of size()==0 and size()==1 + // For the size() == 0 case, we return a "true" matcher. + if (InnerMatchers.size() == 0) { + return BindableMatcher(TrueMatcher()); + } + // For the size() == 1 case, we simply return that one matcher. + // No need to wrap it in a variadic operation. + if (InnerMatchers.size() == 1) { + return BindableMatcher(*InnerMatchers[0]); + } + std::vector DynMatchers; for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) { DynMatchers.push_back(*InnerMatchers[i]); Index: lib/ASTMatchers/ASTMatchersInternal.cpp =================================================================== --- lib/ASTMatchers/ASTMatchersInternal.cpp +++ lib/ASTMatchers/ASTMatchersInternal.cpp @@ -13,6 +13,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchersInternal.h" +#include "llvm/Support/ManagedStatic.h" namespace clang { namespace ast_matchers { @@ -64,6 +65,25 @@ const IntrusiveRefCntPtr InnerMatcher; }; +/// \brief A matcher that always returns true. +/// +/// We only ever need one instance of this matcher, so we create a global one +/// and reuse it to reduce the overhead of the matcher and increase the chance +/// of cache hits. +struct TrueMatcherImpl { + TrueMatcherImpl() : Instance(new Impl) {} + const IntrusiveRefCntPtr Instance; + + class Impl : public DynMatcherInterface { + public: + bool dynMatches(const ast_type_traits::DynTypedNode &, ASTMatchFinder *, + BoundNodesTreeBuilder *) const override { + return true; + } + }; +}; +static llvm::ManagedStatic TrueMatcherInstance; + } // namespace DynTypedMatcher DynTypedMatcher::constructVariadic( @@ -83,6 +103,11 @@ return Result; } +DynTypedMatcher DynTypedMatcher::trueMatcher( + ast_type_traits::ASTNodeKind NodeKind) { + return DynTypedMatcher(NodeKind, NodeKind, TrueMatcherInstance->Instance); +} + DynTypedMatcher DynTypedMatcher::dynCastTo( const ast_type_traits::ASTNodeKind Kind) const { auto Copy = *this; Index: unittests/ASTMatchers/Dynamic/ParserTest.cpp =================================================================== --- unittests/ASTMatchers/Dynamic/ParserTest.cpp +++ unittests/ASTMatchers/Dynamic/ParserTest.cpp @@ -26,7 +26,10 @@ virtual ~MockSema() {} uint64_t expectMatcher(StringRef MatcherName) { - ast_matchers::internal::Matcher M = stmt(); + // Optimizations on the matcher framework make simple matchers like + // 'stmt()' to be all the same matcher. + // Use a more complex expression to prevent that. + ast_matchers::internal::Matcher M = stmt(stmt(), stmt()); ExpectedMatchers.insert(std::make_pair(MatcherName, M)); return M.getID().second; }