From 8513d6234d00cae96811fb07fc71e8000839b7ba Mon Sep 17 00:00:00 2001 From: Samuel Benzaquen Date: Wed, 15 Oct 2014 14:58:46 +0000 Subject: [PATCH] Speed up hasName() matcher. Summary: Speed up hasName() matcher by skipping the expensive generation of the fully qualified name unless we need it. In the common case of matching an unqualified name, we don't need to generate the full name. We might not even need to copy any string at all. This change speeds up our clang-tidy benchmark by ~10% Reviewers: klimek Subscribers: klimek, cfe-commits Differential Revision: http://reviews.llvm.org/D5776 llvm-svn: 219792 --- clang/include/clang/ASTMatchers/ASTMatchers.h | 12 +----- .../clang/ASTMatchers/ASTMatchersInternal.h | 27 +++++++++++++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp | 46 ++++++++++++++++++++++ 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h index 28197664..335e91a 100644 --- a/clang/include/clang/ASTMatchers/ASTMatchers.h +++ b/clang/include/clang/ASTMatchers/ASTMatchers.h @@ -1591,16 +1591,8 @@ inline internal::Matcher sizeOfExpr( /// \code /// namespace a { namespace b { class X; } } /// \endcode -AST_MATCHER_P(NamedDecl, hasName, std::string, Name) { - assert(!Name.empty()); - const std::string FullNameString = "::" + Node.getQualifiedNameAsString(); - const StringRef FullName = FullNameString; - const StringRef Pattern = Name; - if (Pattern.startswith("::")) { - return FullName == Pattern; - } else { - return FullName.endswith(("::" + Pattern).str()); - } +inline internal::Matcher hasName(StringRef Name) { + return internal::Matcher(new internal::HasNameMatcher(Name)); } /// \brief Matches NamedDecl nodes whose fully qualified names contain diff --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h index 5c991e2..d250a8a 100644 --- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h +++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h @@ -574,6 +574,33 @@ private: std::string Name; }; +/// \brief Matches named declarations with a specific name. +/// +/// See \c hasName() in ASTMatchers.h for details. +class HasNameMatcher : public SingleNodeMatcherInterface { + public: + explicit HasNameMatcher(StringRef Name); + + bool matchesNode(const NamedDecl &Node) const override; + + private: + /// \brief Unqualified match routine. + /// + /// It is much faster than the full match, but it only works for unqualified + /// matches. + bool matchesNodeUnqualified(const NamedDecl &Node) const; + + /// \brief Full match routine + /// + /// It generates the fully qualified name of the declaration (which is + /// expensive) before trying to match. + /// It is slower but simple and works on all cases. + bool matchesNodeFull(const NamedDecl &Node) const; + + const bool UseUnqualifiedMatch; + const std::string Name; +}; + /// \brief Matches declarations for QualType and CallExpr. /// /// Type argument DeclMatcherT is required by PolymorphicMatcherWithParam1 but diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp index 9d77cb6..b5d4b52 100644 --- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp +++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -13,6 +13,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchersInternal.h" +#include "llvm/ADT/SmallString.h" #include "llvm/Support/ManagedStatic.h" namespace clang { @@ -221,6 +222,51 @@ bool AnyOfVariadicOperator(const ast_type_traits::DynTypedNode DynNode, return false; } +HasNameMatcher::HasNameMatcher(StringRef NameRef) + : UseUnqualifiedMatch(NameRef.find("::") == NameRef.npos), Name(NameRef) { + assert(!Name.empty()); +} + +bool HasNameMatcher::matchesNodeUnqualified(const NamedDecl &Node) const { + assert(UseUnqualifiedMatch); + if (Node.getIdentifier()) { + // Simple name. + return Name == Node.getName(); + } else if (Node.getDeclName()) { + // Name needs to be constructed. + llvm::SmallString<128> NodeName; + llvm::raw_svector_ostream OS(NodeName); + Node.printName(OS); + return Name == OS.str(); + } else { + return false; + } +} + +bool HasNameMatcher::matchesNodeFull(const NamedDecl &Node) const { + llvm::SmallString<128> NodeName = StringRef("::"); + llvm::raw_svector_ostream OS(NodeName); + Node.printQualifiedName(OS); + const StringRef FullName = OS.str(); + const StringRef Pattern = Name; + if (Pattern.startswith("::")) { + return FullName == Pattern; + } else { + return FullName.endswith(("::" + Pattern).str()); + } +} + +bool HasNameMatcher::matchesNode(const NamedDecl &Node) const { + // FIXME: There is still room for improvement, but it would require copying a + // lot of the logic from NamedDecl::printQualifiedName(). The benchmarks do + // not show like that extra complexity is needed right now. + if (UseUnqualifiedMatch) { + assert(matchesNodeUnqualified(Node) == matchesNodeFull(Node)); + return matchesNodeUnqualified(Node); + } + return matchesNodeFull(Node); +} + } // end namespace internal } // end namespace ast_matchers } // end namespace clang -- 2.7.4