[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
Alex Lorenz via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 28 07:29:17 PDT 2017
arphaman added a comment.
The fact `TraverseCXXOperatorCallExpr` can't call its super `TraverseCXXOperatorCallExpr` makes the current solution kind of broken. The super implementation of `TraverseCXXOperatorCallExpr` is responsible for dispatch to WalkUp##STMT functions which actually call `VisitCXXOperatorCallExpr` which our current visitor will never call because of the custom "override". I would like to suggest an alternative approach that I think solves the problem more "elegantly":
Modify the `DEF_TRAVERSE_STMT` macro in RecursiveASTVisitor.h call to a wrapper function to get the children:
class RecursiveASTVisitor {
public:
Stmt::child_range getStmtChildren(Stmt *S) {
return S->children();
}
...
#define DEF_TRAVERSE_STMT(STMT, CODE) \
template <typename Derived> \
bool RecursiveASTVisitor<Derived>::Traverse##STMT( \
STMT *S, DataRecursionQueue *Queue) { \
bool ShouldVisitChildren = true; \
bool ReturnValue = true; \
if (!getDerived().shouldTraversePostOrder()) \
TRY_TO(WalkUpFrom##STMT(S)); \
{ CODE; } \
if (ShouldVisitChildren) { \
for (Stmt *SubStmt : getDerived().getStmtChildren(S)) { \ // The only change is on this line.
TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt); \
} \
} \
if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder()) \
TRY_TO(WalkUpFrom##STMT(S)); \
return ReturnValue; \
}
...
};
Then you could provide an "override" in the `LexicallyOrderedRecursiveASTVisitor` that adjusts children just for `CXXOperatorCallExpr`. Something like this should work:
class LexicallyOrderedRecursiveASTVisitor {
public:
...
Stmt::child_range getStmtChildren(Stmt *S) {
return S->children();
}
SmallVector<Stmt *, 8> getStmtChildren(CXXOperatorCallExpr *CE) {
SmallVector<Stmt *, 8> Children(CE->children());
bool Swap;
// Switch the operator and the first operand for all infix and postfix
// operations.
switch (CE->getOperator()) {
case OO_Arrow:
case OO_Call:
case OO_Subscript:
Swap = true;
break;
case OO_PlusPlus:
case OO_MinusMinus:
// These are postfix unless there is exactly one argument.
Swap = Children.size() != 2;
break;
default:
Swap = CE->isInfixBinaryOp();
break;
}
if (Swap && Children.size() > 1)
std::swap(Children[0], Children[1]);
return Children;
}
...
};
WDYT?
Sorry about not seeing this earlier. Thanks for your patience!
https://reviews.llvm.org/D37200
More information about the cfe-commits
mailing list