RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

[gribozavr]

Summary

How does RecursiveASTVisitor call the WalkUp callback for expressions?

• In pre-order traversal mode, RecursiveASTVisitor calls the WalkUp callback from the default implementation of Traverse callbacks.

• In post-order traversal mode when we don't have a DataRecursionQueue, RecursiveASTVisitor also calls the WalkUp callback from the default implementation of Traverse callbacks.

• However, in post-order traversal mode when we have a DataRecursionQueue, RecursiveASTVisitor calls the WalkUp callback from PostVisitStmt.

As a result, when the user overrides the Traverse callback, in pre-order
traversal mode they never get the corresponding WalkUp callback. However
in the post-order traversal mode the WalkUp callback is invoked or not
depending on whether the data recursion optimization could be applied.

I had to adjust the implementation of TraverseCXXForRangeStmt in the
syntax tree builder to call the WalkUp method directly, as it was
relying on this behavior. There is an existing test for this
functionality and it prompted me to make this extra fix.

In addition, I had to fix the default implementation implementation of
RecursiveASTVisitor::TraverseSynOrSemInitListExpr to call WalkUpFrom in
the same manner as the implementation generated by the DEF_TRAVERSE_STMT
macro. Without this fix, the InitListExprIsPostOrderNoQueueVisitedTwice
test was failing because WalkUpFromInitListExpr was never called.

[ymandel]

clang/include/clang/AST/RecursiveASTVisitor.h

335

Why use var-args rather than spelling out the type arguments like you have on lines 339-341 or, simpler, line 351?

339

Given that we've established them to be the same type, why two sets of template arguments?

638–657

Do you explain this logic somewhere? Or do you feel it will be obvious to the reader? (I don't have a good intuition about this class to judge).

[gribozavr]

Addressed review comments, added more fixes that must be committed in the same
change, because splitting them would break tests.

[gribozavr2]

clang/include/clang/AST/RecursiveASTVisitor.h

335

Because all of those things are not relevant. However I do see your point, that in some sense it is a trick to reduce the number of characters, but somewhat hurting readability, so I changed the signature to be explicit.

339

Indeed, simplified the code now!

638–657

Added a comment. The RecursiveASTVisitor is full of tricky logic so no, none of this is obvious.

[ymandel]

clang/include/clang/AST/RecursiveASTVisitor.h

646

delete stray period after "children"

[gribozavr]

Removed a stray period in a comment.

[gribozavr2]

clang/include/clang/AST/RecursiveASTVisitor.h

646

Done.

[gribozavr]

Rebased the patch.

[gribozavr]

Added a FIXME about a regression.

[gribozavr]

Rebased the patch.

clang/include/clang/AST/RecursiveASTVisitor.h

 
 private:
   template<typename T, typename U>
   struct has_same_member_pointer_type : std::false_type {};
   template<typename T, typename U, typename R, typename... P>
   struct has_same_member_pointer_type<R (T::*)(P...), R (U::*)(P...)>
       : std::true_type {};
 
+  template <bool has_same_type> struct is_same_method_impl {
+    template <typename FirstMethodPtrTy, typename SecondMethodPtrTy>
+    static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+                             SecondMethodPtrTy SecondMethodPtr) {
+      return false;
+    }
+  };
+
+  template <> struct is_same_method_impl<true> {
+    template <typename FirstMethodPtrTy, typename SecondMethodPtrTy>
+    static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+                             SecondMethodPtrTy SecondMethodPtr) {
+      return FirstMethodPtr == SecondMethodPtr;
+    }
+  };
+
+  /// Returns true if and only if \p FirstMethodPtr and \p SecondMethodPtr
+  /// are pointers to the same non-static member function.
+  template <typename FirstMethodPtrTy, typename SecondMethodPtrTy>
+  bool isSameMethod(FirstMethodPtrTy FirstMethodPtr,
+                    SecondMethodPtrTy SecondMethodPtr) {
+    return is_same_method_impl<
+        has_same_member_pointer_type<FirstMethodPtrTy, SecondMethodPtrTy>::
+            value>::isSameMethod(FirstMethodPtr, SecondMethodPtr);
+  }
+
   // Traverse the given statement. If the most-derived traverse function takes a
   // data recursion queue, pass it on; otherwise, discard it. Note that the
   // first branch of this conditional must compile whether or not the derived
   // class can take a queue, so if we're taking the second arm, make the first
   // arm call our function rather than the derived class version.
 #define TRAVERSE_STMT_BASE(NAME, CLASS, VAR, QUEUE)                            \
   (has_same_member_pointer_type<decltype(                                      \
                                     &RecursiveASTVisitor::Traverse##NAME),     \

 template <typename Derived>
 bool RecursiveASTVisitor<Derived>::PostVisitStmt(Stmt *S) {
   switch (S->getStmtClass()) {
   case Stmt::NoStmtClass:
     break;
 #define ABSTRACT_STMT(STMT)
 #define STMT(CLASS, PARENT)                                                    \
   case Stmt::CLASS##Class:                                                     \
-    TRY_TO(WalkUpFrom##CLASS(static_cast<CLASS *>(S))); break;
+    /* In pre-order traversal mode, each Traverse##STMT method is responsible  \
+     * for calling WalkUpFrom. Therefore, if the user overrides Traverse##STMT \
+     * and does not call the default implementation, the WalkUpFrom callback   \
+     * is not called. Post-order traversal mode should provide the same        \
+     * behavior regarding method overrides.                                    \
+     *                                                                         \
+     * In post-order traversal mode the Traverse##STMT method, when it         \
+     * receives a DataRecursionQueue, can't call WalkUpFrom after traversing   \
+     * children because it only enqueues the children and does not traverse    \
+     * them. TraverseStmt traverses the enqueued children, and we call         \
+     * WalkUpFrom here.                                                        \
+     *                                                                         \
+     * However, to make pre-order and post-order modes identical with regards  \
+     * to whether they call WalkUpFrom at all, we call WalkUpFrom if and only  \
+     * if the user did not override the Traverse##STMT method. */              \
+    if (isSameMethod(&RecursiveASTVisitor::Traverse##CLASS,                    \
+                     &Derived::Traverse##CLASS)) {                             \
+      TRY_TO(WalkUpFrom##CLASS(static_cast<CLASS *>(S)));                      \
+    }                                                                          \
+    break;
 #define INITLISTEXPR(CLASS, PARENT)                                            \
   case Stmt::CLASS##Class:                                                     \
-    {                                                                          \
+    /* See the comment above for the explanation of the isSameMethod check. */ \
+    if (isSameMethod(&RecursiveASTVisitor::Traverse##CLASS,                    \
+                     &Derived::Traverse##CLASS)) {                             \
       auto ILE = static_cast<CLASS *>(S);                                      \
       if (auto Syn = ILE->isSemanticForm() ? ILE->getSyntacticForm() : ILE)    \
         TRY_TO(WalkUpFrom##CLASS(Syn));                                        \
       if (auto Sem = ILE->isSemanticForm() ? ILE : ILE->getSemanticForm())     \
         TRY_TO(WalkUpFrom##CLASS(Sem));                                        \
-      break;                                                                   \
-    }
+    }                                                                          \
+    break;
 #include "clang/AST/StmtNodes.inc"
   }
 
   return true;
 }
 
 #undef DISPATCH_STMT
 
bool RecursiveASTVisitor<Derived>::Traverse##STMT(                           \
     if (!getDerived().shouldTraversePostOrder())                               \
       TRY_TO(WalkUpFrom##STMT(S));                                             \
     { CODE; }                                                                  \
     if (ShouldVisitChildren) {                                                 \
       for (Stmt * SubStmt : getDerived().getStmtChildren(S)) {                 \
         TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt);                              \
       }                                                                        \
     }                                                                          \
-    if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder())       \
+    /* Call WalkUpFrom if TRY_TO_TRAVERSE_OR_ENQUEUE_STMT has traversed the    \
+     * children already. If TRY_TO_TRAVERSE_OR_ENQUEUE_STMT only enqueued the  \
+     * children, PostVisitStmt will call WalkUpFrom after we are done visiting \
+     * children. */                                                            \
+    if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder()) {     \
       TRY_TO(WalkUpFrom##STMT(S));                                             \
+    }                                                                          \
     return ReturnValue;                                                        \
   }
 
 DEF_TRAVERSE_STMT(GCCAsmStmt, {
   TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getAsmString());
   for (unsigned I = 0, E = S->getNumInputs(); I < E; ++I) {
     TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getInputConstraintLiteral(I));
   }
if (S) {
     // in PostVisitStmt.
     if (!getDerived().shouldTraversePostOrder())
       TRY_TO(WalkUpFromInitListExpr(S));
 
     // All we need are the default actions.  FIXME: use a helper function.
     for (Stmt *SubStmt : S->children()) {
       TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt);
     }
+
+    if (!Queue && getDerived().shouldTraversePostOrder())
+      TRY_TO(WalkUpFromInitListExpr(S));
   }
   return true;
 }
 
 template<typename Derived>
 bool RecursiveASTVisitor<Derived>::TraverseConceptReference(
     const ConceptReference &C) {
   TRY_TO(TraverseNestedNameSpecifierLoc(C.getNestedNameSpecifierLoc()));

clang/lib/Tooling/Syntax/BuildTree.cpp

bool WalkUpFromStmt(Stmt *S) {
     return true;
   }
 
   bool TraverseCXXForRangeStmt(CXXForRangeStmt *S) {
     // We override to traverse range initializer as VarDecl.
     // RAV traverses it as a statement, we produce invalid node kinds in that
     // case.
     // FIXME: should do this in RAV instead?
+    bool Result = [&, this]() {
       if (S->getInit() && !TraverseStmt(S->getInit()))
         return false;
       if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable()))
         return false;
       if (S->getRangeInit() && !TraverseStmt(S->getRangeInit()))
         return false;
       if (S->getBody() && !TraverseStmt(S->getBody()))
         return false;
       return true;
+    }();
+    WalkUpFromCXXForRangeStmt(S);
+    return Result;
   }
 
   bool TraverseStmt(Stmt *S) {
     if (auto *DS = llvm::dyn_cast_or_null<DeclStmt>(S)) {
       // We want to consume the semicolon, make sure SimpleDeclaration does not.
       for (auto *D : DS->decls())
         Builder.noticeDeclWithoutSemicolon(D);
     } else if (auto *E = llvm::dyn_cast_or_null<Expr>(S)) {

clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_TraverseLeaf_WalkUpFromLeaf) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
)txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 TraverseIntegerLiteral IntegerLiteral(1)
   WalkUpFromIntegerLiteral IntegerLiteral(1)
     WalkUpFromExpr IntegerLiteral(1)
       WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromIntegerLiteral IntegerLiteral(1)
-  WalkUpFromExpr IntegerLiteral(1)
-    WalkUpFromStmt IntegerLiteral(1)
 TraverseIntegerLiteral IntegerLiteral(2)
   WalkUpFromIntegerLiteral IntegerLiteral(2)
     WalkUpFromExpr IntegerLiteral(2)
       WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromIntegerLiteral IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(2)
-    WalkUpFromStmt IntegerLiteral(2)
 TraverseIntegerLiteral IntegerLiteral(3)
   WalkUpFromIntegerLiteral IntegerLiteral(3)
     WalkUpFromExpr IntegerLiteral(3)
       WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromIntegerLiteral IntegerLiteral(3)
-  WalkUpFromExpr IntegerLiteral(3)
-    WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr BinaryOperator(+)
   WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr DeclRefExpr(add)
   WalkUpFromStmt DeclRefExpr(add)
 WalkUpFromExpr ImplicitCastExpr
   WalkUpFromStmt ImplicitCastExpr
 TraverseIntegerLiteral IntegerLiteral(4)
   WalkUpFromIntegerLiteral IntegerLiteral(4)
     WalkUpFromExpr IntegerLiteral(4)
       WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromIntegerLiteral IntegerLiteral(4)
-  WalkUpFromExpr IntegerLiteral(4)
-    WalkUpFromStmt IntegerLiteral(4)
 TraverseIntegerLiteral IntegerLiteral(5)
   WalkUpFromIntegerLiteral IntegerLiteral(5)
     WalkUpFromExpr IntegerLiteral(5)
       WalkUpFromStmt IntegerLiteral(5)
-WalkUpFromIntegerLiteral IntegerLiteral(5)
-  WalkUpFromExpr IntegerLiteral(5)
-    WalkUpFromStmt IntegerLiteral(5)
 WalkUpFromExpr CallExpr(add)
   WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromLeaf) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
EXPECT_TRUE(visitorCallbackLogEqual(
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt UnaryOperator(-)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // UnaryOperator(-).
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt UnaryOperator(-)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor,
      StmtCallbacks_TraverseUnaryOperator_WalkUpFromUnaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
WalkUpFromUnaryOperator UnaryOperator(-)
   WalkUpFromExpr UnaryOperator(-)
     WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromUnaryOperator
+  // for UnaryyOperator(-).
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromUnaryOperator UnaryOperator(-)
-  WalkUpFromExpr UnaryOperator(-)
-    WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromUnaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {

 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // BinaryOperator(+).
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor,
      StmtCallbacks_TraverseBinaryOperator_WalkUpFromBinaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {

 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromBinaryOperator.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromBinaryOperator BinaryOperator(+)
-  WalkUpFromExpr BinaryOperator(+)
-    WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {

 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromStmt DeclRefExpr(a)
 WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // CompoundAssignOperator(+=).
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromStmt DeclRefExpr(a)
 WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(
     RecursiveASTVisitor,
     StmtCallbacks_TraverseCompoundAssignOperator_WalkUpFromCompoundAssignOperator) {

 WalkUpFromExpr DeclRefExpr(a)
   WalkUpFromStmt DeclRefExpr(a)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to
+  // WalkUpFromCompoundAssignOperator.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromExpr DeclRefExpr(a)
   WalkUpFromStmt DeclRefExpr(a)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
-  WalkUpFromExpr CompoundAssignOperator(+=)
-    WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromCompoundAssignOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {

 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt BinaryOperator(+)
 TraverseCallExpr CallExpr(add)
   WalkUpFromStmt DeclRefExpr(add)
   WalkUpFromStmt ImplicitCastExpr
   WalkUpFromStmt IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(5)
   WalkUpFromStmt CallExpr(add)
-WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_TraverseCallExpr_WalkUpFromCallExpr) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
WalkUpFromExpr ImplicitCastExpr
     WalkUpFromStmt ImplicitCastExpr
   WalkUpFromExpr IntegerLiteral(4)
     WalkUpFromStmt IntegerLiteral(4)
   WalkUpFromExpr IntegerLiteral(5)
     WalkUpFromStmt IntegerLiteral(5)
   WalkUpFromCallExpr CallExpr(add)
     WalkUpFromExpr CallExpr(add)
       WalkUpFromStmt CallExpr(add)
-WalkUpFromCallExpr CallExpr(add)
-  WalkUpFromExpr CallExpr(add)
-    WalkUpFromStmt CallExpr(add)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromCallExpr) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)