[clang] c19c6b1 - Make RecursiveASTVisitor call WalkUpFrom for unary and binary operators in post-order traversal mode

Dmitri Gribenko via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 6 04:41:43 PDT 2020


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


        


More information about the cfe-commits mailing list