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 @@ -331,6 +331,32 @@ struct has_same_member_pointer_type : std::true_type {}; + template struct is_same_method_impl { + template + static bool isSameMethod(FirstMethodPtrTy FirstMethodPtr, + SecondMethodPtrTy SecondMethodPtr) { + return false; + } + }; + + template <> struct is_same_method_impl { + template + 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 + bool isSameMethod(FirstMethodPtrTy FirstMethodPtr, + SecondMethodPtrTy SecondMethodPtr) { + return is_same_method_impl< + has_same_member_pointer_type:: + 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 @@ -609,17 +635,38 @@ #define ABSTRACT_STMT(STMT) #define STMT(CLASS, PARENT) \ case Stmt::CLASS##Class: \ - TRY_TO(WalkUpFrom##CLASS(static_cast(S))); break; + /* 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))); \ + } \ + 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); \ 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" } @@ -2220,8 +2267,13 @@ TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt); \ } \ } \ - if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder()) \ + /* 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()) { \ TRY_TO(WalkUpFrom##STMT(S)); \ + } \ return ReturnValue; \ } @@ -2392,6 +2444,9 @@ for (Stmt *SubStmt : S->children()) { TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt); } + + if (!Queue && getDerived().shouldTraversePostOrder()) + TRY_TO(WalkUpFromInitListExpr(S)); } return true; } diff --git a/clang/lib/Tooling/Syntax/BuildTree.cpp b/clang/lib/Tooling/Syntax/BuildTree.cpp --- a/clang/lib/Tooling/Syntax/BuildTree.cpp +++ b/clang/lib/Tooling/Syntax/BuildTree.cpp @@ -578,15 +578,19 @@ // RAV traverses it as a statement, we produce invalid node kinds in that // case. // FIXME: should do this in RAV instead? - 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 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; } bool TraverseStmt(Stmt *S) { 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 @@ -117,18 +117,13 @@ Code, R"txt( TraverseIntegerLiteral IntegerLiteral(1) -WalkUpFromStmt IntegerLiteral(1) TraverseIntegerLiteral IntegerLiteral(2) -WalkUpFromStmt IntegerLiteral(2) TraverseIntegerLiteral IntegerLiteral(3) -WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt DeclRefExpr(add) WalkUpFromStmt ImplicitCastExpr TraverseIntegerLiteral IntegerLiteral(4) -WalkUpFromStmt IntegerLiteral(4) TraverseIntegerLiteral IntegerLiteral(5) -WalkUpFromStmt IntegerLiteral(5) WalkUpFromStmt CallExpr(add) WalkUpFromStmt CompoundStmt )txt")); @@ -189,18 +184,13 @@ Code, R"txt( TraverseIntegerLiteral IntegerLiteral(1) -WalkUpFromIntegerLiteral IntegerLiteral(1) TraverseIntegerLiteral IntegerLiteral(2) -WalkUpFromIntegerLiteral IntegerLiteral(2) TraverseIntegerLiteral IntegerLiteral(3) -WalkUpFromIntegerLiteral IntegerLiteral(3) WalkUpFromExpr BinaryOperator(+) WalkUpFromExpr DeclRefExpr(add) WalkUpFromExpr ImplicitCastExpr TraverseIntegerLiteral IntegerLiteral(4) -WalkUpFromIntegerLiteral IntegerLiteral(4) TraverseIntegerLiteral IntegerLiteral(5) -WalkUpFromIntegerLiteral IntegerLiteral(5) WalkUpFromExpr CallExpr(add) WalkUpFromStmt CompoundStmt )txt")); @@ -315,7 +305,6 @@ WalkUpFromStmt IntegerLiteral(1) WalkUpFromStmt IntegerLiteral(2) WalkUpFromStmt IntegerLiteral(3) -WalkUpFromStmt BinaryOperator(+) WalkUpFromStmt DeclRefExpr(add) WalkUpFromStmt ImplicitCastExpr WalkUpFromStmt IntegerLiteral(4) @@ -383,7 +372,6 @@ WalkUpFromExpr IntegerLiteral(1) WalkUpFromExpr IntegerLiteral(2) WalkUpFromExpr IntegerLiteral(3) -WalkUpFromBinaryOperator BinaryOperator(+) WalkUpFromExpr DeclRefExpr(add) WalkUpFromExpr ImplicitCastExpr WalkUpFromExpr IntegerLiteral(4) @@ -500,7 +488,6 @@ WalkUpFromStmt IntegerLiteral(3) WalkUpFromStmt BinaryOperator(+) TraverseCallExpr CallExpr(add) -WalkUpFromStmt CallExpr(add) WalkUpFromStmt CompoundStmt )txt")); } @@ -560,7 +547,6 @@ WalkUpFromExpr IntegerLiteral(3) WalkUpFromExpr BinaryOperator(+) TraverseCallExpr CallExpr(add) -WalkUpFromCallExpr CallExpr(add) WalkUpFromStmt CompoundStmt )txt")); }