Author: Dmitri Gribenko Date: 2020-07-06T13:38:01+02:00 New Revision: c19c6b1722e5f71200c09cdb096245b98f03dce0
URL: https://github.com/llvm/llvm-project/commit/c19c6b1722e5f71200c09cdb096245b98f03dce0 DIFF: https://github.com/llvm/llvm-project/commit/c19c6b1722e5f71200c09cdb096245b98f03dce0.diff LOG: Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode Reviewers: ymandel, eduucaldas, rsmith Reviewed By: eduucaldas, rsmith Subscribers: gribozavr2, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D82787 Added: Modified: clang/include/clang/AST/RecursiveASTVisitor.h clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h index 464a80d15cef..9d93f1e99666 100644 --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -593,7 +593,6 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S, BINOP_LIST() #undef OPERATOR -#undef BINOP_LIST #define OPERATOR(NAME) \ case BO_##NAME##Assign: \ @@ -601,7 +600,6 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S, CAO_LIST() #undef OPERATOR -#undef CAO_LIST } } else if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(S)) { switch (UnOp->getOpcode()) { @@ -611,7 +609,6 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S, UNARYOP_LIST() #undef OPERATOR -#undef UNARYOP_LIST } } @@ -633,27 +630,70 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S, 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 (::clang::detail::isSameMethod(&RecursiveASTVisitor::TraverseBin##NAME, \ + &Derived::TraverseBin##NAME)) { \ + TRY_TO(WalkUpFromBin##NAME(static_cast<BinaryOperator *>(S))); \ + } \ + return true; + + BINOP_LIST() +#undef OPERATOR + +#define OPERATOR(NAME) \ + case BO_##NAME##Assign: \ + if (::clang::detail::isSameMethod( \ + &RecursiveASTVisitor::TraverseBin##NAME##Assign, \ + &Derived::TraverseBin##NAME##Assign)) { \ + TRY_TO(WalkUpFromBin##NAME##Assign( \ + static_cast<CompoundAssignOperator *>(S))); \ + } \ + return true; + + CAO_LIST() +#undef OPERATOR + } + } else if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(S)) { + switch (UnOp->getOpcode()) { +#define OPERATOR(NAME) \ + case UO_##NAME: \ + if (::clang::detail::isSameMethod( \ + &RecursiveASTVisitor::TraverseUnary##NAME, \ + &Derived::TraverseUnary##NAME)) { \ + TRY_TO(WalkUpFromUnary##NAME(static_cast<UnaryOperator *>(S))); \ + } \ + return true; + + UNARYOP_LIST() +#undef OPERATOR + } + } + switch (S->getStmtClass()) { case Stmt::NoStmtClass: break; #define ABSTRACT_STMT(STMT) #define STMT(CLASS, PARENT) \ case Stmt::CLASS##Class: \ - /* 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 (::clang::detail::isSameMethod(&RecursiveASTVisitor::Traverse##CLASS, \ &Derived::Traverse##CLASS)) { \ TRY_TO(WalkUpFrom##CLASS(static_cast<CLASS *>(S))); \ @@ -661,7 +701,6 @@ bool RecursiveASTVisitor<Derived>::PostVisitStmt(Stmt *S) { break; #define INITLISTEXPR(CLASS, PARENT) \ case Stmt::CLASS##Class: \ - /* See the comment above for the explanation of the isSameMethod check. */ \ if (::clang::detail::isSameMethod(&RecursiveASTVisitor::Traverse##CLASS, \ &Derived::Traverse##CLASS)) { \ auto ILE = static_cast<CLASS *>(S); \ @@ -3668,6 +3707,10 @@ 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/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp index 1ab843d437a8..0a4e9aa1e9e8 100644 --- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp +++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp @@ -416,13 +416,12 @@ 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")); @@ -488,8 +487,6 @@ 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( @@ -497,6 +494,9 @@ 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 @@ -606,13 +606,14 @@ TraverseUnaryMinus UnaryOperator(-) 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 @@ WalkUpFromExpr IntegerLiteral(1) TraverseUnaryMinus UnaryOperator(-) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromExpr UnaryOperator(-) - WalkUpFromStmt UnaryOperator(-) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -739,7 +738,6 @@ 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( @@ -747,8 +745,9 @@ WalkUpFromExpr IntegerLiteral(1) 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(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")); @@ -872,7 +870,6 @@ 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( @@ -882,6 +879,9 @@ 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 @@ -996,6 +996,8 @@ TraverseBinAdd BinaryOperator(+) 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 @@ WalkUpFromStmt IntegerLiteral(1) TraverseBinAdd BinaryOperator(+) WalkUpFromStmt IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt )txt")); @@ -1077,8 +1078,6 @@ TraverseBinAdd BinaryOperator(+) 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 @@ 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( @@ -1145,8 +1143,9 @@ WalkUpFromExpr IntegerLiteral(2) 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(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")); @@ -1271,8 +1269,6 @@ 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( @@ -1282,6 +1278,9 @@ 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 @@ -1397,6 +1396,8 @@ TraverseBinAddAssign CompoundAssignOperator(+=) 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 @@ WalkUpFromStmt IntegerLiteral(1) TraverseBinAddAssign CompoundAssignOperator(+=) WalkUpFromStmt DeclRefExpr(a) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt )txt")); @@ -1481,8 +1481,6 @@ TraverseBinAddAssign CompoundAssignOperator(+=) 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 @@ 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( @@ -1550,8 +1547,9 @@ WalkUpFromExpr DeclRefExpr(a) 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
