RecursiveASTVisitor: don't call WalkUp unnecessarily in post-order traversal

Summary:
How does RecursiveASTVisitor call the WalkUp callback for expressions?

* In pre-order traversal mode, RecursiveASTVisitor calls the WalkUp
  callback from the default implementation of Traverse callbacks.

* In post-order traversal mode when we don't have a DataRecursionQueue,
  RecursiveASTVisitor also calls the WalkUp callback from the default
  implementation of Traverse callbacks.

* However, in post-order traversal mode when we have a DataRecursionQueue,
  RecursiveASTVisitor calls the WalkUp callback from PostVisitStmt.

As a result, when the user overrides the Traverse callback, in pre-order
traversal mode they never get the corresponding WalkUp callback. However
in the post-order traversal mode the WalkUp callback is invoked or not
depending on whether the data recursion optimization could be applied.

I had to adjust the implementation of TraverseCXXForRangeStmt in the
syntax tree builder to call the WalkUp method directly, as it was
relying on this behavior. There is an existing test for this
functionality and it prompted me to make this extra fix.

In addition, I had to fix the default implementation implementation of
RecursiveASTVisitor::TraverseSynOrSemInitListExpr to call WalkUpFrom in
the same manner as the implementation generated by the DEF_TRAVERSE_STMT
macro. Without this fix, the InitListExprIsPostOrderNoQueueVisitedTwice
test was failing because WalkUpFromInitListExpr was never called.

Reviewers: eduucaldas, ymandel

Reviewed By: eduucaldas, ymandel

Subscribers: gribozavr2, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82486

1 files changed, 13 insertions, 9 deletions

diff --git a/clang/lib/Tooling/Syntax/BuildTree.cpp b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 8185af2..4371a30 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -578,15 +578,19 @@ public:
     // 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) {