[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: test/Parser/switch-recovery.cpp:108
               expected-error {{no member named 'x' in the global namespace; did you mean simply 'x'?}} \
-              expected-warning 2 {{expression result unused}}
+              expected-warning {{expression result unused}}
     9:: :y; // expected-error {{expected ';' after expression}} \
----------------
rsmith wrote:
> Hmm, why do we only get one warning here? I'd expect one warning for the `8;` and one for the `x;` (after applying the fixes from the errors).
We get the one for the `8;` but the other one is a `TypoExpr` and it claims it's type dependent, and we don't warn on type dependent so we bail out pretty early in `Expr::isUnusedResultAWarning()`.


================
Comment at: test/SemaCXX/for-range-examples.cpp:181
     for (+x : {1, 2, 3}) {} // expected-error {{undeclared identifier}} expected-error {{expected ';'}}
-    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}}
+    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}} expected-warning {{expression result unused}}
   }
----------------
rsmith wrote:
> The new warnings here aren't ideal; do you know why they show up?
Because the first part in a for loop can be an expression, but only if it's not a range-based for loop. However, I was able to do some lookahead to retain the old behavior here.


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

https://reviews.llvm.org/D55955