gribozavr updated this revision to Diff 274497. gribozavr added a comment. Rebased the patch.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82787/new/ https://reviews.llvm.org/D82787 Files: clang/include/clang/AST/RecursiveASTVisitor.h clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp =================================================================== --- clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp +++ 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 @@ -739,7 +738,6 @@ WalkUpFromStmt IntegerLiteral(3) )txt")); - // FIXME: The following log should include a call to WalkUpFromUnaryMinus. EXPECT_TRUE(visitorCallbackLogEqual( RecordingVisitor(ShouldTraversePostOrder::Yes), Code, R"txt( @@ -747,8 +745,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 @@ -797,14 +796,13 @@ 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")); @@ -872,7 +870,6 @@ WalkUpFromStmt IntegerLiteral(4) )txt")); - // FIXME: The following log should include a call to WalkUpFromBinaryOperator. EXPECT_TRUE(visitorCallbackLogEqual( RecordingVisitor(ShouldTraversePostOrder::Yes), Code, R"txt( @@ -882,6 +879,9 @@ WalkUpFromStmt IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) +WalkUpFromBinaryOperator BinaryOperator(+) + WalkUpFromExpr BinaryOperator(+) + WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt @@ -996,6 +996,8 @@ 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( @@ -1003,7 +1005,6 @@ TraverseBinAdd BinaryOperator(+) WalkUpFromStmt IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt )txt")); @@ -1077,8 +1078,6 @@ WalkUpFromStmt IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromExpr BinaryOperator(+) - WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt @@ -1135,7 +1134,6 @@ WalkUpFromStmt IntegerLiteral(4) )txt")); - // FIXME: The following log should include a call to WalkUpFromBinAdd. EXPECT_TRUE(visitorCallbackLogEqual( RecordingVisitor(ShouldTraversePostOrder::Yes), Code, R"txt( @@ -1145,8 +1143,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 @@ -1195,14 +1194,13 @@ 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")); @@ -1271,8 +1269,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 +1278,9 @@ WalkUpFromStmt DeclRefExpr(a) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) +WalkUpFromCompoundAssignOperator CompoundAssignOperator(+=) + WalkUpFromExpr CompoundAssignOperator(+=) + WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -1397,6 +1396,8 @@ 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( @@ -1404,7 +1405,6 @@ TraverseBinAddAssign CompoundAssignOperator(+=) WalkUpFromStmt DeclRefExpr(a) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt )txt")); @@ -1481,8 +1481,6 @@ WalkUpFromStmt DeclRefExpr(a) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromExpr CompoundAssignOperator(+=) - WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -1540,7 +1538,6 @@ WalkUpFromStmt IntegerLiteral(3) )txt")); - // FIXME: The following log should include a call to WalkUpFromBinAddAssign. EXPECT_TRUE(visitorCallbackLogEqual( RecordingVisitor(ShouldTraversePostOrder::Yes), Code, R"txt( @@ -1550,8 +1547,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 Index: clang/include/clang/AST/RecursiveASTVisitor.h =================================================================== --- clang/include/clang/AST/RecursiveASTVisitor.h +++ 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<UnaryOperator>(S)) { switch (UnOp->getOpcode()) { @@ -607,7 +605,6 @@ UNARYOP_LIST() #undef OPERATOR -#undef UNARYOP_LIST } } @@ -629,27 +626,68 @@ 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))); \ @@ -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<CLASS *>(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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits