Update matchers to be traverse-aware

[steveire]

Summary

Don't match Stmt or Decl nodes not spelled in the source when using
TK_IgnoreUnlessSpelledInSource. This prevents accidental modification
of source code at incorrect locations.

[aaron.ballman]

I think this change should come with a warning in the release notes because it silently changes the behavior of existing matchers in a way that may be surprising to users. I think the behavior is correct in that these constructs are all implicit nodes that aren't really spelled in source, but users may have come to rely on the existing behavior. I will note that the default argument case is somewhat interesting to me because there is a defensible argument to be made that the expression is spelled in source (it's spelled at the location of the function declaration rather than at the call site) and that same logic applies to explicitly defaulted function declarations. However, given the goal of trying to model this on the syntactic form of what the user wrote, I think the proposed behavior is correct.

clang/include/clang/ASTMatchers/ASTMatchers.h

3115

Given how often this gets repeated (and what a mouthful it is), how about adding a static helper function ClassifyTraversal() (or some such) that returns the traversal kind given a Finder? (Alternatively, isTraversalAsIs() or some specific traversal behavior.)

4054–4062

Please spell out the type.

4059

Likewise here.

4080

And here.

4303

Spell the type out?

5456

In general (not specific to this matcher), I'd like to see some tests for explicitly defaulted functions vs implicitly defaulted ones. e.g., I think we should handle these differently, shouldn't we?

struct S {}; // Has implicitly defaulted constructor
struct T {
  T() = default; // Has explicitly defaulted constructor
};

Specific to this matcher, I am a bit concerned about the behavior of answering "isDefinition" based on whether a function is defaulted or not. The user has already gotten some FunctionDecl object, so why would the implicit vs explicit nature of the declaration matter for deciding whether the node is a definition? The behavior with hasBody makes sense to me because the body may be implicitly provided, but whether something is or is not a definition is a different kind of question.

7060

Same question here about why defaulted matters as above.

[steveire]

Update

[steveire]

I added the release note to D90982, which this change follows.

clang/include/clang/ASTMatchers/ASTMatchers.h

3115

Done in D91144.

5456

I agree for both isDefinition and isInline. hasBody is the one to use for these cases.

[aaron.ballman]

clang/include/clang/ASTMatchers/ASTMatchers.h

3115

Thanks! Just an FYI, but it looks like the changes in D91144 are reflected in this review. I don't mind but wanted to mention it in case it causes you problems when trying to apply the commits to master.

5456

Awesome, thanks for verifying!

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

2368

Was this originally testing behavior with explicitly initializing an implicitly generated ctor (since that's also an implicit node)? Perhaps this change should be reverted and we make a second test?

2429

You can drop the .bind() I believe.

[steveire]

Update

[steveire]

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

2368

Good point, done.

[aaron.ballman]

clang/include/clang/ASTMatchers/ASTMatchers.h

3115

If the traversal is not AsIs, that doesn't mean it's IgnoreUnlessSpelledInSource -- it could be IgnoreImplicitCastsAndParentheses, right? So I think the logic for some of these AST matchers is incorrect when in IgnoreImplicitCastsAndParentheses traversal mode and should be double-checked. We should probably add some extra test coverage for that mode.

4083

Huh, I never noticed that we implicitly are ignoring parens and implicit casts for this (without checking the traversal mode or documenting the behavior!). That seems less-than-ideal in some ways. (No change required, I only noticed it because it made me think through whether we need it on the isa<> check above, which we don't.)

[steveire]

clang/include/clang/ASTMatchers/ASTMatchers.h

3115

As far as I know, the problem of implicit nodes has been known for a long time. Adding calls to IgnoreParenImpCasts() in certain places like hasArgument was one attempt at making the implicit nodes less convenient for users. As far as I know, IgnoreImplicitCastsAndParentheses was just another attempt at the same thing. I must have discovered that by reading code history at some point. Both previous attempts didn't go far enough to actually solve the problem, but IgnoreUnlessSpelledInSource does go all the way, and traverse puts control in the hands of the user. D20801 at least seems to have been an attempt to put control back in the hands of the user. And it was a follow-up to D18243.

So, once this and D90982 are merged, I think it makes sense to start to remove IgnoreImplicitCastsAndParentheses entirely. It is legacy, incompletely useful and just causes some mess in the code.

Two modes aught to be enough for anybody.

4083

Yes, I think calls to ignore* like this within matcher implementations should be removed, giving the user control instead.

[aaron.ballman]

clang/include/clang/ASTMatchers/ASTMatchers.h

3115

As far as I know, IgnoreImplicitCastsAndParentheses was just another attempt at the same thing. I must have discovered that by reading code history at some point.

I don't recall the history of this traversal mode, but I think you're correct.

So, once this and D90982 are merged, I think it makes sense to start to remove IgnoreImplicitCastsAndParentheses entirely. It is legacy, incompletely useful and just causes some mess in the code.

I agree. I see at least two ways to proceed:

1. Change this patch to handle three modes so that we can land it without incorrect behavior, then do a follow-up patch to rip out the IgnoreImplicitCastsAndParentheses mode.
2. Rip out IgnoreImplicitCastsAndParentheses first and then land this patch as-is.

My preference is for #1 so that we don't land this patch in an awkward state (in case the removal of the other mode gets delayed for some reason). Given what I mention below, that shouldn't result in any undue churn, I'd hope.

Two modes aught to be enough for anybody.

Heh, I would like to think that, but given that this enumeration is used for generic traversal of ASTs, I'm less convinced that two modes will be all we ever want to support. I think it's a bit more future-proof to add a helper method along the lines of isTraversalIgnoringImplicitNodes() rather than using !isTraversalAsIs().

4054–4062

auto -> unsigned please

4058

auto -> const Expr * or just get rid of the local variable entirely by sinking the initialization into the isa<>.

4080

const auto * -> const Expr *

4083

I agree. I'm a little bit worried about what will break when we make the change, but I think that model is the more defensible one. (It looks like there are a handful of similar cases in ASTMatchers.h, so we should try to hit them all).

[steveire]

Update

[aaron.ballman]

clang/include/clang/ASTMatchers/ASTMatchers.h

4083

This probably shouldn't compile given that there's no declaration of Arg?

[steveire]

Update

[steveire]

clang/include/clang/ASTMatchers/ASTMatchers.h

4083

Correct. Fixed now.

[aaron.ballman]

LGTM, thank you!

clang/include/clang/ASTMatchers/ASTMatchers.h

 ///   class A { void func(); };
 ///   class B { void member(); };
 /// \endcode
 ///
 /// \c cxxRecordDecl(hasMethod(hasName("func"))) matches the declaration of
 /// \c A but not \c B.
 AST_MATCHER_P(CXXRecordDecl, hasMethod, internal::Matcher<CXXMethodDecl>,
               InnerMatcher) {
-  return matchesFirstInPointerRange(InnerMatcher, Node.method_begin(),
-                                    Node.method_end(), Finder,
-                                    Builder) != Node.method_end();
+  BoundNodesTreeBuilder Result(*Builder);
+  auto MatchIt = matchesFirstInPointerRange(InnerMatcher, Node.method_begin(),
+                                            Node.method_end(), Finder, &Result);
+  if (MatchIt == Node.method_end())
+    return false;
+
+  if (Finder->isTraversalIgnoringImplicitNodes() && (*MatchIt)->isImplicit())
+    return false;
+  *Builder = std::move(Result);
+  return true;
 }
 
 /// Matches the generated class of lambda expressions.
 ///
 /// Given:
 /// \code
 ///   auto x = []{};
 /// \endcode

 ///   void f(int x, int y);
 ///   f(0, 0);
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P(argumentCountIs,
                           AST_POLYMORPHIC_SUPPORTED_TYPES(
                               CallExpr, CXXConstructExpr,
                               CXXUnresolvedConstructExpr, ObjCMessageExpr),
                           unsigned, N) {
-  return Node.getNumArgs() == N;
+  unsigned NumArgs = Node.getNumArgs();
+  if (!Finder->isTraversalIgnoringImplicitNodes())
+    return NumArgs == N;
+  while (NumArgs) {
+    if (!isa<CXXDefaultArgExpr>(Node.getArg(NumArgs - 1)))
+      break;
+    --NumArgs;
+  }
+  return NumArgs == N;
 }
 
 /// Matches the n'th argument of a call expression or a constructor
 /// call expression.
 ///
 /// Example matches y in x(y)
 ///     (matcher = callExpr(hasArgument(0, declRefExpr())))
 /// \code
 ///   void x(int) { int y; x(y); }
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P2(hasArgument,
                            AST_POLYMORPHIC_SUPPORTED_TYPES(
                                CallExpr, CXXConstructExpr,
                                CXXUnresolvedConstructExpr, ObjCMessageExpr),
                            unsigned, N, internal::Matcher<Expr>, InnerMatcher) {
-  return (N < Node.getNumArgs() &&
-          InnerMatcher.matches(
-              *Node.getArg(N)->IgnoreParenImpCasts(), Finder, Builder));
+  if (N >= Node.getNumArgs())
+    return false;
+  const Expr *Arg = Node.getArg(N);
+  if (Finder->isTraversalIgnoringImplicitNodes() && isa<CXXDefaultArgExpr>(Arg))
+    return false;
+  return InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, Builder);
 }
 
 /// Matches the n'th item of an initializer list expression.
 ///
 /// Example matches y.
 ///     (matcher = initListExpr(hasInit(0, expr())))
 /// \code
 ///   int x{y}.

 ///   };
 /// \endcode
 /// cxxRecordDecl(has(cxxConstructorDecl(
 ///   hasAnyConstructorInitializer(anything())
 /// )))
 ///   record matches Foo, hasAnyConstructorInitializer matches foo_(1)
 AST_MATCHER_P(CXXConstructorDecl, hasAnyConstructorInitializer,
               internal::Matcher<CXXCtorInitializer>, InnerMatcher) {
-  return matchesFirstInPointerRange(InnerMatcher, Node.init_begin(),
-                                    Node.init_end(), Finder,
-                                    Builder) != Node.init_end();
+  auto MatchIt = matchesFirstInPointerRange(InnerMatcher, Node.init_begin(),
+                                            Node.init_end(), Finder, Builder);
+  if (MatchIt == Node.init_end())
+    return false;
+  return (*MatchIt)->isWritten() || !Finder->isTraversalIgnoringImplicitNodes();
 }
 
 /// Matches the field declaration of a constructor initializer.
 ///
 /// Given
 /// \code
 ///   struct Foo {
 ///     Foo() : foo_(1) { }

 /// \endcode
 /// objcMessageExpr(hasAnyArgument(integerLiteral(equals(12))))
 ///   matches [i f:12]
 AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
                           AST_POLYMORPHIC_SUPPORTED_TYPES(
                               CallExpr, CXXConstructExpr,
                               CXXUnresolvedConstructExpr, ObjCMessageExpr),
                           internal::Matcher<Expr>, InnerMatcher) {
   for (const Expr *Arg : Node.arguments()) {
+    if (Finder->isTraversalIgnoringImplicitNodes() &&
+        isa<CXXDefaultArgExpr>(Arg))
+      break;
     BoundNodesTreeBuilder Result(*Builder);
     if (InnerMatcher.matches(*Arg, Finder, &Result)) {
       *Builder = std::move(Result);
       return true;
     }
   }
   return false;
 }

 ///   but does not match 'void f();'
 
 AST_POLYMORPHIC_MATCHER_P(hasBody,
                           AST_POLYMORPHIC_SUPPORTED_TYPES(DoStmt, ForStmt,
                                                           WhileStmt,
                                                           CXXForRangeStmt,
                                                           FunctionDecl),
                           internal::Matcher<Stmt>, InnerMatcher) {
+  if (Finder->isTraversalIgnoringImplicitNodes() && isDefaultedHelper(&Node))
+    return false;
   const Stmt *const Statement = internal::GetBodyMatcher<NodeType>::get(Node);
   return (Statement != nullptr &&
           InnerMatcher.matches(*Statement, Finder, Builder));
 }
 
 /// Matches a function declaration that has a given body present in the AST.
 /// Note that this matcher matches all the declarations of a function whose
 /// body is present in the AST.

 ///   Matcher<ObjCMethodDecl>
 AST_POLYMORPHIC_MATCHER(isDefinition,
                         AST_POLYMORPHIC_SUPPORTED_TYPES(TagDecl, VarDecl,
                                                         ObjCMethodDecl,
                                                         FunctionDecl)) {
   return Node.isThisDeclarationADefinition();
 }
 
 /// Matches if a function declaration is variadic.
 ///
 /// Example matches f, but not g or h. The function i will not match, even when
 /// compiled in C mode.
 /// \code
 ///   void f(...);
 ///   void g(int);
 ///   template <typename... Ts> void h(Ts...);
 ///   void i();

 ///   forField(decl().bind("x"))
 /// ))
 ///   will trigger two matches, binding for 'i' and 'j' respectively.
 AST_MATCHER_P(CXXConstructorDecl, forEachConstructorInitializer,
               internal::Matcher<CXXCtorInitializer>, InnerMatcher) {
   BoundNodesTreeBuilder Result;
   bool Matched = false;
   for (const auto *I : Node.inits()) {
+    if (Finder->isTraversalIgnoringImplicitNodes() && !I->isWritten())
+      continue;
     BoundNodesTreeBuilder InitBuilder(*Builder);
     if (InnerMatcher.matches(*I, Finder, &InitBuilder)) {
       Matched = true;
       Result.addMatch(InitBuilder);
     }
   }
   *Builder = std::move(Result);
   return Matched;

 AST_POLYMORPHIC_MATCHER(isInline,
                         AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
                                                         FunctionDecl)) {
   // This is required because the spelling of the function used to determine
   // whether inline is specified or not differs between the polymorphic types.
   if (const auto *FD = dyn_cast<FunctionDecl>(&Node))
     return FD->isInlineSpecified();
   else if (const auto *NSD = dyn_cast<NamespaceDecl>(&Node))
     return NSD->isInline();
   llvm_unreachable("Not a valid polymorphic type");
 }
 
 /// Matches anonymous namespace declarations.
 ///
 /// Given
 /// \code
 ///   namespace n {

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

for (IteratorT I = Start; I != End; ++I) {
     if (Matcher.matches(**I, Finder, &Result)) {
       *Builder = std::move(Result);
       return I;
     }
   }
   return End;
 }
 
+template <typename T, std::enable_if_t<!std::is_base_of<FunctionDecl, T>::value>
+                          * = nullptr>
+inline bool isDefaultedHelper(const T *) {
+  return false;
+}
+inline bool isDefaultedHelper(const FunctionDecl *FD) {
+  return FD->isDefaulted();
+}
+
 // Metafunction to determine if type T has a member called getDecl.
 template <typename Ty>
 class has_getDecl {
   using yes = char[1];
   using no = char[2];
 
   template <typename Inner>
   static yes& test(Inner *I, decltype(I->getDecl()) * = nullptr);
bool matchesSpecialized(const LabelStmt &Node, ASTMatchFinder *Finder,
                           BoundNodesTreeBuilder *Builder) const {
     return matchesDecl(Node.getDecl(), Finder, Builder);
   }
 
   /// Returns whether the inner matcher \c Node. Returns false if \c Node
   /// is \c NULL.
   bool matchesDecl(const Decl *Node, ASTMatchFinder *Finder,
                    BoundNodesTreeBuilder *Builder) const {
+    if (Finder->isTraversalIgnoringImplicitNodes() && Node->isImplicit())
+      return false;
     return Node != nullptr && this->InnerMatcher.matches(
                                   DynTypedNode::create(*Node), Finder, Builder);
   }
 };
 
 /// IsBaseType<T>::value is true if T is a "base" type in the AST
 /// node class hierarchies.
 template <typename T>

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

void copyIt()
     ContainsArray ca2;
     ca2 = ca;
 }
 
 struct HasCtorInits : NoSpecialMethods, NonTrivial
 {
   int m_i;
   NonTrivial m_nt;
   HasCtorInits() : NoSpecialMethods(), m_i(42) {}
 };
 
+struct CtorInitsNonTrivial : NonTrivial
+{
+  int m_i;
+  NonTrivial m_nt;
+  CtorInitsNonTrivial() : NonTrivial(), m_i(42) {}
+};
+
 )cpp";
   {
     auto M = cxxRecordDecl(hasName("NoSpecialMethods"),
                            has(cxxRecordDecl(hasName("NoSpecialMethods"))));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
 
     M = cxxRecordDecl(hasName("NoSpecialMethods"),
)cpp";
     M = cxxRecordDecl(hasName("NoSpecialMethods"),
                       has(cxxConstructorDecl(isDefaultConstructor())));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
 
     M = cxxRecordDecl(hasName("NoSpecialMethods"), has(cxxDestructorDecl()));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+    M = cxxRecordDecl(hasName("NoSpecialMethods"),
+                      hasMethod(cxxConstructorDecl(isCopyConstructor())));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+    M = cxxRecordDecl(hasName("NoSpecialMethods"),
+                      hasMethod(cxxMethodDecl(isCopyAssignmentOperator())));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+    M = cxxRecordDecl(hasName("NoSpecialMethods"),
+                      hasMethod(cxxConstructorDecl(isDefaultConstructor())));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+
+    M = cxxRecordDecl(hasName("NoSpecialMethods"),
+                      hasMethod(cxxDestructorDecl()));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+    // Because the copy-assignment operator is not spelled in the
+    // source (ie, isImplicit()), we don't match it
+    auto M =
+        cxxOperatorCallExpr(hasType(cxxRecordDecl(hasName("NoSpecialMethods"))),
+                            callee(cxxMethodDecl(isCopyAssignmentOperator())));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
   {
     // Compiler generates a forStmt to copy the array
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, forStmt())));
     EXPECT_FALSE(
         matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, forStmt())));
   }
   {
     // The defaulted method declaration can be matched, but not its
     // definition, in IgnoreUnlessSpelledInSource mode
     auto MDecl = cxxMethodDecl(ofClass(cxxRecordDecl(hasName("ContainsArray"))),
                                isCopyAssignmentOperator(), isDefaulted());
 
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MDecl)));
     EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MDecl)));
 
     auto MDef = cxxMethodDecl(MDecl, has(compoundStmt()));
 
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MDef)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MDef)));
 
+    auto MBody = cxxMethodDecl(MDecl, hasBody(compoundStmt()));
+
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MBody)));
+    EXPECT_FALSE(
+        matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MBody)));
+
+    auto MIsDefn = cxxMethodDecl(MDecl, isDefinition());
+
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MIsDefn)));
+    EXPECT_TRUE(
+        matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MIsDefn)));
+
+    auto MIsInline = cxxMethodDecl(MDecl, isInline());
+
+    EXPECT_FALSE(matches(Code, traverse(TK_AsIs, MIsInline)));
+    EXPECT_FALSE(
+        matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MIsInline)));
+
     // The parameter of the defaulted method can still be matched.
     auto MParm =
         cxxMethodDecl(MDecl, hasParameter(0, parmVarDecl(hasName("other"))));
 
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MParm)));
     EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MParm)));
   }
   {
     auto M =
         cxxConstructorDecl(hasName("HasCtorInits"),
                            has(cxxCtorInitializer(forField(hasName("m_i")))));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
   {
     auto M =
         cxxConstructorDecl(hasName("HasCtorInits"),
                            has(cxxCtorInitializer(forField(hasName("m_nt")))));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
   {
+    auto M = cxxConstructorDecl(hasName("HasCtorInits"),
+                                hasAnyConstructorInitializer(cxxCtorInitializer(
+                                    forField(hasName("m_nt")))));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+    auto M =
+        cxxConstructorDecl(hasName("HasCtorInits"),
+                           forEachConstructorInitializer(
+                               cxxCtorInitializer(forField(hasName("m_nt")))));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
     auto M = cxxConstructorDecl(
-        hasName("HasCtorInits"),
-        has(cxxCtorInitializer(withInitializer(cxxConstructExpr(hasDeclaration(
-            cxxConstructorDecl(hasName("NoSpecialMethods"))))))));
+        hasName("CtorInitsNonTrivial"),
+        has(cxxCtorInitializer(withInitializer(cxxConstructExpr(
+            hasDeclaration(cxxConstructorDecl(hasName("NonTrivial"))))))));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
   {
     auto M = cxxConstructorDecl(
         hasName("HasCtorInits"),
-        has(cxxCtorInitializer(withInitializer(cxxConstructExpr(
-            hasDeclaration(cxxConstructorDecl(hasName("NonTrivial"))))))));
+        has(cxxCtorInitializer(withInitializer(cxxConstructExpr(hasDeclaration(
+            cxxConstructorDecl(hasName("NoSpecialMethods"))))))));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
   {
     auto M = cxxCtorInitializer(forField(hasName("m_nt")));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }

 void hasDefaultArg(int i, int j = 0)
 {
 }
 void callDefaultArg()
 {
   hasDefaultArg(42);
 }
 )cpp";
+  auto hasDefaultArgCall = [](auto InnerMatcher) {
+    return callExpr(callee(functionDecl(hasName("hasDefaultArg"))),
+                    InnerMatcher);
+  };
   {
-    auto M = callExpr(has(integerLiteral(equals(42))));
+    auto M = hasDefaultArgCall(has(integerLiteral(equals(42))));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
   {
-    auto M = callExpr(has(cxxDefaultArgExpr()));
+    auto M = hasDefaultArgCall(has(cxxDefaultArgExpr()));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+    auto M = hasDefaultArgCall(argumentCountIs(2));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+    auto M = hasDefaultArgCall(argumentCountIs(1));
+    EXPECT_FALSE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+    auto M = hasDefaultArgCall(hasArgument(1, cxxDefaultArgExpr()));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+    auto M = hasDefaultArgCall(hasAnyArgument(cxxDefaultArgExpr()));
     EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
     EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
 }
 
 template <typename MatcherT>
 bool matcherTemplateWithBinding(StringRef Code, const MatcherT &M) {
   return matchAndVerifyResultTrue(

clang/unittests/Tooling/TransformerTest.cpp

auto RewriteOutput =
 
     testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedLoop),
                       RewriteRule),
              RewriteInput, RewriteOutput);
 
     testRuleFailure(makeRule(traverse(TK_AsIs, MatchedLoop), RewriteRule),
                     RewriteInput);
   }
+  {
+    auto MatchedLoop = forStmt(
+        hasLoopInit(declStmt(
+            hasSingleDecl(varDecl(hasInitializer(integerLiteral(equals(0))))
+                              .bind("loopVar")))),
+        hasCondition(binaryOperator(hasOperatorName("!="),
+                                    hasLHS(ignoringImplicit(declRefExpr(to(
+                                        varDecl(equalsBoundNode("loopVar")))))),
+                                    hasRHS(expr().bind("upperBoundExpr")))),
+        hasIncrement(unaryOperator(hasOperatorName("++"),
+                                   hasUnaryOperand(declRefExpr(to(
+                                       varDecl(equalsBoundNode("loopVar"))))))
+                         .bind("incrementOp")));
+
+    auto RewriteRule =
+        changeTo(transformer::enclose(node("loopVar"), node("incrementOp")),
+                 cat("auto ", name("loopVar"), " : boost::irange(",
+                     node("upperBoundExpr"), ")"));
+
+    testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedLoop),
+                      RewriteRule),
+             RewriteInput, RewriteOutput);
+
+    testRuleFailure(makeRule(traverse(TK_AsIs, MatchedLoop), RewriteRule),
+                    RewriteInput);
+  }
 }
 
 TEST_F(TransformerTest, TemplateInstantiation) {
 
   std::string NonTemplatesInput = R"cpp(
 struct S {
   int m_i;
 };