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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks, this looks great. A couple of the changes to the tests look like the diagnostic output is slightly worse in some error recovery conditions, but generally this is a nice improvement.



================
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}} \
----------------
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).


================
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}}
   }
----------------
The new warnings here aren't ideal; do you know why they show up?


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

https://reviews.llvm.org/D55955