[ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

[ymandel]

Summary

This patch exposes TraversalKind support in the DynTypedMatcher API. While
previously, the match method supported traversal logic, it was not possible to
set or get the traversal kind.

[gribozavr2]

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

234

It might read better as an instance method on DynTypedMatcher: DynTypedMatcher::withTraversalKind(). It is not unprecedented, see dynCastTo().

clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

182

Please use EXPECT_THAT(M.getTraversalKind(), llvm::ValueIs(TK_AsIs)); (also in tests below).

You'll need to include llvm/Testing/Support/SupportHelpers.h.

[ymandel]

updated per comments

[ymandel]

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

234

Agreed, thanks for the suggestion.

clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

182

Thanks! This cleaned up the tests quite nicely.

I also updated the CMake file. Please let me know if that looks right to you. Cargo-culted from other cmake files...

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

public:
   static DynTypedMatcher
   constructVariadic(VariadicOperator Op, ASTNodeKind SupportedKind,
                     std::vector<DynTypedMatcher> InnerMatchers);
 
   static DynTypedMatcher
   constructRestrictedWrapper(const DynTypedMatcher &InnerMatcher,
                              ASTNodeKind RestrictKind);
 
+  /// Creates a new matcher that is identical to the old one, but sets the
+  /// traversal kind. If `InnerMatcher` had already set a traversal kind, then
+  /// the new one overrides it.
+  static DynTypedMatcher
+  constructWithTraversalKind(DynTypedMatcher InnerMatcher, TraversalKind TK);
+
   /// Get a "true" matcher for \p NodeKind.
   ///
   /// It only checks that the node is of the right kind.
   static DynTypedMatcher trueMatcher(ASTNodeKind NodeKind);
 
   void setAllowBind(bool AB) { AllowBind = AB; }
 
   /// Check whether this matcher could ever match a node of kind \p Kind.
public:
   }
 
   /// 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 <typename T> Matcher<T> unconditionalConvertTo() const;
 
+  /// Returns the \c TraversalKind respected by calls to `match()`, if any.
+  ///
+  /// Most matchers will not have a traversal kind set, instead relying on the
+  /// surrounding context. For those, \c llvm::None is returned.
+  llvm::Optional<clang::TraversalKind> getTraversalKind() const {
+    return Implementation->TraversalKind();
+  }
+
 private:
   DynTypedMatcher(ASTNodeKind SupportedKind, ASTNodeKind RestrictKind,
                   IntrusiveRefCntPtr<DynMatcherInterface> Implementation)
       : SupportedKind(SupportedKind), RestrictKind(RestrictKind),
         Implementation(std::move(Implementation)) {}
 
   bool AllowBind = false;
   ASTNodeKind SupportedKind;

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

public:
   }
 
   bool dynMatches(const DynTypedNode &, ASTMatchFinder *,
                   BoundNodesTreeBuilder *) const override {
     return true;
   }
 };
 
+/// A matcher that specifies a particular \c TraversalKind.
+///
+/// The kind provided to the constructor overrides any kind that may be
+/// specified by the `InnerMatcher`.
+class DynTraversalMatcherImpl : public DynMatcherInterface {
+public:
+  explicit DynTraversalMatcherImpl(
+      clang::TraversalKind TK,
+      IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher)
+      : TK(TK), InnerMatcher(std::move(InnerMatcher)) {}
+
+  bool dynMatches(const DynTypedNode &DynNode, ASTMatchFinder *Finder,
+                  BoundNodesTreeBuilder *Builder) const override {
+    return this->InnerMatcher->dynMatches(DynNode, Finder, Builder);
+  }
+
+  llvm::Optional<clang::TraversalKind> TraversalKind() const override {
+    return TK;
+  }
+
+private:
+  clang::TraversalKind TK;
+  IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher;
+};
+
 } // namespace
 
 static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance;
 
 DynTypedMatcher
 DynTypedMatcher::constructVariadic(DynTypedMatcher::VariadicOperator Op,
                                    ASTNodeKind SupportedKind,
                                    std::vector<DynTypedMatcher> InnerMatchers) {

 DynTypedMatcher
 DynTypedMatcher::constructRestrictedWrapper(const DynTypedMatcher &InnerMatcher,
                                             ASTNodeKind RestrictKind) {
   DynTypedMatcher Copy = InnerMatcher;
   Copy.RestrictKind = RestrictKind;
   return Copy;
 }
 
+DynTypedMatcher
+DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher InnerMatcher,
+                                            ast_type_traits::TraversalKind TK) {
+  InnerMatcher.Implementation =
+      new DynTraversalMatcherImpl(TK, std::move(InnerMatcher.Implementation));
+  return InnerMatcher;
+}
+
 DynTypedMatcher DynTypedMatcher::trueMatcher(ASTNodeKind NodeKind) {
   return DynTypedMatcher(NodeKind, NodeKind, &*TrueMatcherInstance);
 }
 
 bool DynTypedMatcher::canMatchNodesOfKind(ASTNodeKind Kind) const {
   return RestrictKind.isBaseOf(Kind);
 }
 

clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Host.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace ast_matchers {
+using internal::DynTypedMatcher;
 
 #if GTEST_HAS_DEATH_TEST
 TEST(HasNameDeathTest, DiesOnEmptyName) {
   ASSERT_DEBUG_DEATH({
     DeclarationMatcher HasEmptyName = recordDecl(hasName(""));
     EXPECT_TRUE(notMatches("class X {};", HasEmptyName));
   }, "");
 }
TEST(Matcher, matchOverEntireASTContext) {
   std::unique_ptr<ASTUnit> AST =
       clang::tooling::buildASTFromCode("struct { int *foo; };");
   ASSERT_TRUE(AST.get());
   auto PT = selectFirst<PointerType>(
       "x", match(pointerType().bind("x"), AST->getASTContext()));
   EXPECT_NE(nullptr, PT);
 }
 
+TEST(DynTypedMatcherTest, TraversalKindForwardsToImpl) {
+  auto M = DynTypedMatcher(decl());
+  EXPECT_FALSE(M.getTraversalKind().hasValue());
+
+  M = DynTypedMatcher(traverse(TK_AsIs, decl()));
+  llvm::Optional<clang::TraversalKind> TK = M.getTraversalKind();
+  EXPECT_TRUE(TK.hasValue());
+  EXPECT_EQ(*TK, TK_AsIs);
+}
+
+TEST(DynTypedMatcherTest, ConstructWithTraversalKindSetsTK) {
+  auto M = DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher(decl()),
+                                                       TK_AsIs);
+  llvm::Optional<clang::TraversalKind> TK = M.getTraversalKind();
+  EXPECT_TRUE(TK.hasValue());
+  EXPECT_EQ(*TK, TK_AsIs);
+}
+
+TEST(DynTypedMatcherTest, ConstructWithTraversalKindOverridesNestedTK) {
+  auto M = DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher(decl()),
+                                                       TK_AsIs);
+  auto M2 = DynTypedMatcher::constructWithTraversalKind(
+      M, TK_IgnoreUnlessSpelledInSource);
+  llvm::Optional<clang::TraversalKind> TK = M2.getTraversalKind();
+  EXPECT_TRUE(TK.hasValue());
+  EXPECT_EQ(*TK, TK_IgnoreUnlessSpelledInSource);
+}
+
 TEST(IsInlineMatcher, IsInline) {
   EXPECT_TRUE(matches("void g(); inline void f();",
                       functionDecl(isInline(), hasName("f"))));
   EXPECT_TRUE(matches("namespace n { inline namespace m {} }",
                       namespaceDecl(isInline(), hasName("m"))));
 }
 
 // FIXME: Figure out how to specify paths so the following tests pass on