[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 3 inline comments as done.
aaron.ballman added inline comments.


================
Comment at: lib/Sema/SemaStmt.cpp:2846
                         diag::warn_empty_range_based_for_body);
+  DiagnoseUnusedExprResult(B);
 
----------------
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?


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

https://reviews.llvm.org/D55955