Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

[gribozavr]

Summary

This feature was only used in two places, but contributed a non-trivial
amount to the complexity of RecursiveASTVisitor, and was buggy (see my
recent patches where I was fixing the bugs that I noticed). I don't
think the convenience benefit of this feature is worth the complexity.

Besides complexity, another issue with the current state of RecursiveASTVisitor is the non-uniformity in how it handles different AST nodes. All AST nodes follow a regular pattern, but operators are special -- and this special behavior not documented. Correct usage of RecursiveASTVisitor relies on shadowing member functions with specific names and signatures. Near misses don't cause any compile-time errors, incorrectly named or typed methods are just silently ignored. Therefore, predictability of RecursiveASTVisitor API is quite important.

This change reduces the size of the clang binary by 38 KB (0.2%) in
release mode, and by 7 MB (0.3%) in debug mode. The clang-tidy binary
is reduced by 205 KB (0.3%) in release mode, and by 5 MB (0.4%) in debug
mode. I don't think these code size improvements are significant enough
to justify this change on its own (for me, the primary motivation is
reducing code complexity), but they I think are a nice side-effect.

[gribozavr]

Removed obsolete comments.

[sammccall]

+1 to the complexity argument, and also for conceptual complexity: there's a lot of value in RAV being regular and predictable given that both its implementation and the AST itself are complex.

I think this should be mentioned in the release notes, as I think it will silently change the behavior of programs that are using these hooks.

[aaron.ballman]

LGTM, I think the complexity argument is reasonable (and I appreciate the information showing this also reduces binary size at the same time).

[ymandel]

+1 Especially given how trivial it was to fix clang/lib/ARCMigrate/TransProperties.cpp, this functionality seems unjustified.

[rsmith]

This introduces a non-uniformity between RecursiveASTVisitor and StmtVisitor. StmtVisitor contains matching logic to treat the various kinds of unary and binary operators as if they were distinct AST node kinds, and that functionality is used much more heavily throughout Clang. Given that, I don't think this change necessarily improves big-picture uniformity, but the reduction in complexity and binary size still seem sufficiently compelling to me.

[gribozavr]

Added release notes.

clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h

private:
 
   /// Overriden methods for RecursiveASTVisitor's traversal.
   bool TraverseArraySubscriptExpr(ArraySubscriptExpr *E);
   bool TraverseCXXMemberCallExpr(CXXMemberCallExpr *MemberCall);
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *OpCall);
   bool TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C,
                              Expr *Init);
   bool TraverseMemberExpr(MemberExpr *Member);
-  bool TraverseUnaryDeref(UnaryOperator *Uop);
+  bool TraverseUnaryOperator(UnaryOperator *Uop);
   bool VisitDeclRefExpr(DeclRefExpr *E);
   bool VisitDeclStmt(DeclStmt *S);
   bool TraverseStmt(Stmt *S);
 
   /// Add an expression to the list of expressions on which the container
   /// expression depends.
   void addComponent(const Expr *E);
 

clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp

 /// For example, if container.begin() and container.end() both return pointers
 /// to int, this makes sure that the initialization for `k` is not counted as an
 /// unconvertible use of the iterator `i`.
 /// \code
 ///   for (int *i = container.begin(), *e = container.end(); i != e; ++i) {
 ///     int k = *i + 2;
 ///   }
 /// \endcode
-bool ForLoopIndexUseVisitor::TraverseUnaryDeref(UnaryOperator *Uop) {
+bool ForLoopIndexUseVisitor::TraverseUnaryOperator(UnaryOperator *Uop) {
   // If we dereference an iterator that's actually a pointer, count the
   // occurrence.
   if (isDereferenceOfUop(Uop, IndexVar)) {
     addUsage(Usage(Uop));
     return true;
   }
 
   return VisitorBase::TraverseUnaryOperator(Uop);

clang/docs/ReleaseNotes.rst

 
 Internal API Changes
 --------------------
 
 These are major API changes that have happened since the 10.0.0 release of
 Clang. If upgrading an external codebase that uses Clang as a library,
 this section should help get you past the largest hurdles of upgrading.
 
+- ``RecursiveASTVisitor`` no longer calls separate methods to visit specific
+  operator kinds. Previously, ``RecursiveASTVisitor`` treated unary, binary,
+  and compound assignment operators as if they were subclasses of the
+  corresponding AST node. For example, the binary operator plus was treated as
+  if it was a ``BinAdd`` subclass of the ``BinaryOperator`` class: during AST
+  traversal of a ``BinaryOperator`` AST node that had a ``BO_Add`` opcode,
+  ``RecursiveASTVisitor`` was calling the ``TraverseBinAdd`` method instead of
+  ``TraverseBinaryOperator``. This feature was contributing a non-trivial
+  amount of complexity to the implementation of ``RecursiveASTVisitor``, it was
+  used only in a minor way in Clang, was not tested, and as a result it was
+  buggy. Furthermore, this feature was creating a non-uniformity in the API.
+  Since this feature was not documented, it was quite difficult to figure out
+  how to use ``RecursiveASTVisitor`` to visit operators.
+
+  To update your code to the new uniform API, move the code from separate
+  visitation methods into methods that correspond to the actual AST node and
+  perform case analysis based on the operator opcode as needed:
+
+  * ``TraverseUnary*() => TraverseUnaryOperator()``
+  * ``WalkUpFromUnary*() => WalkUpFromUnaryOperator()``
+  * ``VisitUnary*() => VisiUnaryOperator()``
+  * ``TraverseBin*() => TraverseBinaryOperator()``
+  * ``WalkUpFromBin*() => WalkUpFromBinaryOperator()``
+  * ``VisitBin*() => VisiBinaryOperator()``
+  * ``TraverseBin*Assign() => TraverseCompoundAssignOperator()``
+  * ``WalkUpFromBin*Assign() => WalkUpFromCompoundAssignOperator()``
+  * ``VisitBin*Assign() => VisiCompoundAssignOperator()``
+
 Build System Changes
 --------------------
 
 These are major changes to the build system that have happened since the 10.0.0
 release of Clang. Users of the build system should adjust accordingly.
 
 - clang-tidy and clang-include-fixer are no longer compiled into libclang by
   default. You can set ``LIBCLANG_INCLUDE_CLANG_TOOLS_EXTRA=ON`` to undo that,

clang/include/clang/AST/RecursiveASTVisitor.h

 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include <algorithm>
 #include <cstddef>
 #include <type_traits>
 
-// The following three macros are used for meta programming.  The code
-// using them is responsible for defining macro OPERATOR().
-
-// All unary operators.
-#define UNARYOP_LIST()                                                         \
-  OPERATOR(PostInc) OPERATOR(PostDec) OPERATOR(PreInc) OPERATOR(PreDec)        \
-      OPERATOR(AddrOf) OPERATOR(Deref) OPERATOR(Plus) OPERATOR(Minus)          \
-      OPERATOR(Not) OPERATOR(LNot) OPERATOR(Real) OPERATOR(Imag)               \
-      OPERATOR(Extension) OPERATOR(Coawait)
-
-// All binary operators (excluding compound assign operators).
-#define BINOP_LIST()                                                           \
-  OPERATOR(PtrMemD) OPERATOR(PtrMemI) OPERATOR(Mul) OPERATOR(Div)              \
-      OPERATOR(Rem) OPERATOR(Add) OPERATOR(Sub) OPERATOR(Shl) OPERATOR(Shr)    \
-      OPERATOR(LT) OPERATOR(GT) OPERATOR(LE) OPERATOR(GE) OPERATOR(EQ)         \
-      OPERATOR(NE) OPERATOR(Cmp) OPERATOR(And) OPERATOR(Xor) OPERATOR(Or)      \
-      OPERATOR(LAnd) OPERATOR(LOr) OPERATOR(Assign) OPERATOR(Comma)
-
-// All compound assign operators.
-#define CAO_LIST()                                                             \
-  OPERATOR(Mul) OPERATOR(Div) OPERATOR(Rem) OPERATOR(Add) OPERATOR(Sub)        \
-      OPERATOR(Shl) OPERATOR(Shr) OPERATOR(And) OPERATOR(Or) OPERATOR(Xor)
-
 namespace clang {
 
 // A helper macro to implement short-circuiting when recursing.  It
 // invokes CALL_EXPR, which must be a method call, on the derived
 // object (s.t. a user of RecursiveASTVisitor can override the method
 // in CALL_EXPR).
 #define TRY_TO(CALL_EXPR)                                                      \
   do {                                                                         \
#define STMT(CLASS, PARENT)                                                    \
   bool WalkUpFrom##CLASS(CLASS *S) {                                           \
     TRY_TO(WalkUpFrom##PARENT(S));                                             \
     TRY_TO(Visit##CLASS(S));                                                   \
     return true;                                                               \
   }                                                                            \
   bool Visit##CLASS(CLASS *S) { return true; }
 #include "clang/AST/StmtNodes.inc"
 
-// Define Traverse*(), WalkUpFrom*(), and Visit*() for unary
-// operator methods.  Unary operators are not classes in themselves
-// (they're all opcodes in UnaryOperator) but do have visitors.
-#define OPERATOR(NAME)                                                         \
-  bool TraverseUnary##NAME(UnaryOperator *S,                                   \
-                           DataRecursionQueue *Queue = nullptr) {              \
-    if (!getDerived().shouldTraversePostOrder())                               \
-      TRY_TO(WalkUpFromUnary##NAME(S));                                        \
-    TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getSubExpr());                          \
-    if (!Queue && getDerived().shouldTraversePostOrder())                      \
-      TRY_TO(WalkUpFromUnary##NAME(S));                                        \
-    return true;                                                               \
-  }                                                                            \
-  bool WalkUpFromUnary##NAME(UnaryOperator *S) {                               \
-    TRY_TO(WalkUpFromUnaryOperator(S));                                        \
-    TRY_TO(VisitUnary##NAME(S));                                               \
-    return true;                                                               \
-  }                                                                            \
-  bool VisitUnary##NAME(UnaryOperator *S) { return true; }
-
-  UNARYOP_LIST()
-#undef OPERATOR
-
-// Define Traverse*(), WalkUpFrom*(), and Visit*() for binary
-// operator methods.  Binary operators are not classes in themselves
-// (they're all opcodes in BinaryOperator) but do have visitors.
-#define GENERAL_BINOP_FALLBACK(NAME, BINOP_TYPE)                               \
-  bool TraverseBin##NAME(BINOP_TYPE *S, DataRecursionQueue *Queue = nullptr) { \
-    if (!getDerived().shouldTraversePostOrder())                               \
-      TRY_TO(WalkUpFromBin##NAME(S));                                          \
-    TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getLHS());                              \
-    TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getRHS());                              \
-    if (!Queue && getDerived().shouldTraversePostOrder())                      \
-      TRY_TO(WalkUpFromBin##NAME(S));                                          \
-    return true;                                                               \
-  }                                                                            \
-  bool WalkUpFromBin##NAME(BINOP_TYPE *S) {                                    \
-    TRY_TO(WalkUpFrom##BINOP_TYPE(S));                                         \
-    TRY_TO(VisitBin##NAME(S));                                                 \
-    return true;                                                               \
-  }                                                                            \
-  bool VisitBin##NAME(BINOP_TYPE *S) { return true; }
-
-#define OPERATOR(NAME) GENERAL_BINOP_FALLBACK(NAME, BinaryOperator)
-  BINOP_LIST()
-#undef OPERATOR
-
-// Define Traverse*(), WalkUpFrom*(), and Visit*() for compound
-// assignment methods.  Compound assignment operators are not
-// classes in themselves (they're all opcodes in
-// CompoundAssignOperator) but do have visitors.
-#define OPERATOR(NAME)                                                         \
-  GENERAL_BINOP_FALLBACK(NAME##Assign, CompoundAssignOperator)
-
-  CAO_LIST()
-#undef OPERATOR
-#undef GENERAL_BINOP_FALLBACK
-
 // ---- Methods on Types ----
 // FIXME: revamp to take TypeLoc's rather than Types.
 
 // Declare Traverse*() for all concrete Type classes.
 #define ABSTRACT_TYPE(CLASS, BASE)
 #define TYPE(CLASS, BASE) bool Traverse##CLASS##Type(CLASS##Type *T);
 #include "clang/AST/TypeNodes.inc"
   // The above header #undefs ABSTRACT_TYPE and TYPE upon exit.
#include "llvm/Frontend/OpenMP/OMPKinds.def"
 
   bool dataTraverseNode(Stmt *S, DataRecursionQueue *Queue);
   bool PostVisitStmt(Stmt *S);
 };
 
 template <typename Derived>
 bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,
                                                     DataRecursionQueue *Queue) {
-#define DISPATCH_STMT(NAME, CLASS, VAR)                                        \
-  return TRAVERSE_STMT_BASE(NAME, CLASS, VAR, Queue);
-
-  // If we have a binary expr, dispatch to the subcode of the binop.  A smart
-  // optimizer (e.g. LLVM) will fold this comparison into the switch stmt
-  // below.
-  if (BinaryOperator *BinOp = dyn_cast<BinaryOperator>(S)) {
-    switch (BinOp->getOpcode()) {
-#define OPERATOR(NAME)                                                         \
-  case BO_##NAME:                                                              \
-    DISPATCH_STMT(Bin##NAME, BinaryOperator, S);
-
-      BINOP_LIST()
-#undef OPERATOR
-
-#define OPERATOR(NAME)                                                         \
-  case BO_##NAME##Assign:                                                      \
-    DISPATCH_STMT(Bin##NAME##Assign, CompoundAssignOperator, S);
-
-      CAO_LIST()
-#undef OPERATOR
-    }
-  } else if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(S)) {
-    switch (UnOp->getOpcode()) {
-#define OPERATOR(NAME)                                                         \
-  case UO_##NAME:                                                              \
-    DISPATCH_STMT(Unary##NAME, UnaryOperator, S);
-
-      UNARYOP_LIST()
-#undef OPERATOR
-    }
-  }
-
   // Top switch stmt: dispatch to TraverseFooStmt for each concrete FooStmt.
   switch (S->getStmtClass()) {
   case Stmt::NoStmtClass:
     break;
 #define ABSTRACT_STMT(STMT)
 #define STMT(CLASS, PARENT)                                                    \
   case Stmt::CLASS##Class:                                                     \
-    DISPATCH_STMT(CLASS, CLASS, S);
+    return TRAVERSE_STMT_BASE(CLASS, CLASS, S, Queue);
 #include "clang/AST/StmtNodes.inc"
   }
 
   return true;
 }
 
 #undef DISPATCH_STMT
 
bool RecursiveASTVisitor<Derived>::PostVisitStmt(Stmt *S) {
   // 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. We implement the override
   // check with isSameMethod calls below.
 
-  if (BinaryOperator *BinOp = dyn_cast<BinaryOperator>(S)) {
-    switch (BinOp->getOpcode()) {
-#define OPERATOR(NAME)                                                         \
-  case BO_##NAME:                                                              \
-    if (::clang::detail::isSameMethod(&RecursiveASTVisitor::TraverseBin##NAME, \
-                                      &Derived::TraverseBin##NAME)) {          \
-      TRY_TO(WalkUpFromBin##NAME(static_cast<BinaryOperator *>(S)));           \
-    }                                                                          \
-    return true;
-
-      BINOP_LIST()
-#undef OPERATOR
-
-#define OPERATOR(NAME)                                                         \
-  case BO_##NAME##Assign:                                                      \
-    if (::clang::detail::isSameMethod(                                         \
-            &RecursiveASTVisitor::TraverseBin##NAME##Assign,                   \
-            &Derived::TraverseBin##NAME##Assign)) {                            \
-      TRY_TO(WalkUpFromBin##NAME##Assign(                                      \
-          static_cast<CompoundAssignOperator *>(S)));                          \
-    }                                                                          \
-    return true;
-
-      CAO_LIST()
-#undef OPERATOR
-    }
-  } else if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(S)) {
-    switch (UnOp->getOpcode()) {
-#define OPERATOR(NAME)                                                         \
-  case UO_##NAME:                                                              \
-    if (::clang::detail::isSameMethod(                                         \
-            &RecursiveASTVisitor::TraverseUnary##NAME,                         \
-            &Derived::TraverseUnary##NAME)) {                                  \
-      TRY_TO(WalkUpFromUnary##NAME(static_cast<UnaryOperator *>(S)));          \
-    }                                                                          \
-    return true;
-
-      UNARYOP_LIST()
-#undef OPERATOR
-    }
-  }
-
   switch (S->getStmtClass()) {
   case Stmt::NoStmtClass:
     break;
 #define ABSTRACT_STMT(STMT)
 #define STMT(CLASS, PARENT)                                                    \
   case Stmt::CLASS##Class:                                                     \
     if (::clang::detail::isSameMethod(&RecursiveASTVisitor::Traverse##CLASS,   \
                                       &Derived::Traverse##CLASS)) {            \

 //    Every class that has getQualifier.
 
 #undef DEF_TRAVERSE_STMT
 #undef TRAVERSE_STMT
 #undef TRAVERSE_STMT_BASE
 
 #undef TRY_TO
 
-#undef UNARYOP_LIST
-#undef BINOP_LIST
-#undef CAO_LIST
-
 } // end namespace clang
 
 #endif // LLVM_CLANG_AST_RECURSIVEASTVISITOR_H

clang/lib/ARCMigrate/TransProperties.cpp

bool addAttribute(StringRef attr, SourceLocation atLoc) const {
     return MigrateCtx.addPropertyAttribute(attr, atLoc);
   }
 
   class PlusOneAssign : public RecursiveASTVisitor<PlusOneAssign> {
     ObjCIvarDecl *Ivar;
   public:
     PlusOneAssign(ObjCIvarDecl *D) : Ivar(D) {}
 
-    bool VisitBinAssign(BinaryOperator *E) {
+    bool VisitBinaryOperator(BinaryOperator *E) {
+      if (E->getOpcode() != BO_Assign)
+        return true;
+
       Expr *lhs = E->getLHS()->IgnoreParenImpCasts();
       if (ObjCIvarRefExpr *RE = dyn_cast<ObjCIvarRefExpr>(lhs)) {
         if (RE->getDecl() != Ivar)
           return true;
 
         if (isPlusOneAssign(E))
           return false;
       }

clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

TEST(RecursiveASTVisitor, StmtCallbacks_TraverseUnaryOperator) {
   StringRef Code = R"cpp(
 void test() {
   1;
   -2;
   3;
 }
 )cpp";
 
-  // TraverseUnaryOperator is not called because RecursiveASTVisitor treats
-  // individual operators as subclasses, for which it calls their Traverse
-  // methods.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::No), Code,
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
+TraverseUnaryOperator UnaryOperator(-)
   WalkUpFromStmt UnaryOperator(-)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
+TraverseUnaryOperator UnaryOperator(-)
   WalkUpFromStmt IntegerLiteral(2)
   WalkUpFromStmt UnaryOperator(-)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor,
      StmtCallbacks_TraverseUnaryOperator_WalkUpFromUnaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
TEST(RecursiveASTVisitor,
   StringRef Code = R"cpp(
 void test() {
   1;
   -2;
   3;
 }
 )cpp";
 
-  // TraverseUnaryOperator is not called because RecursiveASTVisitor treats
-  // individual operators as subclasses, for which it calls their Traverse
-  // methods.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::No), Code,
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
+TraverseUnaryOperator UnaryOperator(-)
   WalkUpFromUnaryOperator UnaryOperator(-)
     WalkUpFromExpr UnaryOperator(-)
       WalkUpFromStmt UnaryOperator(-)
   WalkUpFromExpr IntegerLiteral(2)
     WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
+TraverseUnaryOperator UnaryOperator(-)
   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> {
WalkUpFromUnaryOperator UnaryOperator(-)
   WalkUpFromExpr UnaryOperator(-)
     WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor, StmtCallbacks_TraverseUnaryMinus) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseUnaryMinus(UnaryOperator *UO) {
-      recordCallback(__func__, UO,
-                     [&]() { RecordingVisitorBase::TraverseUnaryMinus(UO); });
-      return true;
-    }
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test() {
-  1;
-  -2;
-  3;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromStmt IntegerLiteral(1)
-TraverseUnaryMinus UnaryOperator(-)
-  WalkUpFromStmt UnaryOperator(-)
-  WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(3)
-)txt"));
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
-      R"txt(
-WalkUpFromStmt IntegerLiteral(1)
-TraverseUnaryMinus UnaryOperator(-)
-  WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromStmt UnaryOperator(-)
-WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
-TEST(RecursiveASTVisitor,
-     StmtCallbacks_TraverseUnaryMinus_WalkUpFromUnaryMinus) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseUnaryMinus(UnaryOperator *UO) {
-      recordCallback(__func__, UO,
-                     [&]() { RecordingVisitorBase::TraverseUnaryMinus(UO); });
-      return true;
-    }
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-
-    bool WalkUpFromExpr(Expr *E) {
-      recordCallback(__func__, E,
-                     [&]() { RecordingVisitorBase::WalkUpFromExpr(E); });
-      return true;
-    }
-
-    bool WalkUpFromUnaryMinus(UnaryOperator *UO) {
-      recordCallback(__func__, UO,
-                     [&]() { RecordingVisitorBase::WalkUpFromUnaryMinus(UO); });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test() {
-  1;
-  -2;
-  3;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-TraverseUnaryMinus UnaryOperator(-)
-  WalkUpFromUnaryMinus UnaryOperator(-)
-    WalkUpFromExpr UnaryOperator(-)
-      WalkUpFromStmt UnaryOperator(-)
-  WalkUpFromExpr IntegerLiteral(2)
-    WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-)txt"));
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
-      R"txt(
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-TraverseUnaryMinus UnaryOperator(-)
-  WalkUpFromExpr IntegerLiteral(2)
-    WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromUnaryMinus UnaryOperator(-)
-    WalkUpFromExpr UnaryOperator(-)
-      WalkUpFromStmt UnaryOperator(-)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
-TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromUnaryMinus) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-
-    bool WalkUpFromExpr(Expr *E) {
-      recordCallback(__func__, E,
-                     [&]() { RecordingVisitorBase::WalkUpFromExpr(E); });
-      return true;
-    }
-
-    bool WalkUpFromUnaryMinus(UnaryOperator *UO) {
-      recordCallback(__func__, UO,
-                     [&]() { RecordingVisitorBase::WalkUpFromUnaryMinus(UO); });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test() {
-  1;
-  -2;
-  3;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromUnaryMinus UnaryOperator(-)
-  WalkUpFromExpr UnaryOperator(-)
-    WalkUpFromStmt UnaryOperator(-)
-WalkUpFromExpr IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-)txt"));
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
-      R"txt(
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromExpr IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromUnaryMinus UnaryOperator(-)
-  WalkUpFromExpr UnaryOperator(-)
-    WalkUpFromStmt UnaryOperator(-)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
 TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
     bool TraverseBinaryOperator(BinaryOperator *BO) {
       recordCallback(__func__, BO, [&]() {
TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinaryOperator) {
   StringRef Code = R"cpp(
 void test() {
   1;
   2 + 3;
   4;
 }
 )cpp";
 
-  // TraverseBinaryOperator is not called because RecursiveASTVisitor treats
-  // individual operators as subclasses, for which it calls their Traverse
-  // methods.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::No), Code,
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
+TraverseBinaryOperator BinaryOperator(+)
   WalkUpFromStmt BinaryOperator(+)
   WalkUpFromStmt IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
+TraverseBinaryOperator BinaryOperator(+)
   WalkUpFromStmt IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(3)
   WalkUpFromStmt BinaryOperator(+)
 WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor,
      StmtCallbacks_TraverseBinaryOperator_WalkUpFromBinaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
TEST(RecursiveASTVisitor,
   StringRef Code = R"cpp(
 void test() {
   1;
   2 + 3;
   4;
 }
 )cpp";
 
-  // TraverseBinaryOperator is not called because RecursiveASTVisitor treats
-  // individual operators as subclasses, for which it calls their Traverse
-  // methods.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::No), Code,
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
+TraverseBinaryOperator BinaryOperator(+)
   WalkUpFromBinaryOperator BinaryOperator(+)
     WalkUpFromExpr BinaryOperator(+)
       WalkUpFromStmt BinaryOperator(+)
   WalkUpFromExpr IntegerLiteral(2)
     WalkUpFromStmt IntegerLiteral(2)
   WalkUpFromExpr IntegerLiteral(3)
     WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
+TraverseBinaryOperator BinaryOperator(+)
   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> {
WalkUpFromBinaryOperator BinaryOperator(+)
   WalkUpFromExpr BinaryOperator(+)
     WalkUpFromStmt BinaryOperator(+)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinAdd) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseBinAdd(BinaryOperator *BO) {
-      recordCallback(__func__, BO,
-                     [&]() { RecordingVisitorBase::TraverseBinAdd(BO); });
-      return true;
-    }
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test() {
-  1;
-  2 + 3;
-  4;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAdd BinaryOperator(+)
-  WalkUpFromStmt BinaryOperator(+)
-  WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(4)
-)txt"));
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
-      R"txt(
-WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAdd BinaryOperator(+)
-  WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(3)
-  WalkUpFromStmt BinaryOperator(+)
-WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
-TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinAdd_WalkUpFromBinAdd) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseBinAdd(BinaryOperator *BO) {
-      recordCallback(__func__, BO,
-                     [&]() { RecordingVisitorBase::TraverseBinAdd(BO); });
-      return true;
-    }
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-
-    bool WalkUpFromExpr(Expr *E) {
-      recordCallback(__func__, E,
-                     [&]() { RecordingVisitorBase::WalkUpFromExpr(E); });
-      return true;
-    }
-
-    bool WalkUpFromBinAdd(BinaryOperator *BO) {
-      recordCallback(__func__, BO,
-                     [&]() { RecordingVisitorBase::WalkUpFromBinAdd(BO); });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test() {
-  1;
-  2 + 3;
-  4;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAdd BinaryOperator(+)
-  WalkUpFromBinAdd BinaryOperator(+)
-    WalkUpFromExpr BinaryOperator(+)
-      WalkUpFromStmt BinaryOperator(+)
-  WalkUpFromExpr IntegerLiteral(2)
-    WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(3)
-    WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromExpr IntegerLiteral(4)
-  WalkUpFromStmt IntegerLiteral(4)
-)txt"));
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
-      R"txt(
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAdd BinaryOperator(+)
-  WalkUpFromExpr IntegerLiteral(2)
-    WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(3)
-    WalkUpFromStmt IntegerLiteral(3)
-  WalkUpFromBinAdd BinaryOperator(+)
-    WalkUpFromExpr BinaryOperator(+)
-      WalkUpFromStmt BinaryOperator(+)
-WalkUpFromExpr IntegerLiteral(4)
-  WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
-TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinAdd) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-
-    bool WalkUpFromExpr(Expr *E) {
-      recordCallback(__func__, E,
-                     [&]() { RecordingVisitorBase::WalkUpFromExpr(E); });
-      return true;
-    }
-
-    bool WalkUpFromBinAdd(BinaryOperator *BO) {
-      recordCallback(__func__, BO,
-                     [&]() { RecordingVisitorBase::WalkUpFromBinAdd(BO); });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test() {
-  1;
-  2 + 3;
-  4;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromBinAdd BinaryOperator(+)
-  WalkUpFromExpr BinaryOperator(+)
-    WalkUpFromStmt BinaryOperator(+)
-WalkUpFromExpr IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromExpr IntegerLiteral(4)
-  WalkUpFromStmt IntegerLiteral(4)
-)txt"));
-
-  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)
-WalkUpFromBinAdd BinaryOperator(+)
-  WalkUpFromExpr BinaryOperator(+)
-    WalkUpFromStmt BinaryOperator(+)
-WalkUpFromExpr IntegerLiteral(4)
-  WalkUpFromStmt IntegerLiteral(4)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
 TEST(RecursiveASTVisitor, StmtCallbacks_TraverseCompoundAssignOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
     bool TraverseCompoundAssignOperator(CompoundAssignOperator *CAO) {
       recordCallback(__func__, CAO, [&]() {
TEST(RecursiveASTVisitor, StmtCallbacks_TraverseCompoundAssignOperator) {
   StringRef Code = R"cpp(
 void test(int a) {
   1;
   a += 2;
   3;
 }
 )cpp";
 
-  // TraverseCompoundAssignOperator is not called because RecursiveASTVisitor
-  // treats individual operators as subclasses, for which it calls their
-  // Traverse methods.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::No), Code,
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromStmt CompoundAssignOperator(+=)
   WalkUpFromStmt DeclRefExpr(a)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromStmt DeclRefExpr(a)
   WalkUpFromStmt IntegerLiteral(2)
   WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(
     RecursiveASTVisitor,
     StmtCallbacks_TraverseCompoundAssignOperator_WalkUpFromCompoundAssignOperator) {
TEST(
   StringRef Code = R"cpp(
 void test(int a) {
   1;
   a += 2;
   3;
 }
 )cpp";
 
-  // TraverseCompoundAssignOperator is not called because RecursiveASTVisitor
-  // treats individual operators as subclasses, for which it calls their
-  // Traverse methods.
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::No), Code,
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
     WalkUpFromExpr CompoundAssignOperator(+=)
       WalkUpFromStmt CompoundAssignOperator(+=)
   WalkUpFromExpr DeclRefExpr(a)
     WalkUpFromStmt DeclRefExpr(a)
   WalkUpFromExpr IntegerLiteral(2)
     WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
   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> {
WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromExpr CompoundAssignOperator(+=)
     WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinAddAssign) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseBinAddAssign(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::TraverseBinAddAssign(CAO);
-      });
-      return true;
-    }
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test(int a) {
-  1;
-  a += 2;
-  3;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
-  WalkUpFromStmt CompoundAssignOperator(+=)
-  WalkUpFromStmt DeclRefExpr(a)
-  WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(3)
-)txt"));
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
-      R"txt(
-WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
-  WalkUpFromStmt DeclRefExpr(a)
-  WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
-TEST(RecursiveASTVisitor,
-     StmtCallbacks_TraverseBinAddAssign_WalkUpFromBinAddAssign) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseBinAddAssign(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::TraverseBinAddAssign(CAO);
-      });
-      return true;
-    }
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-
-    bool WalkUpFromExpr(Expr *E) {
-      recordCallback(__func__, E,
-                     [&]() { RecordingVisitorBase::WalkUpFromExpr(E); });
-      return true;
-    }
-
-    bool WalkUpFromBinAddAssign(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::WalkUpFromBinAddAssign(CAO);
-      });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test(int a) {
-  1;
-  a += 2;
-  3;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
-  WalkUpFromBinAddAssign CompoundAssignOperator(+=)
-    WalkUpFromExpr CompoundAssignOperator(+=)
-      WalkUpFromStmt CompoundAssignOperator(+=)
-  WalkUpFromExpr DeclRefExpr(a)
-    WalkUpFromStmt DeclRefExpr(a)
-  WalkUpFromExpr IntegerLiteral(2)
-    WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-)txt"));
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
-      R"txt(
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
-  WalkUpFromExpr DeclRefExpr(a)
-    WalkUpFromStmt DeclRefExpr(a)
-  WalkUpFromExpr IntegerLiteral(2)
-    WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromBinAddAssign CompoundAssignOperator(+=)
-    WalkUpFromExpr CompoundAssignOperator(+=)
-      WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
-TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinAddAssign) {
-  class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool WalkUpFromStmt(Stmt *S) {
-      recordCallback(__func__, S,
-                     [&]() { RecordingVisitorBase::WalkUpFromStmt(S); });
-      return true;
-    }
-
-    bool WalkUpFromExpr(Expr *E) {
-      recordCallback(__func__, E,
-                     [&]() { RecordingVisitorBase::WalkUpFromExpr(E); });
-      return true;
-    }
-
-    bool WalkUpFromBinAddAssign(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::WalkUpFromBinAddAssign(CAO);
-      });
-      return true;
-    }
-  };
-
-  StringRef Code = R"cpp(
-void test(int a) {
-  1;
-  a += 2;
-  3;
-}
-)cpp";
-
-  EXPECT_TRUE(visitorCallbackLogEqual(
-      RecordingVisitor(ShouldTraversePostOrder::No), Code,
-      R"txt(
-WalkUpFromStmt CompoundStmt
-WalkUpFromExpr IntegerLiteral(1)
-  WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromBinAddAssign CompoundAssignOperator(+=)
-  WalkUpFromExpr CompoundAssignOperator(+=)
-    WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromExpr DeclRefExpr(a)
-  WalkUpFromStmt DeclRefExpr(a)
-WalkUpFromExpr IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-)txt"));
-
-  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)
-WalkUpFromBinAddAssign CompoundAssignOperator(+=)
-  WalkUpFromExpr CompoundAssignOperator(+=)
-    WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt CompoundStmt
-)txt"));
-}
-
 TEST(RecursiveASTVisitor, StmtCallbacks_TraverseCallExpr) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
     bool TraverseCallExpr(CallExpr *CE) {
       recordCallback(__func__, CE,