Skip to content

Conversation

@ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Dec 6, 2025

This was noted in #170540, and seems to simply be an oversight. This patch just add the same logic already used in other addMatcher() implementations that honor the traversal kind.

This was noted in llvm#170540, and seems to simply be an oversight. This patch
just add the same logic already used in other addMatcher() implementations
that honor the traversal kind.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Dec 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2025

@llvm/pr-subscribers-clang

Author: Paul Kirth (ilovepi)

Changes

This was noted in #170540, and seems to simply be an oversight. This patch just add the same logic already used in other addMatcher() implementations that honor the traversal kind.


Full diff: https://github.com/llvm/llvm-project/pull/170953.diff

2 Files Affected:

  • (modified) clang/lib/ASTMatchers/ASTMatchFinder.cpp (+50-7)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp (+35)
diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index e8a0004c2e187..81fb881302af5 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -1681,7 +1681,13 @@ void MatchFinder::addMatcher(const DeclarationMatcher &NodeMatch,
 
 void MatchFinder::addMatcher(const TypeMatcher &NodeMatch,
                              MatchCallback *Action) {
-  Matchers.Type.emplace_back(NodeMatch, Action);
+  std::optional<TraversalKind> TK;
+  if (Action)
+    TK = Action->getCheckTraversalKind();
+  if (TK)
+    Matchers.Type.emplace_back(traverse(*TK, NodeMatch), Action);
+  else
+    Matchers.Type.emplace_back(NodeMatch, Action);
   Matchers.AllCallbacks.insert(Action);
 }
 
@@ -1699,37 +1705,74 @@ void MatchFinder::addMatcher(const StatementMatcher &NodeMatch,
 
 void MatchFinder::addMatcher(const NestedNameSpecifierMatcher &NodeMatch,
                              MatchCallback *Action) {
-  Matchers.NestedNameSpecifier.emplace_back(NodeMatch, Action);
+  std::optional<TraversalKind> TK;
+  if (Action)
+    TK = Action->getCheckTraversalKind();
+  if (TK)
+    Matchers.NestedNameSpecifier.emplace_back(traverse(*TK, NodeMatch), Action);
+  else
+    Matchers.NestedNameSpecifier.emplace_back(NodeMatch, Action);
   Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierLocMatcher &NodeMatch,
                              MatchCallback *Action) {
-  Matchers.NestedNameSpecifierLoc.emplace_back(NodeMatch, Action);
+  std::optional<TraversalKind> TK;
+  if (Action)
+    TK = Action->getCheckTraversalKind();
+  if (TK)
+    Matchers.NestedNameSpecifierLoc.emplace_back(traverse(*TK, NodeMatch),
+                                                 Action);
+  else
+    Matchers.NestedNameSpecifierLoc.emplace_back(NodeMatch, Action);
   Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeLocMatcher &NodeMatch,
                              MatchCallback *Action) {
-  Matchers.TypeLoc.emplace_back(NodeMatch, Action);
+  std::optional<TraversalKind> TK;
+  if (Action)
+    TK = Action->getCheckTraversalKind();
+  if (TK)
+    Matchers.TypeLoc.emplace_back(traverse(*TK, NodeMatch), Action);
+  else
+    Matchers.TypeLoc.emplace_back(NodeMatch, Action);
   Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const CXXCtorInitializerMatcher &NodeMatch,
                              MatchCallback *Action) {
-  Matchers.CtorInit.emplace_back(NodeMatch, Action);
+  std::optional<TraversalKind> TK;
+  if (Action)
+    TK = Action->getCheckTraversalKind();
+  if (TK)
+    Matchers.CtorInit.emplace_back(traverse(*TK, NodeMatch), Action);
+  else
+    Matchers.CtorInit.emplace_back(NodeMatch, Action);
   Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TemplateArgumentLocMatcher &NodeMatch,
                              MatchCallback *Action) {
-  Matchers.TemplateArgumentLoc.emplace_back(NodeMatch, Action);
+  std::optional<TraversalKind> TK;
+  if (Action)
+    TK = Action->getCheckTraversalKind();
+  if (TK)
+    Matchers.TemplateArgumentLoc.emplace_back(traverse(*TK, NodeMatch), Action);
+  else
+    Matchers.TemplateArgumentLoc.emplace_back(NodeMatch, Action);
   Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const AttrMatcher &AttrMatch,
                              MatchCallback *Action) {
-  Matchers.Attr.emplace_back(AttrMatch, Action);
+  std::optional<TraversalKind> TK;
+  if (Action)
+    TK = Action->getCheckTraversalKind();
+  if (TK)
+    Matchers.Attr.emplace_back(traverse(*TK, AttrMatch), Action);
+  else
+    Matchers.Attr.emplace_back(AttrMatch, Action);
   Matchers.AllCallbacks.insert(Action);
 }
 
diff --git a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
index a930638f355b9..3fa71804710ac 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -297,6 +297,41 @@ TEST(DynTypedMatcherTest, ConstructWithTraversalKindOverridesNestedTK) {
               llvm::ValueIs(TK_IgnoreUnlessSpelledInSource));
 }
 
+TEST(MatchFinder, AddMatcherOverloadsHonorTraversalKind) {
+  StringRef Code = R"cpp(
+    struct B {};
+    struct C : B {
+      C() {}
+    };
+  )cpp";
+
+  // C() has an implicit initializer for B.
+  auto Matcher = cxxCtorInitializer(isBaseInitializer());
+
+  {
+    bool Matched = false;
+    MatchFinder Finder;
+    struct TestCallback : public MatchFinder::MatchCallback {
+      std::optional<TraversalKind> TK;
+      bool *Matched;
+      TestCallback(std::optional<TraversalKind> TK, bool *Matched)
+          : TK(TK), Matched(Matched) {}
+      void run(const MatchFinder::MatchResult &Result) override {
+        *Matched = true;
+      }
+      std::optional<TraversalKind> getCheckTraversalKind() const override {
+        return TK;
+      }
+    } Callback(TK_IgnoreUnlessSpelledInSource, &Matched);
+    Finder.addMatcher(Matcher, &Callback);
+    std::unique_ptr<FrontendActionFactory> Factory(
+        newFrontendActionFactory(&Finder));
+    ASSERT_TRUE(tooling::runToolOnCode(Factory->create(), Code));
+    EXPECT_FALSE(Matched) << "Matcher not using specified TraversalKind, "
+                             "TK_IgnoreUnlessSpelledInSource";
+  }
+}
+
 TEST(IsInlineMatcher, IsInline) {
   EXPECT_TRUE(matches("void g(); inline void f();",
                       functionDecl(isInline(), hasName("f"))));

@zwuis
Copy link
Contributor

zwuis commented Dec 6, 2025

Please add a release note entry. Otherwise LGTM.

EXPECT_THAT(M.getTraversalKind(), llvm::ValueIs(TK_AsIs));
}

TEST(DynTypedMatcherTest, ConstructWithTraversalKindOverridesNestedTK) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two withTraversalKind seemed intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants