Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

[gribozavr]

Rebased the patch.

[gribozavr]

Rebased the patch.

[eduucaldas]

clang/include/clang/AST/RecursiveASTVisitor.h

649

There's only one place that call PostVisitStmt. Could we pull this conditional there? It is repeated in every macro. It would make this function much simpler, and make clear at call site what is being done

[gribozavr2]

clang/include/clang/AST/RecursiveASTVisitor.h

649

Unfortunately, we can't, because every call to isSameMethod has different arguments.

clang/include/clang/AST/RecursiveASTVisitor.h

#define DISPATCH_STMT(NAME, CLASS, VAR)                                        \
   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
-#undef BINOP_LIST
 
 #define OPERATOR(NAME)                                                         \
   case BO_##NAME##Assign:                                                      \
     DISPATCH_STMT(Bin##NAME##Assign, CompoundAssignOperator, S);
 
       CAO_LIST()
 #undef OPERATOR
-#undef CAO_LIST
     }
   } 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
-#undef UNARYOP_LIST
     }
   }
 
   // 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);
 #include "clang/AST/StmtNodes.inc"
   }
 
   return true;
 }
 
 #undef DISPATCH_STMT
 
 template <typename Derived>
 bool RecursiveASTVisitor<Derived>::PostVisitStmt(Stmt *S) {
+  // In pre-order traversal mode, each Traverse##STMT method is responsible for
+  // calling WalkUpFrom. Therefore, if the user overrides Traverse##STMT and
+  // does not call the default implementation, the WalkUpFrom callback is not
+  // called. Post-order traversal mode should provide the same behavior
+  // regarding method overrides.
+  //
+  // In post-order traversal mode the Traverse##STMT method, when it receives a
+  // DataRecursionQueue, can't call WalkUpFrom after traversing children because
+  // it only enqueues the children and does not traverse them. TraverseStmt
+  // traverses the enqueued children, and we call WalkUpFrom here.
+  //
+  // However, to make pre-order and post-order modes identical with regards to
+  // whether they call WalkUpFrom at all, we call WalkUpFrom if and only if the
+  // user did not override the Traverse##STMT method. 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 (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 (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 (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:                                                     \
-    /* In pre-order traversal mode, each Traverse##STMT method is responsible  \
-     * for calling WalkUpFrom. Therefore, if the user overrides Traverse##STMT \
-     * and does not call the default implementation, the WalkUpFrom callback   \
-     * is not called. Post-order traversal mode should provide the same        \
-     * behavior regarding method overrides.                                    \
-     *                                                                         \
-     * In post-order traversal mode the Traverse##STMT method, when it         \
-     * receives a DataRecursionQueue, can't call WalkUpFrom after traversing   \
-     * children because it only enqueues the children and does not traverse    \
-     * them. TraverseStmt traverses the enqueued children, and we call         \
-     * WalkUpFrom here.                                                        \
-     *                                                                         \
-     * However, to make pre-order and post-order modes identical with regards  \
-     * to whether they call WalkUpFrom at all, we call WalkUpFrom if and only  \
-     * if the user did not override the Traverse##STMT method. */              \
     if (isSameMethod(&RecursiveASTVisitor::Traverse##CLASS,                    \
                      &Derived::Traverse##CLASS)) {                             \
       TRY_TO(WalkUpFrom##CLASS(static_cast<CLASS *>(S)));                      \
     }                                                                          \
     break;
 #define INITLISTEXPR(CLASS, PARENT)                                            \
   case Stmt::CLASS##Class:                                                     \
-    /* See the comment above for the explanation of the isSameMethod check. */ \
     if (isSameMethod(&RecursiveASTVisitor::Traverse##CLASS,                    \
                      &Derived::Traverse##CLASS)) {                             \
       auto ILE = static_cast<CLASS *>(S);                                      \
       if (auto Syn = ILE->isSemanticForm() ? ILE->getSyntacticForm() : ILE)    \
         TRY_TO(WalkUpFrom##CLASS(Syn));                                        \
       if (auto Sem = ILE->isSemanticForm() ? ILE : ILE->getSemanticForm())     \
         TRY_TO(WalkUpFrom##CLASS(Sem));                                        \
     }                                                                          \

 //    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/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp

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

 WalkUpFromStmt CompoundStmt
 WalkUpFromStmt IntegerLiteral(1)
 TraverseUnaryMinus UnaryOperator(-)
   WalkUpFromStmt UnaryOperator(-)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
+  // FIXME: The following log should include a call to WalkUpFromStmt for
+  // UnaryOperator(-).
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromStmt IntegerLiteral(1)
 TraverseUnaryMinus UnaryOperator(-)
   WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromStmt UnaryOperator(-)
 WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor,
      StmtCallbacks_TraverseUnaryMinus_WalkUpFromUnaryMinus) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
)txt"));
   EXPECT_TRUE(visitorCallbackLogEqual(
       RecordingVisitor(ShouldTraversePostOrder::Yes), Code,
       R"txt(
 WalkUpFromExpr IntegerLiteral(1)
   WalkUpFromStmt IntegerLiteral(1)
 TraverseUnaryMinus UnaryOperator(-)
   WalkUpFromExpr IntegerLiteral(2)
     WalkUpFromStmt IntegerLiteral(2)
-WalkUpFromExpr UnaryOperator(-)
-  WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromStmt CompoundStmt
 )txt"));
 }
 
 TEST(RecursiveASTVisitor, StmtCallbacks_WalkUpFromUnaryMinus) {
   class RecordingVisitor : public RecordingVisitorBase<RecordingVisitor> {
WalkUpFromUnaryMinus UnaryOperator(-)
   WalkUpFromExpr UnaryOperator(-)
     WalkUpFromStmt UnaryOperator(-)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
-  // FIXME: The following log should include a call to WalkUpFromUnaryMinus.
   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> {

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

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

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

 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 WalkUpFromExpr IntegerLiteral(4)
   WalkUpFromStmt IntegerLiteral(4)
 )txt"));
 
-  // FIXME: The following log should include a call to WalkUpFromBinAdd.
   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> {

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

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

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

 WalkUpFromExpr DeclRefExpr(a)
   WalkUpFromStmt DeclRefExpr(a)
 WalkUpFromExpr IntegerLiteral(2)
   WalkUpFromStmt IntegerLiteral(2)
 WalkUpFromExpr IntegerLiteral(3)
   WalkUpFromStmt IntegerLiteral(3)
 )txt"));
 
-  // FIXME: The following log should include a call to WalkUpFromBinAddAssign.
   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> {