rsmith added inline comments.
================
Comment at: lib/Sema/SemaStmt.cpp:2846
diag::warn_empty_range_based_for_body);
+ DiagnoseUnusedExprResult(B);
----------------
aaron.ballman wrote:
> rsmith wrote:
> > rsmith wrote:
> > > While this looks correct per the current approach to this function, we
> > > really shouldn't be duplicating calls to this everywhere. Can we move all
> > > the calls to a single call in `ActOnFinishFullStmt`?
> > Looks like that's not actually called from almost anywhere, but checking
> > from `ActOnFinishFullExpr` in the case where `DiscardedValue` is `true`
> > seems appropriate.
> That seems sensible, but how will that work with GNU statement expressions?
> If we check for unused expression results, then we will trigger on code like
> this:
> ```
> int a = ({blah(); yada(); 0;});
> // or
> int b = ({blah(); yada(); some_no_discard_call();});
> ```
> I don't know of a way to handle that case -- we call `ActOnFinishFullExpr()`
> while parsing the compound statement for the `StmtExpr`, so there's no way to
> know whether there's another statement coming (without lookahead) to
> determine whether to suppress the diagnostic for the last expression
> statement. Do you have ideas on how to handle that?
If we're calling `ActOnFinishFullExpr` there with `DiscardedValue == true`,
that would be a bug that we should be fixing regardless. I don't think the
lookahead is so bad itself -- it should just be a one-token lookahead for a `}`
after the `;` -- but it might be awkward to wire it into our compound-statement
/ expression-statement parsing. Still, it seems necessary for correctness.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55955/new/
https://reviews.llvm.org/D55955
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
