[clang] 5689b38 - Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

Dmitri Gribenko via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 6 04:41:47 PDT 2020


Author: Dmitri Gribenko
Date: 2020-07-06T13:38:01+02:00
New Revision: 5689b38c6a4220cc5f6ba68a56486229b10071bf

URL: https://github.com/llvm/llvm-project/commit/5689b38c6a4220cc5f6ba68a56486229b10071bf
DIFF: https://github.com/llvm/llvm-project/commit/5689b38c6a4220cc5f6ba68a56486229b10071bf.diff

LOG: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

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.

Reviewers: rsmith, sammccall, ymandel, aaron.ballman

Reviewed By: rsmith, sammccall, ymandel, aaron.ballman

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82921

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
    clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
    clang/docs/ReleaseNotes.rst
    clang/include/clang/AST/RecursiveASTVisitor.h
    clang/lib/ARCMigrate/TransProperties.cpp
    clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index 67f281b3ed1f..56d4cceb6002 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -500,7 +500,7 @@ void ForLoopIndexUseVisitor::addUsage(const Usage &U) {
 ///     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)) {

diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index b2cba8c0172a..4f75c9c7522f 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -346,7 +346,7 @@ class ForLoopIndexUseVisitor
   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);

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d0328b0ef54c..8a9a58aa01f8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -243,6 +243,34 @@ 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 
diff icult 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
 --------------------
 

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 0d7c2b81512c..3dcfc9fee629 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -48,29 +48,6 @@
 #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
@@ -407,64 +384,6 @@ template <typename Derived> class RecursiveASTVisitor {
   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.
 
@@ -583,39 +502,6 @@ template <typename Derived> class RecursiveASTVisitor {
 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:
@@ -623,7 +509,7 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,
 #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"
   }
 
@@ -650,48 +536,6 @@ bool RecursiveASTVisitor<Derived>::PostVisitStmt(Stmt *S) {
   // 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;
@@ -3711,10 +3555,6 @@ bool RecursiveASTVisitor<Derived>::VisitOMPAffinityClause(
 
 #undef TRY_TO
 
-#undef UNARYOP_LIST
-#undef BINOP_LIST
-#undef CAO_LIST
-
 } // end namespace clang
 
 #endif // LLVM_CLANG_AST_RECURSIVEASTVISITOR_H

diff  --git a/clang/lib/ARCMigrate/TransProperties.cpp b/clang/lib/ARCMigrate/TransProperties.cpp
index adfb4bd77ac6..cba2256ef97b 100644
--- a/clang/lib/ARCMigrate/TransProperties.cpp
+++ b/clang/lib/ARCMigrate/TransProperties.cpp
@@ -287,7 +287,10 @@ class PropertiesRewriter {
   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)

diff  --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
index 70c7eb37ad4c..71f2bf6be599 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
@@ -403,622 +403,40 @@ void test() {
 }
 )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)
-WalkUpFromStmt UnaryOperator(-)
-WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(3)
-)txt"));
-
-  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> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseUnaryOperator(UnaryOperator *UO) {
-      recordCallback(__func__, UO, [&]() {
-        RecordingVisitorBase::TraverseUnaryOperator(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 WalkUpFromUnaryOperator(UnaryOperator *UO) {
-      recordCallback(__func__, UO, [&]() {
-        RecordingVisitorBase::WalkUpFromUnaryOperator(UO);
-      });
-      return true;
-    }
-  };
-
-  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)
-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)
-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> {
-  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 WalkUpFromUnaryOperator(UnaryOperator *UO) {
-      recordCallback(__func__, UO, [&]() {
-        RecordingVisitorBase::WalkUpFromUnaryOperator(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)
-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)
-WalkUpFromExpr IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(2)
-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, [&]() {
-        RecordingVisitorBase::TraverseBinaryOperator(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";
-
-  // 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)
-WalkUpFromStmt BinaryOperator(+)
-WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(4)
-)txt"));
-
-  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> {
-  public:
-    RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
-        : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
-
-    bool TraverseBinaryOperator(BinaryOperator *BO) {
-      recordCallback(__func__, BO, [&]() {
-        RecordingVisitorBase::TraverseBinaryOperator(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 WalkUpFromBinaryOperator(BinaryOperator *BO) {
-      recordCallback(__func__, BO, [&]() {
-        RecordingVisitorBase::WalkUpFromBinaryOperator(BO);
-      });
-      return true;
-    }
-  };
-
-  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)
-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)
-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> {
-  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 WalkUpFromBinaryOperator(BinaryOperator *BO) {
-      recordCallback(__func__, BO, [&]() {
-        RecordingVisitorBase::WalkUpFromBinaryOperator(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)
-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)
-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_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(+)
+TraverseUnaryOperator UnaryOperator(-)
+  WalkUpFromStmt UnaryOperator(-)
   WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromStmt IntegerLiteral(4)
+WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAdd BinaryOperator(+)
+TraverseUnaryOperator UnaryOperator(-)
   WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(3)
-  WalkUpFromStmt BinaryOperator(+)
-WalkUpFromStmt IntegerLiteral(4)
+  WalkUpFromStmt UnaryOperator(-)
+WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinAdd_WalkUpFromBinAdd) {
+TEST(RecursiveASTVisitor,
+     StmtCallbacks_TraverseUnaryOperator_WalkUpFromUnaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
-    bool TraverseBinAdd(BinaryOperator *BO) {
-      recordCallback(__func__, BO,
-                     [&]() { RecordingVisitorBase::TraverseBinAdd(BO); });
+    bool TraverseUnaryOperator(UnaryOperator *UO) {
+      recordCallback(__func__, UO, [&]() {
+        RecordingVisitorBase::TraverseUnaryOperator(UO);
+      });
       return true;
     }
 
@@ -1034,9 +452,10 @@ TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinAdd_WalkUpFromBinAdd) {
       return true;
     }
 
-    bool WalkUpFromBinAdd(BinaryOperator *BO) {
-      recordCallback(__func__, BO,
-                     [&]() { RecordingVisitorBase::WalkUpFromBinAdd(BO); });
+    bool WalkUpFromUnaryOperator(UnaryOperator *UO) {
+      recordCallback(__func__, UO, [&]() {
+        RecordingVisitorBase::WalkUpFromUnaryOperator(UO);
+      });
       return true;
     }
   };
@@ -1044,8 +463,8 @@ TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinAdd_WalkUpFromBinAdd) {
   StringRef Code = R"cpp(
 void test() {
   1;
-  2 + 3;
-  4;
+  -2;
+  3;
 }
 )cpp";
 
@@ -1055,16 +474,14 @@ void test() {
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAdd BinaryOperator(+)
-  WalkUpFromBinAdd BinaryOperator(+)
-    WalkUpFromExpr BinaryOperator(+)
-      WalkUpFromStmt BinaryOperator(+)
+TraverseUnaryOperator UnaryOperator(-)
+  WalkUpFromUnaryOperator UnaryOperator(-)
+    WalkUpFromExpr UnaryOperator(-)
+      WalkUpFromStmt UnaryOperator(-)
   WalkUpFromExpr IntegerLiteral(2)
     WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(3)
-    WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromExpr IntegerLiteral(4)
-  WalkUpFromStmt IntegerLiteral(4)
+WalkUpFromExpr IntegerLiteral(3)
+  WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
@@ -1072,21 +489,19 @@ WalkUpFromExpr IntegerLiteral(4)
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAdd BinaryOperator(+)
+TraverseUnaryOperator UnaryOperator(-)
   WalkUpFromExpr IntegerLiteral(2)
     WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromExpr IntegerLiteral(3)
-    WalkUpFromStmt IntegerLiteral(3)
-  WalkUpFromBinAdd BinaryOperator(+)
-    WalkUpFromExpr BinaryOperator(+)
-      WalkUpFromStmt BinaryOperator(+)
-WalkUpFromExpr IntegerLiteral(4)
-  WalkUpFromStmt IntegerLiteral(4)
+  WalkUpFromUnaryOperator UnaryOperator(-)
+    WalkUpFromExpr UnaryOperator(-)
+      WalkUpFromStmt UnaryOperator(-)
+WalkUpFromExpr IntegerLiteral(3)
+  WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinAdd) {
+TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromUnaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
@@ -1104,9 +519,10 @@ TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinAdd) {
       return true;
     }
 
-    bool WalkUpFromBinAdd(BinaryOperator *BO) {
-      recordCallback(__func__, BO,
-                     [&]() { RecordingVisitorBase::WalkUpFromBinAdd(BO); });
+    bool WalkUpFromUnaryOperator(UnaryOperator *UO) {
+      recordCallback(__func__, UO, [&]() {
+        RecordingVisitorBase::WalkUpFromUnaryOperator(UO);
+      });
       return true;
     }
   };
@@ -1114,8 +530,8 @@ TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinAdd) {
   StringRef Code = R"cpp(
 void test() {
   1;
-  2 + 3;
-  4;
+  -2;
+  3;
 }
 )cpp";
 
@@ -1125,15 +541,13 @@ void test() {
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromBinAdd BinaryOperator(+)
-  WalkUpFromExpr BinaryOperator(+)
-    WalkUpFromStmt BinaryOperator(+)
+WalkUpFromUnaryOperator UnaryOperator(-)
+  WalkUpFromExpr UnaryOperator(-)
+    WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
-WalkUpFromExpr IntegerLiteral(4)
-  WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
   EXPECT_TRUE(visitorCallbackLogEqual(
@@ -1143,26 +557,24 @@ WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
+WalkUpFromUnaryOperator UnaryOperator(-)
+  WalkUpFromExpr UnaryOperator(-)
+    WalkUpFromStmt UnaryOperator(-)
 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) {
+TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
-    bool TraverseCompoundAssignOperator(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::TraverseCompoundAssignOperator(CAO);
+    bool TraverseBinaryOperator(BinaryOperator *BO) {
+      recordCallback(__func__, BO, [&]() {
+        RecordingVisitorBase::TraverseBinaryOperator(BO);
       });
       return true;
     }
@@ -1175,50 +587,48 @@ TEST(RecursiveASTVisitor, StmtCallbacks_TraverseCompoundAssignOperator) {
   };
 
   StringRef Code = R"cpp(
-void test(int a) {
+void test() {
   1;
-  a += 2;
-  3;
+  2 + 3;
+  4;
 }
 )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)
-WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromStmt DeclRefExpr(a)
-WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt IntegerLiteral(3)
+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)
-WalkUpFromStmt DeclRefExpr(a)
-WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromStmt IntegerLiteral(3)
+TraverseBinaryOperator BinaryOperator(+)
+  WalkUpFromStmt IntegerLiteral(2)
+  WalkUpFromStmt IntegerLiteral(3)
+  WalkUpFromStmt BinaryOperator(+)
+WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(
-    RecursiveASTVisitor,
-    StmtCallbacks_TraverseCompoundAssignOperator_WalkUpFromCompoundAssignOperator) {
+TEST(RecursiveASTVisitor,
+     StmtCallbacks_TraverseBinaryOperator_WalkUpFromBinaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
-    bool TraverseCompoundAssignOperator(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::TraverseCompoundAssignOperator(CAO);
+    bool TraverseBinaryOperator(BinaryOperator *BO) {
+      recordCallback(__func__, BO, [&]() {
+        RecordingVisitorBase::TraverseBinaryOperator(BO);
       });
       return true;
     }
@@ -1235,40 +645,38 @@ TEST(
       return true;
     }
 
-    bool WalkUpFromCompoundAssignOperator(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::WalkUpFromCompoundAssignOperator(CAO);
+    bool WalkUpFromBinaryOperator(BinaryOperator *BO) {
+      recordCallback(__func__, BO, [&]() {
+        RecordingVisitorBase::WalkUpFromBinaryOperator(BO);
       });
       return true;
     }
   };
 
   StringRef Code = R"cpp(
-void test(int a) {
+void test() {
   1;
-  a += 2;
-  3;
+  2 + 3;
+  4;
 }
 )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)
-WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
-  WalkUpFromExpr CompoundAssignOperator(+=)
-    WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromExpr DeclRefExpr(a)
-  WalkUpFromStmt DeclRefExpr(a)
-WalkUpFromExpr IntegerLiteral(2)
-  WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromExpr IntegerLiteral(3)
-  WalkUpFromStmt IntegerLiteral(3)
+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(
@@ -1276,20 +684,21 @@ WalkUpFromExpr IntegerLiteral(3)
       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)
+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_WalkUpFromCompoundAssignOperator) {
+TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinaryOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
@@ -1307,19 +716,19 @@ TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromCompoundAssignOperator) {
       return true;
     }
 
-    bool WalkUpFromCompoundAssignOperator(CompoundAssignOperator *CAO) {
-      recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::WalkUpFromCompoundAssignOperator(CAO);
+    bool WalkUpFromBinaryOperator(BinaryOperator *BO) {
+      recordCallback(__func__, BO, [&]() {
+        RecordingVisitorBase::WalkUpFromBinaryOperator(BO);
       });
       return true;
     }
   };
 
   StringRef Code = R"cpp(
-void test(int a) {
+void test() {
   1;
-  a += 2;
-  3;
+  2 + 3;
+  4;
 }
 )cpp";
 
@@ -1329,15 +738,15 @@ void test(int a) {
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
-  WalkUpFromExpr CompoundAssignOperator(+=)
-    WalkUpFromStmt CompoundAssignOperator(+=)
-WalkUpFromExpr DeclRefExpr(a)
-  WalkUpFromStmt DeclRefExpr(a)
+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(
@@ -1345,28 +754,28 @@ WalkUpFromExpr IntegerLiteral(3)
       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)
+WalkUpFromBinaryOperator BinaryOperator(+)
+  WalkUpFromExpr BinaryOperator(+)
+    WalkUpFromStmt BinaryOperator(+)
+WalkUpFromExpr IntegerLiteral(4)
+  WalkUpFromStmt IntegerLiteral(4)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor, StmtCallbacks_TraverseBinAddAssign) {
+TEST(RecursiveASTVisitor, StmtCallbacks_TraverseCompoundAssignOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
-    bool TraverseBinAddAssign(CompoundAssignOperator *CAO) {
+    bool TraverseCompoundAssignOperator(CompoundAssignOperator *CAO) {
       recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::TraverseBinAddAssign(CAO);
+        RecordingVisitorBase::TraverseCompoundAssignOperator(CAO);
       });
       return true;
     }
@@ -1391,7 +800,7 @@ void test(int a) {
       R"txt(
 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromStmt CompoundAssignOperator(+=)
   WalkUpFromStmt DeclRefExpr(a)
   WalkUpFromStmt IntegerLiteral(2)
@@ -1402,7 +811,7 @@ WalkUpFromStmt IntegerLiteral(3)
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromStmt DeclRefExpr(a)
   WalkUpFromStmt IntegerLiteral(2)
   WalkUpFromStmt CompoundAssignOperator(+=)
@@ -1411,16 +820,17 @@ WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor,
-     StmtCallbacks_TraverseBinAddAssign_WalkUpFromBinAddAssign) {
+TEST(
+    RecursiveASTVisitor,
+    StmtCallbacks_TraverseCompoundAssignOperator_WalkUpFromCompoundAssignOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
         : RecordingVisitorBase(ShouldTraversePostOrderValue) {}
 
-    bool TraverseBinAddAssign(CompoundAssignOperator *CAO) {
+    bool TraverseCompoundAssignOperator(CompoundAssignOperator *CAO) {
       recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::TraverseBinAddAssign(CAO);
+        RecordingVisitorBase::TraverseCompoundAssignOperator(CAO);
       });
       return true;
     }
@@ -1437,9 +847,9 @@ TEST(RecursiveASTVisitor,
       return true;
     }
 
-    bool WalkUpFromBinAddAssign(CompoundAssignOperator *CAO) {
+    bool WalkUpFromCompoundAssignOperator(CompoundAssignOperator *CAO) {
       recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::WalkUpFromBinAddAssign(CAO);
+        RecordingVisitorBase::WalkUpFromCompoundAssignOperator(CAO);
       });
       return true;
     }
@@ -1459,8 +869,8 @@ void test(int a) {
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
-  WalkUpFromBinAddAssign CompoundAssignOperator(+=)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
+  WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
     WalkUpFromExpr CompoundAssignOperator(+=)
       WalkUpFromStmt CompoundAssignOperator(+=)
   WalkUpFromExpr DeclRefExpr(a)
@@ -1476,12 +886,12 @@ WalkUpFromExpr IntegerLiteral(3)
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-TraverseBinAddAssign CompoundAssignOperator(+=)
+TraverseCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromExpr DeclRefExpr(a)
     WalkUpFromStmt DeclRefExpr(a)
   WalkUpFromExpr IntegerLiteral(2)
     WalkUpFromStmt IntegerLiteral(2)
-  WalkUpFromBinAddAssign CompoundAssignOperator(+=)
+  WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
     WalkUpFromExpr CompoundAssignOperator(+=)
       WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromExpr IntegerLiteral(3)
@@ -1490,7 +900,7 @@ WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
-TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinAddAssign) {
+TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromCompoundAssignOperator) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
   public:
     RecordingVisitor(ShouldTraversePostOrder ShouldTraversePostOrderValue)
@@ -1508,9 +918,9 @@ TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromBinAddAssign) {
       return true;
     }
 
-    bool WalkUpFromBinAddAssign(CompoundAssignOperator *CAO) {
+    bool WalkUpFromCompoundAssignOperator(CompoundAssignOperator *CAO) {
       recordCallback(__func__, CAO, [&]() {
-        RecordingVisitorBase::WalkUpFromBinAddAssign(CAO);
+        RecordingVisitorBase::WalkUpFromCompoundAssignOperator(CAO);
       });
       return true;
     }
@@ -1530,7 +940,7 @@ void test(int a) {
 WalkUpFromStmt CompoundStmt
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
-WalkUpFromBinAddAssign CompoundAssignOperator(+=)
+WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromExpr CompoundAssignOperator(+=)
     WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromExpr DeclRefExpr(a)
@@ -1550,7 +960,7 @@ WalkUpFromExpr DeclRefExpr(a)
   WalkUpFromStmt DeclRefExpr(a)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromBinAddAssign CompoundAssignOperator(+=)
+WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=)
   WalkUpFromExpr CompoundAssignOperator(+=)
     WalkUpFromStmt CompoundAssignOperator(+=)
 WalkUpFromExpr IntegerLiteral(3)


        


More information about the cfe-commits mailing list