diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -589,7 +589,6 @@ BINOP_LIST() #undef OPERATOR -#undef BINOP_LIST #define OPERATOR(NAME) \ case BO_##NAME##Assign: \ @@ -597,7 +596,6 @@ CAO_LIST() #undef OPERATOR -#undef CAO_LIST } } else if (UnaryOperator *UnOp = dyn_cast(S)) { switch (UnOp->getOpcode()) { @@ -607,7 +605,6 @@ UNARYOP_LIST() #undef OPERATOR -#undef UNARYOP_LIST } } @@ -629,27 +626,68 @@ template bool RecursiveASTVisitor::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(S)) { + switch (BinOp->getOpcode()) { +#define OPERATOR(NAME) \ + case BO_##NAME: \ + if (isSameMethod(&RecursiveASTVisitor::TraverseBin##NAME, \ + &Derived::TraverseBin##NAME)) { \ + TRY_TO(WalkUpFromBin##NAME(static_cast(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(S))); \ + } \ + return true; + + CAO_LIST() +#undef OPERATOR + } + } else if (UnaryOperator *UnOp = dyn_cast(S)) { + switch (UnOp->getOpcode()) { +#define OPERATOR(NAME) \ + case UO_##NAME: \ + if (isSameMethod(&RecursiveASTVisitor::TraverseUnary##NAME, \ + &Derived::TraverseUnary##NAME)) { \ + TRY_TO(WalkUpFromUnary##NAME(static_cast(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(S))); \ @@ -657,7 +695,6 @@ 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(S); \ @@ -3662,6 +3699,10 @@ #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/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp --- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp +++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp @@ -416,13 +416,12 @@ 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")); @@ -488,8 +487,6 @@ 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( @@ -497,6 +494,9 @@ WalkUpFromStmt IntegerLiteral(1) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) +WalkUpFromUnaryOperator UnaryOperator(-) + WalkUpFromExpr UnaryOperator(-) + WalkUpFromStmt UnaryOperator(-) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -606,13 +606,14 @@ 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")); @@ -683,8 +684,6 @@ TraverseUnaryMinus UnaryOperator(-) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromExpr UnaryOperator(-) - WalkUpFromStmt UnaryOperator(-) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -747,8 +746,9 @@ WalkUpFromStmt IntegerLiteral(1) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromExpr UnaryOperator(-) - WalkUpFromStmt UnaryOperator(-) +WalkUpFromUnaryMinus UnaryOperator(-) + WalkUpFromExpr UnaryOperator(-) + WalkUpFromStmt UnaryOperator(-) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -805,6 +805,7 @@ WalkUpFromStmt IntegerLiteral(1) WalkUpFromStmt IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(3) +WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt )txt")); @@ -882,6 +883,9 @@ WalkUpFromStmt IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) +WalkUpFromBinaryOperator BinaryOperator(+) + WalkUpFromExpr BinaryOperator(+) + WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt @@ -1003,7 +1007,6 @@ TraverseBinAdd BinaryOperator(+) WalkUpFromStmt IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt )txt")); @@ -1077,8 +1080,6 @@ WalkUpFromStmt IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromExpr BinaryOperator(+) - WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt @@ -1145,8 +1146,9 @@ WalkUpFromStmt IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromExpr BinaryOperator(+) - WalkUpFromStmt BinaryOperator(+) +WalkUpFromBinAdd BinaryOperator(+) + WalkUpFromExpr BinaryOperator(+) + WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt @@ -1203,6 +1205,7 @@ WalkUpFromStmt IntegerLiteral(1) WalkUpFromStmt DeclRefExpr(a) WalkUpFromStmt IntegerLiteral(2) +WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt )txt")); @@ -1271,8 +1274,6 @@ WalkUpFromStmt IntegerLiteral(3) )txt")); - // FIXME: The following log should include a call to - // WalkUpFromCompoundAssignOperator. EXPECT_TRUE(visitorCallbackLogEqual( RecordingVisitor(ShouldTraversePostOrder::Yes), Code, R"txt( @@ -1282,6 +1283,9 @@ WalkUpFromStmt DeclRefExpr(a) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) +WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=) + WalkUpFromExpr CompoundAssignOperator(+=) + WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -1404,7 +1408,6 @@ TraverseBinAddAssign CompoundAssignOperator(+=) WalkUpFromStmt DeclRefExpr(a) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt )txt")); @@ -1481,8 +1484,6 @@ WalkUpFromStmt DeclRefExpr(a) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromExpr CompoundAssignOperator(+=) - WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -1550,8 +1551,9 @@ WalkUpFromStmt DeclRefExpr(a) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromExpr CompoundAssignOperator(+=) - WalkUpFromStmt CompoundAssignOperator(+=) +WalkUpFromBinAddAssign CompoundAssignOperator(+=) + WalkUpFromExpr CompoundAssignOperator(+=) + WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt