From e136c9dba2063de9b90da23b9a9807257a91b4f9 Mon Sep 17 00:00:00 2001
From: Samuel Benzaquen <sbenza@google.com>
Date: Thu, 9 Oct 2014 19:28:18 +0000
Subject: [PATCH] Special case 0 and 1 matcher in makeAllOfComposite().
Summary:
Remove unnecessary wrapping for the 0 and 1 matcher cases of
makeAllOfComposite(). We don't need a variadic wrapper for those cases.
Refactor TrueMatcher to take advandage of the new conversions between
DynTypedMatcher and Matcher<T>. Also, make it a singleton.
This change improves our clang-tidy related benchmarks by ~12%.
Reviewers: klimek
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D5675
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@219431 91177308-0d34-0410-b5e6-96231b3b80d8
---
include/clang/ASTMatchers/ASTMatchers.h | 5 +-
.../clang/ASTMatchers/ASTMatchersInternal.h | 83 +++++++++----------
lib/ASTMatchers/ASTMatchersInternal.cpp | 25 ++++++
unittests/ASTMatchers/Dynamic/ParserTest.cpp | 5 +-
4 files changed, 68 insertions(+), 50 deletions(-)
diff --git a/include/clang/ASTMatchers/ASTMatchers.h b/include/clang/ASTMatchers/ASTMatchers.h
index 7d268ead2b0..28197664839 100644
--- a/include/clang/ASTMatchers/ASTMatchers.h
+++ b/include/clang/ASTMatchers/ASTMatchers.h
@@ -140,10 +140,7 @@ typedef internal::Matcher<NestedNameSpecifierLoc> NestedNameSpecifierLocMatcher;
/// \endcode
///
/// Usable as: Any Matcher
-inline internal::PolymorphicMatcherWithParam0<internal::TrueMatcher>
-anything() {
- return internal::PolymorphicMatcherWithParam0<internal::TrueMatcher>();
-}
+inline internal::TrueMatcher anything() { return internal::TrueMatcher(); }
/// \brief Matches declarations.
///
diff --git a/include/clang/ASTMatchers/ASTMatchersInternal.h b/include/clang/ASTMatchers/ASTMatchersInternal.h
index f00277c1a61..5c991e2a3b8 100644
--- a/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -270,6 +270,11 @@ public:
constructVariadic(VariadicOperatorFunction Func,
std::vector<DynTypedMatcher> 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 @@ public:
template <typename T> Matcher<T> unconditionalConvertTo() const;
private:
+ DynTypedMatcher(ast_type_traits::ASTNodeKind SupportedKind,
+ ast_type_traits::ASTNodeKind RestrictKind,
+ IntrusiveRefCntPtr<DynMatcherInterface> 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 @@ public:
};
private:
+ // For Matcher<T> <=> Matcher<U> conversions.
template <typename U> friend class Matcher;
+ // For DynTypedMatcher::unconditionalConvertTo<T>.
+ friend class DynTypedMatcher;
static DynTypedMatcher restrictMatcher(const DynTypedMatcher &Other) {
return Other.dynCastTo(ast_type_traits::ASTNodeKind::getFromNodeKind<T>());
@@ -982,46 +998,16 @@ private:
///
/// 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 <typename T>
-class TrueMatcher : public SingleNodeMatcherInterface<T> {
-public:
- bool matchesNode(const T &Node) const override {
- return true;
- }
-};
-
-/// \brief Matcher<T> that wraps an inner Matcher<T> and binds the matched node
-/// to an ID if the inner matcher matches on the node.
-template <typename T>
-class IdMatcher : public MatcherInterface<T> {
-public:
- /// \brief Creates an IdMatcher that binds to 'ID' if 'InnerMatcher' matches
- /// the node.
- IdMatcher(StringRef ID, const Matcher<T> &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 <typename T>
+ operator Matcher<T>() const {
+ return DynTypedMatcher::trueMatcher(
+ ast_type_traits::ASTNodeKind::getFromNodeKind<T>())
+ .template unconditionalConvertTo<T>();
}
-
-private:
- const std::string ID;
- const Matcher<T> InnerMatcher;
};
/// \brief A Matcher that allows binding the node it matches to an id.
@@ -1040,9 +1026,9 @@ public:
/// The returned matcher is equivalent to this matcher, but will
/// bind the matched node on a match.
Matcher<T> bind(StringRef ID) const {
- // FIXME: Use DynTypedMatcher's IdMatcher instead. No need for a template
- // version anymore.
- return Matcher<T>(new IdMatcher<T>(ID, *this));
+ return DynTypedMatcher(*this)
+ .tryBind(ID)
+ ->template unconditionalConvertTo<T>();
}
/// \brief Same as Matcher<T>'s conversion operator, but enables binding on
@@ -1315,16 +1301,23 @@ bool AnyOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode,
template <typename T>
inline Matcher<T> DynTypedMatcher::unconditionalConvertTo() const {
- // FIXME: Remove this extra indirection and connect directly to Matcher<T>().
- return Matcher<T>(new VariadicOperatorMatcherInterface<T>(
- AllOfVariadicOperator, llvm::makeArrayRef(*this)));
+ return Matcher<T>(*this);
}
/// \brief Creates a Matcher<T> that matches if all inner matchers match.
template<typename T>
BindableMatcher<T> makeAllOfComposite(
ArrayRef<const Matcher<T> *> 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<T>(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<T>(*InnerMatchers[0]);
+ }
+
std::vector<DynTypedMatcher> DynMatchers;
for (size_t i = 0, e = InnerMatchers.size(); i != e; ++i) {
DynMatchers.push_back(*InnerMatchers[i]);
diff --git a/lib/ASTMatchers/ASTMatchersInternal.cpp b/lib/ASTMatchers/ASTMatchersInternal.cpp
index ec60f0d9498..f6fd5ba2a3f 100644
--- a/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/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 @@ class IdDynMatcher : public DynMatcherInterface {
const IntrusiveRefCntPtr<DynMatcherInterface> 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<DynMatcherInterface> Instance;
+
+ class Impl : public DynMatcherInterface {
+ public:
+ bool dynMatches(const ast_type_traits::DynTypedNode &, ASTMatchFinder *,
+ BoundNodesTreeBuilder *) const override {
+ return true;
+ }
+ };
+};
+static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance;
+
} // namespace
DynTypedMatcher DynTypedMatcher::constructVariadic(
@@ -83,6 +103,11 @@ DynTypedMatcher DynTypedMatcher::constructVariadic(
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;
diff --git a/unittests/ASTMatchers/Dynamic/ParserTest.cpp b/unittests/ASTMatchers/Dynamic/ParserTest.cpp
index ed507d57333..2a9a61b543d 100644
--- a/unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ b/unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -26,7 +26,10 @@ public:
virtual ~MockSema() {}
uint64_t expectMatcher(StringRef MatcherName) {
- ast_matchers::internal::Matcher<Stmt> 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<Stmt> M = stmt(stmt(), stmt());
ExpectedMatchers.insert(std::make_pair(MatcherName, M));
return M.getID().second;
}
--
GitLab