johannes added inline comments.
================
Comment at: include/clang/AST/RecursiveASTVisitor.h:327
+
+ SmallVector<Stmt *, 8> getStmtChildren(CXXOperatorCallExpr *CE) {
+ SmallVector<Stmt *, 8> Children(CE->children());
----------------
rsmith wrote:
> The copy here is more expensive than I'd like. Instead of returning a list of
> children, please make this function *traverse* the children (which is what
> its one and only caller is going to do anyway) to avoid the copy.
I wasn't sure whether there is a way to do this in a function without writing a
custom iterator that checks whether it should be swapped; I used a different
approach.
================
Comment at: include/clang/AST/RecursiveASTVisitor.h:334
+ case OO_Arrow:
+ case OO_Call:
+ case OO_Subscript:
----------------
rsmith wrote:
> johannes wrote:
> > klimek wrote:
> > > Why do we need to swap for calls?
> > The idea is that the opening parenthesis/bracket comes after the caller,
> > for example in this call with two arguments.
> >
> > ```
> > `-CXXOperatorCallExpr
> > |-ImplicitCastExpr
> > | `-DeclRefExpr operator()
> > |-DeclRefExpr caller
> > |-IntegerLiteral
> > `-IntegerLiteral
> > ```
> >
> > Of course we fail to capture the fact that there is also a closing
> > parenthesis or bracket. So this is no perfect solution.
> > Of course we fail to capture the fact that there is also a closing
> > parenthesis or bracket.
>
> One way we could handle this would be to effectively order by the start
> location for preorder traversal, and by the end location for postorder
> traversal. So for a case like your `caller(1,2)`, we would visit
> `CXXOperatorCallExpr`, `caller`, `operator()`, `1`, then `2` when doing
> preorder visitation, and `caller`, `1`, `2`, `operator()`, then
> `CXXOperatorCallExpr` when doing postorder visitation (because the notional
> source location for the `operator()` invocation extends across both `1` and
> `2` subexpressions).
>
> But I think it's probably not worthwhile to go to this level of detail, and
> treating the `(` as the location of the `operator()` call and ignoring the
> `)` seems like a reasonable approach to me.
Yeah, we can do that if someone needs it.
Another way would be to model the operator as parent of the operands, although
I assume that this would not compose well.
I don't expect it to be feasible but I imagine CXXOperatorCallExpr could also
inherit from DeclRefExpr.
https://reviews.llvm.org/D37663
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
