jcking1034 updated this revision to Diff 389620. jcking1034 marked 3 inline comments as done. jcking1034 added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1.
Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113917/new/ https://reviews.llvm.org/D113917 Files: clang/include/clang/ASTMatchers/ASTMatchersInternal.h clang/include/clang/ASTMatchers/ASTMatchersMacros.h clang/lib/ASTMatchers/ASTMatchersInternal.cpp clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp =================================================================== --- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp +++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp @@ -20,6 +20,30 @@ namespace ast_matchers { using internal::DynTypedMatcher; +TEST(GetNameTest, ReturnsCorrectName) { + // Node + EXPECT_EQ(typeLoc().getName(), "`TypeLoc` Node"); + EXPECT_EQ(typeLoc(pointerTypeLoc()).getName(), "`TypeLoc` Node"); + EXPECT_EQ(pointerTypeLoc(hasPointeeLoc(typeLoc())).getName(), + "`PointerTypeLoc` Node"); + EXPECT_EQ(varDecl(hasInitializer(expr()), hasTypeLoc(typeLoc())).getName(), + "`VarDecl` Node"); + EXPECT_EQ(ompDefaultClause().getName(), "`OMPDefaultClause` Node"); + + // Narrowing + EXPECT_EQ(isConst().getName(), "isConst"); + + // Traversal + EXPECT_EQ(hasLoopVariable(varDecl()).getName(), "hasLoopVariable"); + + // Polymorphic + EXPECT_EQ(internal::Matcher<BinaryOperator>(hasLHS(expr())).getName(), + "hasLHS"); + + // Bound + EXPECT_EQ((typeLoc().bind("TL")).getName(), "`TypeLoc` Node"); +} + #if GTEST_HAS_DEATH_TEST TEST(HasNameDeathTest, DiesOnEmptyName) { ASSERT_DEBUG_DEATH({ Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp =================================================================== --- clang/lib/ASTMatchers/ASTMatchersInternal.cpp +++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -115,15 +115,20 @@ template <VariadicOperatorFunction Func> class VariadicMatcher : public DynMatcherInterface { public: - VariadicMatcher(std::vector<DynTypedMatcher> InnerMatchers) - : InnerMatchers(std::move(InnerMatchers)) {} + VariadicMatcher(std::string MatcherName, + std::vector<DynTypedMatcher> InnerMatchers) + : MatcherName(std::move(MatcherName)), + InnerMatchers(std::move(InnerMatchers)) {} bool dynMatches(const DynTypedNode &DynNode, ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const override { return Func(DynNode, Finder, Builder, InnerMatchers); } + std::string getName() const override { return MatcherName; } + private: + const std::string MatcherName; std::vector<DynTypedMatcher> InnerMatchers; }; @@ -144,11 +149,40 @@ return InnerMatcher->TraversalKind(); } + std::string getName() const override { return InnerMatcher->getName(); } + private: const std::string ID; const IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher; }; +/// A matcher that specifies a particular name. +/// +/// The name provided to the constructor overrides any name that may be +/// specified by the `InnerMatcher`. +class NameMatcherImpl : public DynMatcherInterface { +public: + NameMatcherImpl(std::string MatcherName, + IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher) + : MatcherName(std::move(MatcherName)), + InnerMatcher(std::move(InnerMatcher)) {} + + bool dynMatches(const DynTypedNode &DynNode, ASTMatchFinder *Finder, + BoundNodesTreeBuilder *Builder) const override { + return InnerMatcher->dynMatches(DynNode, Finder, Builder); + } + + std::string getName() const override { return MatcherName; } + + llvm::Optional<clang::TraversalKind> TraversalKind() const override { + return InnerMatcher->TraversalKind(); + } + +private: + const std::string MatcherName; + const IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher; +}; + /// A matcher that always returns true. class TrueMatcherImpl : public DynMatcherInterface { public: @@ -158,6 +192,8 @@ BoundNodesTreeBuilder *) const override { return true; } + + std::string getName() const override { return "TrueMatcher"; } }; /// A matcher that specifies a particular \c TraversalKind. @@ -180,6 +216,8 @@ return TK; } + std::string getName() const override { return InnerMatcher->getName(); } + private: clang::TraversalKind TK; IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher; @@ -219,31 +257,31 @@ RestrictKind = ASTNodeKind::getMostDerivedType(RestrictKind, IM.RestrictKind); } - return DynTypedMatcher( - SupportedKind, RestrictKind, - new VariadicMatcher<allOfVariadicOperator>(std::move(InnerMatchers))); + return DynTypedMatcher(SupportedKind, RestrictKind, + new VariadicMatcher<allOfVariadicOperator>( + "allOf", std::move(InnerMatchers))); case VO_AnyOf: - return DynTypedMatcher( - SupportedKind, RestrictKind, - new VariadicMatcher<anyOfVariadicOperator>(std::move(InnerMatchers))); + return DynTypedMatcher(SupportedKind, RestrictKind, + new VariadicMatcher<anyOfVariadicOperator>( + "anyOf", std::move(InnerMatchers))); case VO_EachOf: - return DynTypedMatcher( - SupportedKind, RestrictKind, - new VariadicMatcher<eachOfVariadicOperator>(std::move(InnerMatchers))); + return DynTypedMatcher(SupportedKind, RestrictKind, + new VariadicMatcher<eachOfVariadicOperator>( + "eachOf", std::move(InnerMatchers))); case VO_Optionally: return DynTypedMatcher(SupportedKind, RestrictKind, new VariadicMatcher<optionallyVariadicOperator>( - std::move(InnerMatchers))); + "optionally", std::move(InnerMatchers))); case VO_UnaryNot: // FIXME: Implement the Not operator to take a single matcher instead of a // vector. return DynTypedMatcher( SupportedKind, RestrictKind, - new VariadicMatcher<notUnaryOperator>(std::move(InnerMatchers))); + new VariadicMatcher<notUnaryOperator>("not", std::move(InnerMatchers))); } llvm_unreachable("Invalid Op value."); } @@ -263,6 +301,14 @@ return Copy; } +DynTypedMatcher +DynTypedMatcher::withMatcherName(std::string MatcherName) const { + auto Copy = *this; + Copy.Implementation = new NameMatcherImpl(std::move(MatcherName), + std::move(Copy.Implementation)); + return Copy; +} + DynTypedMatcher DynTypedMatcher::trueMatcher(ASTNodeKind NodeKind) { // We only ever need one instance of TrueMatcherImpl, so we create a static // instance and reuse it to reduce the overhead of the matcher and increase Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h =================================================================== --- clang/include/clang/ASTMatchers/ASTMatchersMacros.h +++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h @@ -101,6 +101,7 @@ ::clang::ast_matchers::internal::ASTMatchFinder *Finder, \ ::clang::ast_matchers::internal::BoundNodesTreeBuilder \ *Builder) const override; \ + std::string getName() const override { return #DefineMatcher; } \ }; \ } \ inline ::clang::ast_matchers::internal::Matcher<Type> DefineMatcher() { \ @@ -141,6 +142,7 @@ ::clang::ast_matchers::internal::ASTMatchFinder *Finder, \ ::clang::ast_matchers::internal::BoundNodesTreeBuilder \ *Builder) const override; \ + std::string getName() const override { return #DefineMatcher; } \ \ private: \ ParamType Param; \ @@ -190,6 +192,7 @@ ::clang::ast_matchers::internal::ASTMatchFinder *Finder, \ ::clang::ast_matchers::internal::BoundNodesTreeBuilder \ *Builder) const override; \ + std::string getName() const override { return #DefineMatcher; } \ \ private: \ ParamType1 Param1; \ @@ -237,6 +240,7 @@ ::clang::ast_matchers::internal::ASTMatchFinder *Finder, \ ::clang::ast_matchers::internal::BoundNodesTreeBuilder \ *Builder) const override; \ + std::string getName() const override { return #DefineMatcher; } \ }; \ } \ inline ::clang::ast_matchers::internal::PolymorphicMatcher< \ @@ -279,6 +283,7 @@ ::clang::ast_matchers::internal::ASTMatchFinder *Finder, \ ::clang::ast_matchers::internal::BoundNodesTreeBuilder \ *Builder) const override; \ + std::string getName() const override { return #DefineMatcher; } \ \ private: \ ParamType Param; \ @@ -331,6 +336,7 @@ ::clang::ast_matchers::internal::ASTMatchFinder *Finder, \ ::clang::ast_matchers::internal::BoundNodesTreeBuilder \ *Builder) const override; \ + std::string getName() const override { return #DefineMatcher; } \ \ private: \ ParamType1 Param1; \ Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h =================================================================== --- clang/include/clang/ASTMatchers/ASTMatchersInternal.h +++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h @@ -83,6 +83,94 @@ namespace internal { +/// Helper function that obtains a name for a matcher based on its type, which +/// can be useful for cases like debugging matchers. This function template is +/// specialized below to cover most of the AST, so the default string returned +/// here should rarely be used. +template <typename T> std::string makeMatcherNameFromType() { + return "Matcher<T>"; +} + +#define TYPELOC(CLASS, PARENT) \ + template <> inline std::string makeMatcherNameFromType<CLASS##TypeLoc>() { \ + return "`" #CLASS "TypeLoc` Node"; \ + } +#define ABSTRACT_TYPELOC(CLASS, PARENT) +#include "clang/AST/TypeLocNodes.def" + +template <> inline std::string makeMatcherNameFromType<TypeLoc>() { + return "`TypeLoc` Node"; +} + +#define DECL(CLASS, PARENT) \ + template <> inline std::string makeMatcherNameFromType<CLASS##Decl>() { \ + return "`" #CLASS "Decl` Node"; \ + } +#define ABSTRACT_DECL(CLASS) +#include "clang/AST/DeclNodes.inc" + +template <> inline std::string makeMatcherNameFromType<Decl>() { + return "`Decl` Node"; +} + +#define STMT(CLASS, PARENT) \ + template <> inline std::string makeMatcherNameFromType<CLASS>() { \ + return "`" #CLASS "` Node"; \ + } +#define ABSTRACT_STMT(CLASS) +#include "clang/AST/StmtNodes.inc" + +template <> inline std::string makeMatcherNameFromType<Stmt>() { + return "`Stmt` Node"; +} + +#define TYPE(CLASS, PARENT) \ + template <> inline std::string makeMatcherNameFromType<CLASS##Type>() { \ + return "`" #CLASS "Type` Node"; \ + } +#define ABSTRACT_TYPE(CLASS, PARENT) +#include "clang/AST/TypeNodes.inc" + +template <> inline std::string makeMatcherNameFromType<Type>() { + return "`Type` Node"; +} + +#define CLAUSE_CLASS(ENUM, STR, CLASS) \ + template <> inline std::string makeMatcherNameFromType<CLASS>() { \ + return "`" #CLASS "` Node"; \ + } +#define GEN_CLANG_CLAUSE_CLASS +#include "llvm/Frontend/OpenMP/OMP.inc" + +template <> inline std::string makeMatcherNameFromType<OMPClause>() { + return "`OMPClause` Node"; +} + +#define ATTR(A) \ + template <> inline std::string makeMatcherNameFromType<A##Attr>() { \ + return "`" #A "Attr` Node"; \ + } +#include "clang/Basic/AttrList.inc" + +template <> inline std::string makeMatcherNameFromType<Attr>() { + return "`Attr` Node"; +} + +#define MAKE_MATCHER_NAME_FROM_TYPE(CLASS) \ + template <> inline std::string makeMatcherNameFromType<CLASS>() { \ + return "`" #CLASS "` Node"; \ + } +MAKE_MATCHER_NAME_FROM_TYPE(TemplateArgument) +MAKE_MATCHER_NAME_FROM_TYPE(TemplateArgumentLoc) +MAKE_MATCHER_NAME_FROM_TYPE(TemplateName) +MAKE_MATCHER_NAME_FROM_TYPE(NestedNameSpecifier) +MAKE_MATCHER_NAME_FROM_TYPE(NestedNameSpecifierLoc) +MAKE_MATCHER_NAME_FROM_TYPE(QualType) +MAKE_MATCHER_NAME_FROM_TYPE(CXXBaseSpecifier) +MAKE_MATCHER_NAME_FROM_TYPE(CXXCtorInitializer) +MAKE_MATCHER_NAME_FROM_TYPE(LambdaCapture) +#undef MAKE_MATCHER_NAME_FROM_TYPE + /// A type-list implementation. /// /// A "linked list" of types, accessible by using the ::head and ::tail @@ -357,6 +445,12 @@ virtual llvm::Optional<clang::TraversalKind> TraversalKind() const { return llvm::None; } + + /// Returns a name relevant to the interface. + /// + /// The name returned should be specific to the interface, such that it can + /// be used to identify said interface. + virtual std::string getName() const = 0; }; /// Generic interface for matchers on an AST node of type T. @@ -381,6 +475,10 @@ BoundNodesTreeBuilder *Builder) const override { return matches(DynNode.getUnchecked<T>(), Finder, Builder); } + + std::string getName() const override { + return ::clang::ast_matchers::internal::makeMatcherNameFromType<T>(); + } }; /// Interface for matchers that only evaluate properties on a single @@ -467,12 +565,18 @@ /// restricts the node types for \p Kind. DynTypedMatcher dynCastTo(const ASTNodeKind Kind) const; - /// Return a matcher that that points to the same implementation, but sets the + /// Return a matcher that points to the same implementation, but sets the /// traversal kind. /// /// If the traversal kind is already set, then \c TK overrides it. DynTypedMatcher withTraversalKind(TraversalKind TK); + /// Return a matcher that points to the same implementation, but sets the + /// name. + /// + /// If the name is already set, then \c MatcherName overrides it. + DynTypedMatcher withMatcherName(std::string MatcherName) const; + /// Returns true if the matcher matches the given \c DynNode. bool matches(const DynTypedNode &DynNode, ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder) const; @@ -544,6 +648,8 @@ return Implementation->TraversalKind(); } + std::string getName() const { return Implementation->getName(); } + private: DynTypedMatcher(ASTNodeKind SupportedKind, ASTNodeKind RestrictKind, IntrusiveRefCntPtr<DynMatcherInterface> Implementation) @@ -663,6 +769,8 @@ } }; + std::string getName() const { return Implementation.getName(); } + private: // For Matcher<T> <=> Matcher<U> conversions. template <typename U> friend class Matcher; @@ -927,6 +1035,8 @@ return matchesSpecialized(Node); } + std::string getName() const override { return "HasOverloadedOperatorName"; } + private: /// CXXOperatorCallExpr exist only for calls to overloaded operators @@ -956,6 +1066,8 @@ bool matchesNode(const NamedDecl &Node) const override; + std::string getName() const override { return "HasName"; } + private: /// Unqualified match routine. /// @@ -1011,6 +1123,8 @@ return matchesSpecialized(Node, Finder, Builder); } + std::string getName() const override { return "HasDeclaration"; } + private: /// Forwards to matching on the underlying type of the QualType. bool matchesSpecialized(const QualType &Node, ASTMatchFinder *Finder, @@ -1272,14 +1386,19 @@ template <typename T> BindableMatcher<T> makeAllOfComposite(ArrayRef<const Matcher<T> *> InnerMatchers) { - // For the size() == 0 case, we return a "true" matcher. + auto MatcherName = makeMatcherNameFromType<T>(); + // For the size() == 0 case, we wrap a "true" matcher. if (InnerMatchers.empty()) { - return BindableMatcher<T>(TrueMatcher()); + return BindableMatcher<T>(DynTypedMatcher(Matcher<T>(TrueMatcher())) + .withMatcherName(MatcherName) + .template unconditionalConvertTo<T>()); } - // For the size() == 1 case, we simply return that one matcher. + // For the size() == 1 case, we simply wrap that one matcher. // No need to wrap it in a variadic operation. if (InnerMatchers.size() == 1) { - return BindableMatcher<T>(*InnerMatchers[0]); + return BindableMatcher<T>(DynTypedMatcher(*InnerMatchers[0]) + .withMatcherName(MatcherName) + .template unconditionalConvertTo<T>()); } using PI = llvm::pointee_iterator<const Matcher<T> *const *>; @@ -1290,6 +1409,7 @@ DynTypedMatcher::constructVariadic(DynTypedMatcher::VO_AllOf, ASTNodeKind::getFromNodeKind<T>(), std::move(DynMatchers)) + .withMatcherName(MatcherName) .template unconditionalConvertTo<T>()); } @@ -1773,6 +1893,8 @@ return Node.getValue() == ExpectedValue; } + std::string getName() const override { return "ValueEquals"; } + private: ValueT ExpectedValue; }; @@ -2251,6 +2373,8 @@ return OptOpName && llvm::is_contained(Names, *OptOpName); } + std::string getName() const override { return "HasAnyOperatorName"; } + private: static Optional<StringRef> getOpName(const UnaryOperator &Node) { return Node.getOpcodeStr(Node.getOpcode());
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits