[ASTMatchers] Fix traversal matchers with explicit and defaulted methods

[aaron.ballman]

LGTM with a few comments.

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

727

Would it be a bit cleaner to make this a private function that's a friend of ASTChildrenNotSpelledInSourceScope to discourage users from thinking they should set this manually themselves? (The query side of things seems reasonable as a public interface though.)

As a drive-by comment: we introduced some naming inconsistencies with capitalization and this continues the inconsistency. We should probably go back through and change the IsMatching* to be isMatching* to fit the usual naming conventions.

[steveire]

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

727

The query method is also just an implementation detail, so I made that private too.

clang/include/clang/ASTMatchers/ASTMatchers.h

 ///   class A {
 ///    public:
 ///     A();
 ///   };
 ///   A a = A();
 /// \endcode
 AST_MATCHER_P(CXXMethodDecl, ofClass,
               internal::Matcher<CXXRecordDecl>, InnerMatcher) {
+
+  ASTChildrenNotSpelledInSourceScope RAII(Finder, false);
+
   const CXXRecordDecl *Parent = Node.getParent();
   return (Parent != nullptr &&
           InnerMatcher.matches(*Parent, Finder, Builder));
 }
 
 /// Matches each method overridden by the given method. This matcher may
 /// produce multiple matches.
 ///

 /// cxxConstructorDecl(hasExplicitSpecifier(constantExpr())) will match #7, #8 and #9, but not #1 or #2.
 /// cxxConversionDecl(hasExplicitSpecifier(constantExpr())) will not match #3 or #4.
 /// cxxDeductionGuideDecl(hasExplicitSpecifier(constantExpr())) will not match #5 or #6.
 AST_MATCHER_P(FunctionDecl, hasExplicitSpecifier, internal::Matcher<Expr>,
               InnerMatcher) {
   ExplicitSpecifier ES = ExplicitSpecifier::getFromDecl(&Node);
   if (!ES.getExpr())
     return false;
+
+  ASTChildrenNotSpelledInSourceScope RAII(Finder, false);
+
   return InnerMatcher.matches(*ES.getExpr(), Finder, Builder);
 }
 
 /// Matches function and namespace declarations that are marked with
 /// the inline keyword.
 ///
 /// Given
 /// \code

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

return matchesAncestorOf(DynTypedNode::create(Node), getASTContext(),
                              Matcher, Builder, MatchMode);
   }
 
   virtual ASTContext &getASTContext() const = 0;
 
   virtual bool IsMatchingInASTNodeNotSpelledInSource() const = 0;
 
   virtual bool IsMatchingInASTNodeNotAsIs() const = 0;
 
   bool isTraversalIgnoringImplicitNodes() const;
 
 protected:
   virtual bool matchesChildOf(const DynTypedNode &Node, ASTContext &Ctx,
                               const DynTypedMatcher &Matcher,
                               BoundNodesTreeBuilder *Builder,
                               BindKind Bind) = 0;
 
   virtual bool matchesDescendantOf(const DynTypedNode &Node, ASTContext &Ctx,
                                    const DynTypedMatcher &Matcher,
                                    BoundNodesTreeBuilder *Builder,
                                    BindKind Bind) = 0;
 
   virtual bool matchesAncestorOf(const DynTypedNode &Node, ASTContext &Ctx,
                                  const DynTypedMatcher &Matcher,
                                  BoundNodesTreeBuilder *Builder,
                                  AncestorMatchMode MatchMode) = 0;
+private:
+  friend struct ASTChildrenNotSpelledInSourceScope;
+  virtual bool isMatchingChildrenNotSpelledInSource() const = 0;
+  virtual void setMatchingChildrenNotSpelledInSource(bool Set) = 0;
+};
+
+struct ASTChildrenNotSpelledInSourceScope {
+  ASTChildrenNotSpelledInSourceScope(ASTMatchFinder *V, bool B)
+      : MV(V), MB(V->isMatchingChildrenNotSpelledInSource()) {
+    V->setMatchingChildrenNotSpelledInSource(B);
+  }
+  ~ASTChildrenNotSpelledInSourceScope() {
+    MV->setMatchingChildrenNotSpelledInSource(MB);
+  }
+
+private:
+  ASTMatchFinder *MV;
+  bool MB;
 };
 
 /// Specialization of the conversion functions for QualType.
 ///
 /// This specialization provides the Matcher<Type>->Matcher<QualType>
 /// conversion that the static API does.
 template <>
 inline Matcher<QualType> DynTypedMatcher::convertTo<QualType>() const {

clang/lib/ASTMatchers/ASTMatchFinder.cpp

public:
   ASTContext &getASTContext() const override { return *ActiveASTContext; }
 
   bool shouldVisitTemplateInstantiations() const { return true; }
   bool shouldVisitImplicitCode() const { return true; }
 
   bool IsMatchingInASTNodeNotSpelledInSource() const override {
     return TraversingASTNodeNotSpelledInSource;
   }
+  bool isMatchingChildrenNotSpelledInSource() const override {
+    return TraversingASTChildrenNotSpelledInSource;
+  }
+  void setMatchingChildrenNotSpelledInSource(bool Set) override {
+    TraversingASTChildrenNotSpelledInSource = Set;
+  }
+
   bool IsMatchingInASTNodeNotAsIs() const override {
     return TraversingASTNodeNotAsIs;
   }
 
   bool TraverseTemplateInstantiations(ClassTemplateDecl *D) {
     ASTNodeNotSpelledInSourceScope RAII(this, true);
     return RecursiveASTVisitor<MatchASTVisitor>::TraverseTemplateInstantiations(
         D);
struct ASTNodeNotAsIsSourceScope {
     }
     ~ASTNodeNotAsIsSourceScope() { MV->TraversingASTNodeNotAsIs = MB; }
 
   private:
     MatchASTVisitor *MV;
     bool MB;
   };
 
-  struct ASTChildrenNotSpelledInSource {
-    ASTChildrenNotSpelledInSource(MatchASTVisitor *V, bool B)
-        : MV(V), MB(V->TraversingASTChildrenNotSpelledInSource) {
-      V->TraversingASTChildrenNotSpelledInSource = B;
-    }
-    ~ASTChildrenNotSpelledInSource() {
-      MV->TraversingASTChildrenNotSpelledInSource = MB;
-    }
-
-  private:
-    MatchASTVisitor *MV;
-    bool MB;
-  };
-
   class TimeBucketRegion {
   public:
     TimeBucketRegion() : Bucket(nullptr) {}
     ~TimeBucketRegion() { setBucket(nullptr); }
 
     /// Start timing for \p NewBucket.
     ///
     /// If there was a bucket already set, it will finish the timing for that
if (SK == TSK_ExplicitInstantiationDeclaration ||
         SK == TSK_ExplicitInstantiationDefinition)
       ScopedChildren = true;
   } else if (const auto *FD = dyn_cast<FunctionDecl>(DeclNode)) {
     if (FD->isDefaulted())
       ScopedChildren = true;
   }
 
   ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);
-  ASTChildrenNotSpelledInSource RAII2(this, ScopedChildren);
+  ASTChildrenNotSpelledInSourceScope RAII2(this, ScopedChildren);
 
   match(*DeclNode);
   return RecursiveASTVisitor<MatchASTVisitor>::TraverseDecl(DeclNode);
 }
 
 bool MatchASTVisitor::TraverseStmt(Stmt *StmtNode, DataRecursionQueue *Queue) {
   if (!StmtNode) {
     return true;

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

auto hasDefaultArgCall = [](auto InnerMatcher) {
     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)));
   }
+  Code = R"cpp(
+struct A
+{
+    ~A();
+private:
+    int i;
+};
+
+A::~A() = default;
+)cpp";
+  {
+    auto M = cxxDestructorDecl(isDefaulted(),
+                               ofClass(cxxRecordDecl(has(fieldDecl()))));
+    EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+    EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  Code = R"cpp(
+struct S
+{
+    static constexpr bool getTrue() { return true; }
+};
+
+struct A
+{
+    explicit(S::getTrue()) A();
+};
+
+A::A() = default;
+)cpp";
+  {
+    EXPECT_TRUE(matchesConditionally(
+        Code,
+        traverse(TK_AsIs,
+                 cxxConstructorDecl(
+                     isDefaulted(),
+                     hasExplicitSpecifier(expr(ignoringImplicit(
+                         callExpr(has(ignoringImplicit(declRefExpr())))))))),
+        true, {"-std=c++20"}));
+    EXPECT_TRUE(matchesConditionally(
+        Code,
+        traverse(TK_IgnoreUnlessSpelledInSource,
+                 cxxConstructorDecl(
+                     isDefaulted(),
+                     hasExplicitSpecifier(callExpr(has(declRefExpr()))))),
+        true, {"-std=c++20"}));
+  }
 }
 
 template <typename MatcherT>
 bool matcherTemplateWithBinding(StringRef Code, const MatcherT &M) {
   return matchAndVerifyResultTrue(
       Code, M.bind("matchedStmt"),
       std::make_unique<VerifyIdIsBoundTo<ReturnStmt>>("matchedStmt", 1));
 }