Author: Dmitri Gribenko Date: 2020-07-03T13:48:24+02:00 New Revision: 19eaff650c9c091e844f0a342540f1d10573772c
URL: https://github.com/llvm/llvm-project/commit/19eaff650c9c091e844f0a342540f1d10573772c DIFF: https://github.com/llvm/llvm-project/commit/19eaff650c9c091e844f0a342540f1d10573772c.diff LOG: Revert RecursiveASTVisitor fixes. This reverts commit 8bf4c40af813e73de77739b33b8808f6bd13497b. This reverts commit 7b0be962d681c408c8ecf7180c6ad8f9fbcdaf2d. This reverts commit 94454442c3c15a67ae70ef3a73616632968973fc. Some compilers on some buildbots didn't accept the specialization of is_same_method_impl in a non-namespace scope. Added: Modified: clang/include/clang/AST/RecursiveASTVisitor.h clang/lib/Tooling/Syntax/BuildTree.cpp clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h index ba36d5b23fc6..b16c1ae1e483 100644 --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -331,32 +331,6 @@ template <typename Derived> class RecursiveASTVisitor { struct has_same_member_pointer_type<R (T::*)(P...), R (U::*)(P...)> : std::true_type {}; - template <bool has_same_type> struct is_same_method_impl { - template <typename FirstMethodPtrTy, typename SecondMethodPtrTy> - static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr, - SecondMethodPtrTy SecondMethodPtr) { - return false; - } - }; - - template <> struct is_same_method_impl<true> { - template <typename FirstMethodPtrTy, typename SecondMethodPtrTy> - static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr, - SecondMethodPtrTy SecondMethodPtr) { - return FirstMethodPtr == SecondMethodPtr; - } - }; - - /// Returns true if and only if \p FirstMethodPtr and \p SecondMethodPtr - /// are pointers to the same non-static member function. - template <typename FirstMethodPtrTy, typename SecondMethodPtrTy> - bool isSameMethod(FirstMethodPtrTy FirstMethodPtr, - SecondMethodPtrTy SecondMethodPtr) { - return is_same_method_impl< - has_same_member_pointer_type<FirstMethodPtrTy, SecondMethodPtrTy>:: - value>::isSameMethod(FirstMethodPtr, SecondMethodPtr); - } - // Traverse the given statement. If the most-derived traverse function takes a // data recursion queue, pass it on; otherwise, discard it. Note that the // first branch of this conditional must compile whether or not the derived @@ -412,8 +386,6 @@ template <typename Derived> class RecursiveASTVisitor { 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) { \ @@ -435,8 +407,6 @@ template <typename Derived> class RecursiveASTVisitor { 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) { \ @@ -593,6 +563,7 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S, BINOP_LIST() #undef OPERATOR +#undef BINOP_LIST #define OPERATOR(NAME) \ case BO_##NAME##Assign: \ @@ -600,6 +571,7 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S, CAO_LIST() #undef OPERATOR +#undef CAO_LIST } } else if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(S)) { switch (UnOp->getOpcode()) { @@ -609,6 +581,7 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S, UNARYOP_LIST() #undef OPERATOR +#undef UNARYOP_LIST } } @@ -630,84 +603,23 @@ 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 (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: \ - if (isSameMethod(&RecursiveASTVisitor::Traverse##CLASS, \ - &Derived::Traverse##CLASS)) { \ - TRY_TO(WalkUpFrom##CLASS(static_cast<CLASS *>(S))); \ - } \ - break; + TRY_TO(WalkUpFrom##CLASS(static_cast<CLASS *>(S))); break; #define INITLISTEXPR(CLASS, PARENT) \ case Stmt::CLASS##Class: \ - 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)); \ - } \ - break; + break; \ + } #include "clang/AST/StmtNodes.inc" } @@ -2304,13 +2216,8 @@ DEF_TRAVERSE_DECL(RequiresExprBodyDecl, {}) TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt); \ } \ } \ - /* Call WalkUpFrom if TRY_TO_TRAVERSE_OR_ENQUEUE_STMT has traversed the \ - * children already. If TRY_TO_TRAVERSE_OR_ENQUEUE_STMT only enqueued the \ - * children, PostVisitStmt will call WalkUpFrom after we are done visiting \ - * children. */ \ - if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder()) { \ + if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder()) \ TRY_TO(WalkUpFrom##STMT(S)); \ - } \ return ReturnValue; \ } @@ -2481,9 +2388,6 @@ bool RecursiveASTVisitor<Derived>::TraverseSynOrSemInitListExpr( for (Stmt *SubStmt : S->children()) { TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt); } - - if (!Queue && getDerived().shouldTraversePostOrder()) - TRY_TO(WalkUpFromInitListExpr(S)); } return true; } @@ -3705,10 +3609,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/Tooling/Syntax/BuildTree.cpp b/clang/lib/Tooling/Syntax/BuildTree.cpp index 4371a303bb9d..8185af2a6cc7 100644 --- a/clang/lib/Tooling/Syntax/BuildTree.cpp +++ b/clang/lib/Tooling/Syntax/BuildTree.cpp @@ -578,19 +578,15 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> { // RAV traverses it as a statement, we produce invalid node kinds in that // case. // FIXME: should do this in RAV instead? - bool Result = [&, this]() { - if (S->getInit() && !TraverseStmt(S->getInit())) - return false; - if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable())) - return false; - if (S->getRangeInit() && !TraverseStmt(S->getRangeInit())) - return false; - if (S->getBody() && !TraverseStmt(S->getBody())) - return false; - return true; - }(); - WalkUpFromCXXForRangeStmt(S); - return Result; + if (S->getInit() && !TraverseStmt(S->getInit())) + return false; + if (S->getLoopVariable() && !TraverseDecl(S->getLoopVariable())) + return false; + if (S->getRangeInit() && !TraverseStmt(S->getRangeInit())) + return false; + if (S->getBody() && !TraverseStmt(S->getBody())) + return false; + return true; } bool TraverseStmt(Stmt *S) { diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp index 70c7eb37ad4c..66aa1d763833 100644 --- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp +++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp @@ -154,17 +154,22 @@ TraverseIntegerLiteral IntegerLiteral(5) R"txt( TraverseIntegerLiteral IntegerLiteral(1) WalkUpFromStmt IntegerLiteral(1) +WalkUpFromStmt IntegerLiteral(1) TraverseIntegerLiteral IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) +WalkUpFromStmt IntegerLiteral(2) TraverseIntegerLiteral IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) +WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt DeclRefExpr(add) WalkUpFromStmt ImplicitCastExpr TraverseIntegerLiteral IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) +WalkUpFromStmt IntegerLiteral(4) TraverseIntegerLiteral IntegerLiteral(5) WalkUpFromStmt IntegerLiteral(5) +WalkUpFromStmt IntegerLiteral(5) WalkUpFromStmt CallExpr(add) WalkUpFromStmt CompoundStmt )txt")); @@ -253,14 +258,23 @@ TraverseIntegerLiteral IntegerLiteral(1) WalkUpFromIntegerLiteral IntegerLiteral(1) WalkUpFromExpr IntegerLiteral(1) WalkUpFromStmt IntegerLiteral(1) +WalkUpFromIntegerLiteral IntegerLiteral(1) + WalkUpFromExpr IntegerLiteral(1) + WalkUpFromStmt IntegerLiteral(1) TraverseIntegerLiteral IntegerLiteral(2) WalkUpFromIntegerLiteral IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) +WalkUpFromIntegerLiteral IntegerLiteral(2) + WalkUpFromExpr IntegerLiteral(2) + WalkUpFromStmt IntegerLiteral(2) TraverseIntegerLiteral IntegerLiteral(3) WalkUpFromIntegerLiteral IntegerLiteral(3) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) +WalkUpFromIntegerLiteral IntegerLiteral(3) + WalkUpFromExpr IntegerLiteral(3) + WalkUpFromStmt IntegerLiteral(3) WalkUpFromExpr BinaryOperator(+) WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr DeclRefExpr(add) @@ -271,10 +285,16 @@ TraverseIntegerLiteral IntegerLiteral(4) WalkUpFromIntegerLiteral IntegerLiteral(4) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) +WalkUpFromIntegerLiteral IntegerLiteral(4) + WalkUpFromExpr IntegerLiteral(4) + WalkUpFromStmt IntegerLiteral(4) TraverseIntegerLiteral IntegerLiteral(5) WalkUpFromIntegerLiteral IntegerLiteral(5) WalkUpFromExpr IntegerLiteral(5) WalkUpFromStmt IntegerLiteral(5) +WalkUpFromIntegerLiteral IntegerLiteral(5) + WalkUpFromExpr IntegerLiteral(5) + WalkUpFromStmt IntegerLiteral(5) WalkUpFromExpr CallExpr(add) WalkUpFromStmt CallExpr(add) WalkUpFromStmt CompoundStmt @@ -612,7 +632,7 @@ WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(1) TraverseUnaryMinus UnaryOperator(-) WalkUpFromStmt IntegerLiteral(2) - WalkUpFromStmt UnaryOperator(-) +WalkUpFromStmt UnaryOperator(-) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt )txt")); @@ -674,6 +694,7 @@ 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( @@ -682,9 +703,8 @@ WalkUpFromExpr IntegerLiteral(1) TraverseUnaryMinus UnaryOperator(-) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) - WalkUpFromUnaryMinus UnaryOperator(-) - WalkUpFromExpr UnaryOperator(-) - WalkUpFromStmt UnaryOperator(-) +WalkUpFromExpr UnaryOperator(-) + WalkUpFromStmt UnaryOperator(-) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -739,6 +759,7 @@ 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( @@ -746,9 +767,8 @@ WalkUpFromExpr IntegerLiteral(1) WalkUpFromStmt IntegerLiteral(1) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromUnaryMinus UnaryOperator(-) - WalkUpFromExpr UnaryOperator(-) - WalkUpFromStmt UnaryOperator(-) +WalkUpFromExpr UnaryOperator(-) + WalkUpFromStmt UnaryOperator(-) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -1004,7 +1024,7 @@ WalkUpFromStmt IntegerLiteral(1) TraverseBinAdd BinaryOperator(+) WalkUpFromStmt IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(3) - WalkUpFromStmt BinaryOperator(+) +WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt )txt")); @@ -1067,6 +1087,7 @@ 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( @@ -1077,9 +1098,8 @@ TraverseBinAdd BinaryOperator(+) WalkUpFromStmt IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) - WalkUpFromBinAdd BinaryOperator(+) - WalkUpFromExpr BinaryOperator(+) - WalkUpFromStmt BinaryOperator(+) +WalkUpFromExpr BinaryOperator(+) + WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt @@ -1136,6 +1156,7 @@ 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,9 +1166,8 @@ WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromBinAdd BinaryOperator(+) - WalkUpFromExpr BinaryOperator(+) - WalkUpFromStmt BinaryOperator(+) +WalkUpFromExpr BinaryOperator(+) + WalkUpFromStmt BinaryOperator(+) WalkUpFromExpr IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt CompoundStmt @@ -1405,7 +1425,7 @@ WalkUpFromStmt IntegerLiteral(1) TraverseBinAddAssign CompoundAssignOperator(+=) WalkUpFromStmt DeclRefExpr(a) WalkUpFromStmt IntegerLiteral(2) - WalkUpFromStmt CompoundAssignOperator(+=) +WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt )txt")); @@ -1471,6 +1491,7 @@ 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( @@ -1481,9 +1502,8 @@ TraverseBinAddAssign CompoundAssignOperator(+=) WalkUpFromStmt DeclRefExpr(a) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) - WalkUpFromBinAddAssign CompoundAssignOperator(+=) - WalkUpFromExpr CompoundAssignOperator(+=) - WalkUpFromStmt CompoundAssignOperator(+=) +WalkUpFromExpr CompoundAssignOperator(+=) + WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -1541,6 +1561,7 @@ 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,9 +1571,8 @@ WalkUpFromExpr DeclRefExpr(a) WalkUpFromStmt DeclRefExpr(a) WalkUpFromExpr IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(2) -WalkUpFromBinAddAssign CompoundAssignOperator(+=) - WalkUpFromExpr CompoundAssignOperator(+=) - WalkUpFromStmt CompoundAssignOperator(+=) +WalkUpFromExpr CompoundAssignOperator(+=) + WalkUpFromStmt CompoundAssignOperator(+=) WalkUpFromExpr IntegerLiteral(3) WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt CompoundStmt @@ -1616,6 +1636,7 @@ TraverseCallExpr CallExpr(add) WalkUpFromStmt IntegerLiteral(4) WalkUpFromStmt IntegerLiteral(5) WalkUpFromStmt CallExpr(add) +WalkUpFromStmt CallExpr(add) WalkUpFromStmt CompoundStmt )txt")); } @@ -1709,6 +1730,9 @@ TraverseCallExpr CallExpr(add) WalkUpFromCallExpr CallExpr(add) WalkUpFromExpr CallExpr(add) WalkUpFromStmt CallExpr(add) +WalkUpFromCallExpr CallExpr(add) + WalkUpFromExpr CallExpr(add) + WalkUpFromStmt CallExpr(add) WalkUpFromStmt CompoundStmt )txt")); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits