[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