Skip to content

Commit

Permalink
Make RecursiveASTVisitor call WalkUpFrom for unary and binary operato…
Browse files Browse the repository at this point in the history 
…rs 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
  • Loading branch information
@gribozavr
gribozavr committed Jul 3, 2020
1 parent 9445444 commit 7b0be96
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 48 deletions.
79 changes: 60 additions & 19 deletions clang/include/clang/AST/RecursiveASTVisitor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,15 +589,13 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,

BINOP_LIST()
#undef OPERATOR
#undef BINOP_LIST

#define OPERATOR(NAME) \
case BO_##NAME##Assign: \
DISPATCH_STMT(Bin##NAME##Assign, CompoundAssignOperator, S);

CAO_LIST()
#undef OPERATOR
#undef CAO_LIST
}
} else if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(S)) {
switch (UnOp->getOpcode()) {
Expand All @@ -607,7 +605,6 @@ bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,

UNARYOP_LIST()
#undef OPERATOR
#undef UNARYOP_LIST
}
}

Expand All @@ -629,35 +626,75 @@ 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: \
/* 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<CLASS *>(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<CLASS *>(S); \
Expand Down Expand Up @@ -3664,6 +3701,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
56 changes: 27 additions & 29 deletions clang/unittests/Tooling/RecursiveASTVisitorTests/Callbacks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -488,15 +487,16 @@ 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(
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
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -739,16 +738,16 @@ 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(
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
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -996,14 +996,15 @@ 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(
WalkUpFromStmt IntegerLiteral(1)
TraverseBinAdd BinaryOperator(+)
WalkUpFromStmt IntegerLiteral(2)
WalkUpFromStmt IntegerLiteral(3)
WalkUpFromStmt BinaryOperator(+)
WalkUpFromStmt IntegerLiteral(4)
WalkUpFromStmt CompoundStmt
)txt"));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -1397,14 +1396,15 @@ 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(
WalkUpFromStmt IntegerLiteral(1)
TraverseBinAddAssign CompoundAssignOperator(+=)
WalkUpFromStmt DeclRefExpr(a)
WalkUpFromStmt IntegerLiteral(2)
WalkUpFromStmt CompoundAssignOperator(+=)
WalkUpFromStmt IntegerLiteral(3)
WalkUpFromStmt CompoundStmt
)txt"));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down

0 comments on commit 7b0be96

Please sign in to comment.