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

[Sam Panzer]

Hi rsmith,

This patch adds an additional note when ++__begin, !=__begin, *__begin, or the loop variable declaration is invalid, stating the high-level source of the error (e.g. "cannot use type T as an iterator; no operator ++ is defined"). Ideally, this would be the error message rather than a note, but this should be suffcient to clarify the existing errors.

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

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaStmt.cpp
  test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp

Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1424,6 +1424,11 @@
 def err_for_range_dereference : Error<
   "invalid range expression of type %0; did you mean to dereference it "
   "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 <
+  "loop variable %0 has type %1">;
 def note_for_range_begin_end : Note<
   "selected '%select{begin|end}0' %select{function|template }1%2 with iterator type %3">;
 
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -1959,6 +1959,8 @@
     NotEqExpr = ActOnBooleanCondition(S, ColonLoc, NotEqExpr.get());
     NotEqExpr = ActOnFinishFullExpr(NotEqExpr.get());
     if (NotEqExpr.isInvalid()) {
+      Diag(RangeLoc, diag::note_for_range_invalid_iterator)
+        << RangeLoc << BeginRangeRef.get()->getType() << 0 ;
       NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
       if (!Context.hasSameType(BeginType, EndType))
         NoteForRangeBeginEndFunction(*this, EndExpr.get(), BEF_end);
@@ -1974,6 +1976,8 @@
     IncrExpr = ActOnUnaryOp(S, ColonLoc, tok::plusplus, BeginRef.get());
     IncrExpr = ActOnFinishFullExpr(IncrExpr.get());
     if (IncrExpr.isInvalid()) {
+      Diag(RangeLoc, diag::note_for_range_invalid_iterator)
+        << RangeLoc << BeginRangeRef.get()->getType() << 2;
       NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
       return StmtError();
     }
@@ -1986,16 +1990,21 @@
 
     ExprResult DerefExpr = ActOnUnaryOp(S, ColonLoc, tok::star, BeginRef.get());
     if (DerefExpr.isInvalid()) {
+      Diag(RangeLoc, diag::note_for_range_invalid_iterator)
+        << RangeLoc << BeginRangeRef.get()->getType() << 1;
       NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
       return StmtError();
     }
 
     // Attach  *__begin  as initializer for VD.
     if (!LoopVar->isInvalidDecl()) {
       AddInitializerToDecl(LoopVar, DerefExpr.get(), /*DirectInit=*/false,
                            /*TypeMayContainAuto=*/true);
-      if (LoopVar->isInvalidDecl())
+      if (LoopVar->isInvalidDecl()) {
+        Diag(RangeLoc, diag::note_for_range_loop_variable_type)
+          << RangeLoc << LoopVar->getName() << DerefExpr.get()->getType();
         NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
+      }
     }
   } else {
     // The range is implicitly used as a placeholder when it is dependent.
Index: test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
===================================================================
--- test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
+++ test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
@@ -47,21 +47,28 @@
 void g() {
   for (int a : A())
     A __begin;
-  for (char *a : A()) { // expected-error {{cannot initialize a variable of type 'char *' with an lvalue of type 'int'}}
+  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'}}
   }
-  for (char *a : B()) { // expected-error {{cannot initialize a variable of type 'char *' with an lvalue of type 'int'}}
+  for (char *a : B()) { // expected-error {{cannot initialize a variable of type 'char *' with an lvalue of type 'int'}} \
+    expected-note {{loop variable a has type 'int'}}
+
   }
   // FIXME: Terrible diagnostic here. auto deduction should fail, but does not!
   for (double a : f) { // expected-error {{cannot use type '<overloaded function type>' as a range}}
   }
   for (auto a : A()) {
   }
   for (auto a : B()) {
   }
-  for (auto *a : A()) { // expected-error {{variable 'a' with type 'auto *' has incompatible initializer of type 'int'}}
+  for (auto *a : A()) { // expected-error {{variable 'a' with type 'auto *' has incompatible initializer of type 'int'}} \
+    expected-note {{loop variable a has type 'int'}}
+
   }
   // : is not a typo for :: here.
-  for (A NS:A()) { // expected-error {{no viable conversion from 'int' to 'A'}}
+  for (A NS:A()) { // expected-error {{no viable conversion from 'int' to 'A'}} \
+    expected-note {{loop variable NS has type 'int'}}
+
   }
   for (auto not_in_scope : not_in_scope) { // expected-error {{use of undeclared identifier 'not_in_scope'}}
   }
@@ -129,22 +136,35 @@
   };
   for (auto u : NoBegin()) { // expected-error {{range type 'NoBegin' has 'end' member but no 'begin' member}}
   }
-  for (auto u : NoEnd()) { // expected-error {{range type 'NoEnd' has 'begin' member but no 'end' member}}
+  for (auto u : NoEnd()) { // expected-error {{range type 'NoEnd' has 'begin' member but no 'end' member}} 
   }
 
   struct NoIncr {
     void *begin(); // expected-note {{selected 'begin' function with iterator type 'void *'}}
     void *end();
   };
-  for (auto u : NoIncr()) { // expected-error {{arithmetic on a pointer to void}}
+  for (auto u : NoIncr()) { // expected-error {{arithmetic on a pointer to void}}\
+    expected-note {{cannot use type 'NoIncr' as iterator in range-based for loop; no viable operator ++ is defined}}
   }
 
   struct NoNotEq {
     NoNotEq begin(); // expected-note {{selected 'begin' function with iterator type 'NoNotEq'}}
     NoNotEq end();
     void operator++();
   };
-  for (auto u : NoNotEq()) { // expected-error {{invalid operands to binary expression}}
+  for (auto u : NoNotEq()) { // expected-error {{invalid operands to binary expression}}\
+    expected-note {{cannot use type 'NoNotEq' as iterator in range-based for loop; no viable operator != is defined}}
+  }
+
+  struct NoDeref {
+    NoDeref begin(); // expected-note {{selected 'begin' function}}
+    NoDeref end();
+    void operator++();
+    bool operator!=(NoDeref &);
+  };
+
+  for (auto u : NoDeref()) { // expected-error {{indirection requires pointer operand}} \
+    expected-note {{cannot use type 'NoDeref' as iterator in range-based for loop; no viable operator * is defined}}
   }
 
   struct NoCopy {
@@ -165,7 +185,8 @@
 
 template<typename T, typename U>
 void h(T t) {
-  for (U u : t) { // expected-error {{no viable conversion from 'A' to 'int'}}
+  for (U u : t) { // expected-error {{no viable conversion from 'A' to 'int'}} \
+    expected-note {{loop variable u has type 'A'}}
   }
   for (auto u : t) {
   }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32.1.patch
Type: text/x-patch
Size: 6627 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120829/85883e1a/attachment.bin>