lebedev.ri added a comment.
In D58894#1416691 <https://reviews.llvm.org/D58894#1416691>, @djtodoro wrote:
> @lebedev.ri Thanks for your comment!
>
> > Is there any way to model this more generically?
> > I.e don't duplicate every naive matcher (ignoring possible presence of ,
> > op) with an variant that does not ignore ,.
> > E.g. will this handle (a,b)+=1 ?
>
> Hmm, I am not sure if there is a place for that, since this is very specific
> to comma operators. Any suggestions ?
> It handles the situation (`(a,b)+=1`).
>
> > What about (a,b).callNonConstMethod(), (a,b).callConstMethod() ?
>
> This needs to be extended to support function/method calls. I guess
> `ExprMutationAnalyzer::findFunctionArgMutation` requires an improvement.
Well, given
// LHS of any assignment operators.
const auto AsAssignmentLhs =
binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp)));
The problem is that `hasLHS()` may get `,` op.
In `isAssigmentWithCommma()`, you check if that is `,` op, and if so, return
it's right hand.
And this duplication is the problem as far as i can see.
Now, i don't know the final solution, but have you considered adding something
like:
AST_MATCHER_P(Expr, skipCommaOps,
ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
const Expr* Result = Node;
while (const auto *BOComma =
dyn_cast_or_null<BinaryOperator>(Result->IgnoreParens())) {
if (!BOComma->isCommaOp())
break;
Result = BOComma->getRHS();
}
return Result;
}
and then simply changing
// LHS of any assignment operators.
const auto AsAssignmentLhs =
binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp)));
to
// LHS of any assignment operators.
const auto AsAssignmentLhs =
binaryOperator(isAssignmentOperator(),
hasLHS(skipCommaOps(equalsNode(Exp))));
?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58894/new/
https://reviews.llvm.org/D58894
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
