[cfe-commits] [PATCH] Better diagnostics for iterators missing ++, !=, and * in range-based for loops.

Sam Panzer reviews at llvm-reviews.chandlerc.com
Thu Sep 6 14:25:37 PDT 2012


  Changed the notes as per rsmith's suggestions.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1427-1429
@@ -1426,1 +1426,5 @@
   "with '*'?">;
+def note_for_range_invalid_iterator : Note <
+  "cannot use type %0 as iterator in range-based for loop; "
+  "no viable operator %select{!=|*|++}1 is defined">;
+def note_for_range_loop_variable_type : Note <
----------------
Richard Smith wrote:
> Perhaps this would be clearer if presented as a context-style note:
> 
>   in implicit call to 'operator!=' for iterator of type %0
> 
> That'd also remove the issue that 'no viable operator' isn't necessarily the problem (it could be an ambiguous operator, or an inaccessible destructor, etc).
Done. I feel it would be better still as the primary error message, if that becomes possible.

================
Comment at: test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp:51
@@ -51,1 +50,3 @@
+  for (char *a : A()) { // expected-error {{cannot initialize a variable of type 'char *' with an lvalue of type 'int'}} \
+    expected-note {{loop variable a has type 'int'}}
   }
----------------
Richard Smith wrote:
> This note should presumably either say
> 
>    loop variable 'a' has type 'char *'
> 
> or
> 
>    loop variable 'a' is initialized by a value of type 'int'
> 
> Also, in these cases, it's not clear to me that this note is adding much value (since the primary diagnostic already mentions both relevant types). Are there other cases where the value is higher?
I wasn't sure about this note either, and I can't remember the case where it seemed necessary.


http://llvm-reviews.chandlerc.com/D32



More information about the cfe-commits mailing list