[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 6 09:47:05 PST 2019


lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Anyways, this looks good in this state.
Thank you for working on this!

If you don't intend to immediately work on fixing the rest of `,` cases here,
*please* do file a bug so it won't get lost. (and link it here)



================
Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:27
 
+AST_MATCHER_P(Expr, skipCommaOps,
+             ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
----------------
djtodoro wrote:
> lebedev.ri wrote:
> > djtodoro wrote:
> > > lebedev.ri wrote:
> > > > (Can anyone suggest a better name maybe?
> > > > I'm not sure this is the most correct name, but i can't suggest a better alternative.)
> > > Maybe `evalCommaExpr`?
> > Ah, yes, `eval` sounds better.
> > But, it won't always eval comma op or bailout.
> > It will eval if it is there.
> > So maybe `maybeEvalCommaExpr` (pun intended!).
> Sure, `maybeEvalCommaExpr ` sounds better.
> Or, one more suggestion is `tryEvalCommaExpr` ?
`try`, at least to me, has the same meaning as `evalCommaExpr`, i.e. one might expect that if it fails, it will fail.
So not sure.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58894/new/

https://reviews.llvm.org/D58894





More information about the cfe-commits mailing list