diff mbox

[C++] PR 63985

Message ID 5498925E.2070908@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Dec. 22, 2014, 9:51 p.m. UTC 
Hi,

On 12/22/2014 10:12 PM, Jason Merrill wrote:
> We could also peek for a colon in cp_parser_init_declarator after 
> parsing the initializer, and give an error then.
I see.
>> For
>> the record, clang vs edg appear to handle this case differently: clang
>> considers it a wrong for loop, edg a wrong range-based for loop. Humm...
>
> They're both right.  :)
Well, if they are both right, could we maybe do something closer to 
clang, instead, thus not consider the for loop a range-based for loop 
only because after the initializer there is a colon when the decl is 
error_mark_node? As an heuristics I think it may make sense in these 
borderline cases because the declarator is more restricted for a 
range-based. The below passes testing.

Thanks,
Paolo.

/////////////

Comments

Jason Merrill Dec. 22, 2014, 9:58 p.m. UTC | #1 
On 12/22/2014 04:51 PM, Paolo Carlini wrote:
> Well, if they are both right, could we maybe do something closer to
> clang, instead, thus not consider the for loop a range-based for loop
> only because after the initializer there is a colon when the decl is
> error_mark_node?

I think we want to tell the user that you can't have an explicit 
initializer in a range-based for loop, since that's the mistake that 
they've made; that seems more helpful than saying you can't have a colon 
after a for-init-statement.

Jason
Paolo Carlini Dec. 23, 2014, 11:22 a.m. UTC | #2 
.. and, to make things more interesting ;) for:

   for (int a, b, c: arr)

we currently give:

63985.C:7:19: error: expected initializer before ‘:’ token

63985.C:7:21: warning: range-based ‘for’ loops only available with 
-std=c++11 or -std=gnu++11

because, inside cp_parse_init_declarator we are in "error mode" for 
range-based after the first comma thus, as a for loop, we complain about 
the missing initializer; then in cp_parser_for_init_statement we notice 
the ':' and we give the warning / we think is a range-based.

Paolo.
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 219030)
+++ cp/parser.c	(working copy)
@@ -10894,16 +10894,26 @@  cp_parser_for_init_statement (cp_parser* parser, t
       parser->colon_corrects_to_scope_p = saved_colon_corrects_to_scope_p;
       if (cp_lexer_next_token_is (parser->lexer, CPP_COLON))
 	{
-	  /* It is a range-for, consume the ':' */
-	  cp_lexer_consume_token (parser->lexer);
-	  is_range_for = true;
-	  if (cxx_dialect < cxx11)
+	  if (*decl == error_mark_node)
 	    {
-	      pedwarn (cp_lexer_peek_token (parser->lexer)->location, 0,
-		       "range-based %<for%> loops only available with "
-		       "-std=c++11 or -std=gnu++11");
-	      *decl = error_mark_node;
+	      /* Give an error and consume the ':' anyway for better
+		 error recovery.  */
+	      cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
+	      cp_lexer_consume_token (parser->lexer);
 	    }
+	  else
+	    {
+	      /* It is a range-for, consume the ':' */
+	      cp_lexer_consume_token (parser->lexer);
+	      is_range_for = true;
+	      if (cxx_dialect < cxx11)
+		{
+		  pedwarn (cp_lexer_peek_token (parser->lexer)->location, 0,
+			   "range-based %<for%> loops only available with "
+			   "-std=c++11 or -std=gnu++11");
+		  *decl = error_mark_node;
+		}
+	    }
 	}
       else
 	  /* The ';' is not consumed yet because we told
Index: testsuite/g++.dg/cpp0x/range-for29.C
===================================================================
--- testsuite/g++.dg/cpp0x/range-for29.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/range-for29.C	(working copy)
@@ -0,0 +1,10 @@ 
+// PR c++/63985
+// { dg-require-effective-target c++11 }
+
+int main()
+{
+  int arr;
+  for (int i = 5: arr)  // { dg-error "expected ';'" }
+    return 1;
+  return 0;
+}