diff mbox

[C++] report better diagnostic for static following '[' in parameter declaration

Message ID CAAgBjMkRurZGqT6QgcnBX8CSimYUsyP2xb4oQFioreSvJcOOug@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Jan. 29, 2016, 5:01 p.m. UTC 
On 29 January 2016 at 05:03, Marek Polacek <polacek@redhat.com> wrote:
> On Fri, Jan 29, 2016 at 04:46:56AM +0530, Prathamesh Kulkarni wrote:
>> @@ -19016,10 +19017,22 @@ cp_parser_direct_declarator (cp_parser* parser,
>>         cp_lexer_consume_token (parser->lexer);
>>         /* Peek at the next token.  */
>>         token = cp_lexer_peek_token (parser->lexer);
>> +
>> +       /* If static keyword immediately follows [, report error.  */
>> +       if (cp_lexer_next_token_is_keyword (parser->lexer, RID_STATIC)
>> +           && current_binding_level->kind == sk_function_parms)
>> +         {
>> +           error_at (token->location,
>> +                     "static array size is a C99 feature,"
>> +                     "not permitted in C++");
>> +           bounds = error_mark_node;
>> +         }
>> +
>
> I think this isn't sufficient as-is; if we're changing the diagnostics here,
> we should also handle e.g. void f(int a[const 10]); where clang++ says
> g.C:1:13: error: qualifier in array size is a C99 feature, not permitted in C++
>
> And also e.g.
> void f(int a[const static 10]);
> void f(int a[static const 10]);
> and similar.
Thanks for the review. AFAIK the type-qualifiers would be const,
restrict, volatile and _Atomic (n1570 p 6.7.3) ?
I added a check for those and for variable length array.
I am having issues with writing the test-case,
some cases pass with -std=c++11 but fail with -std=c++98.
Could you please have a look ?

Thanks,
Prathamesh
>
>         Marek

Comments

Manuel López-Ibáñez Jan. 29, 2016, 7:51 p.m. UTC | #1 
On 29/01/16 17:01, Prathamesh Kulkarni wrote:
> Thanks for the review. AFAIK the type-qualifiers would be const,
> restrict, volatile and _Atomic (n1570 p 6.7.3) ?
> I added a check for those and for variable length array.
> I am having issues with writing the test-case,
> some cases pass with -std=c++11 but fail with -std=c++98.
> Could you please have a look ?

Is there _Atomic in C++?

Also, why not simply reuse cp_parser_cv_qualifier_seq_opt (cp_parser* parser), 
perhaps adding a complain parameter that defaults to tf_error and calling it 
here with tf_none.

I think you will get nicer errors if you don't set bounds to error-mark, just 
give the error, consume the tokens and continue as usual.

Ideally, smart error-recovery should only be done when things already go wrong, 
thus after

  bounds	= cp_parser_constant_expression (parser,
					 /*allow_non_constant=*/true,
					 &non_constant_p);
  if (!non_constant_p)
	/* OK */;

fails, however our C++ parser tends to give errors quite deep in the stack 
instead of letting the caller decide what to do, which makes this too noisy in 
this case. Nonetheless, moving this error-recovery within:
if (token->type != CPP_CLOSE_SQUARE)    { }
but before the above can only make the parser (marginally) faster for correct code.

Cheers,

Manuel.
diff mbox

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 33f1df3..04137b3 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -982,6 +982,24 @@  cp_lexer_next_token_is_decl_specifier_keyword (cp_lexer *lexer)
     }
 }
 
+static bool
+cp_lexer_next_token_is_c_type_qual (cp_lexer *lexer)
+{
+  if (cp_lexer_next_token_is_keyword (lexer, RID_CONST) 
+      || cp_lexer_next_token_is_keyword (lexer, RID_VOLATILE))
+    return true;
+
+  cp_token *token = cp_lexer_peek_token (lexer);
+  if (token->type == CPP_NAME)
+    {
+      tree name = token->u.value;
+      const char *p = IDENTIFIER_POINTER (name);
+      return !strcmp (p, "restrict") || !strcmp (p, "_Atomic");
+    }
+
+  return false;
+}
+
 /* Returns TRUE iff the token T begins a decltype type.  */
 
 static bool
@@ -18998,10 +19016,40 @@  cp_parser_direct_declarator (cp_parser* parser,
 	  cp_lexer_consume_token (parser->lexer);
 	  /* Peek at the next token.  */
 	  token = cp_lexer_peek_token (parser->lexer);
+
+	  /* If static or type-qualifier or * immediately follows [,
+	     report error.  */
+	  if (current_binding_level->kind == sk_function_parms)
+	    {
+	      if (cp_lexer_next_token_is_keyword (parser->lexer, RID_STATIC))
+	        {
+		    error_at (token->location,
+			      "static array size is a C99 feature, "
+			      "not permitted in C++");
+		    bounds = error_mark_node;
+		}
+	      else if (cp_lexer_next_token_is_c_type_qual (parser->lexer))
+		{
+		    error_at (token->location,
+			      "qualifier in array size is a C99 feature, "
+			      "not permitted in C++");
+		    bounds = error_mark_node;
+		}
+	      
+	      else if (token->type == CPP_MULT)
+		{
+		    error_at (token->location,
+			      "variable-length array size is a C99 feature, "
+			      "not permitted in C++");
+		    bounds = error_mark_node;
+		}
+	    }
+
 	  /* If the next token is `]', then there is no
 	     constant-expression.  */
-	  if (token->type != CPP_CLOSE_SQUARE)
+	  if (token->type != CPP_CLOSE_SQUARE && bounds != error_mark_node)
 	    {
+
 	      bool non_constant_p;
 	      bounds
 		= cp_parser_constant_expression (parser,
diff --git a/gcc/testsuite/g++.dg/parse/static-array-error.C b/gcc/testsuite/g++.dg/parse/static-array-error.C
new file mode 100644
index 0000000..028320d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/static-array-error.C
@@ -0,0 +1,33 @@ 
+// { dg-do compile }
+
+void f1(int a[static 10]);  /* { dg-error "static array size is a C99 feature" } */
+/* { dg-error "expected '\\]' before 'static'" "" { target *-*-* } 3 } */
+/* { dg-error "expected '\\)' before 'static'" "" { target *-*-* } 3 } */
+/* { dg-error "expected initializer before 'static'" "" { target *-*-* } 3 } */
+
+void f2(int a[const 10]); /* { dg-error "qualifier in array size is a C99 feature" } */
+/* { dg-error "expected '\\]' before 'const'" "" { target *-*-* } 8 } */
+/* { dg-error "expected '\\)' before 'const'" "" { target *-*-* } 8 } */
+/* { dg-error "expected initializer before numeric constant" "" { target *-*-* } 8 } */
+
+void f3(int a[restrict 10]); /* { dg-error "qualifier in array size is a C99 feature" } */
+/* { dg-error "expected '\\]' before 'restrict'" "" { target *-*-* } 13 } */
+/* { dg-error "expected '\\)' before 'restrict'" "" { target *-*-* } 13 } */
+/* { dg-error "expected initializer before 'restrict'" "" { target *-*-* } 13 } */
+
+void f4(int a[volatile 10]); /* { dg-error "qualifier in array size is a C99 feature" } */
+/* { dg-error "expected '\\]' before 'volatile'" "" { target *-*-* } 18 } */
+/* { dg-error "expected '\\)' before 'volatile'" "" { target *-*-* } 18 } */
+/* { dg-error "expected initializer before numeric constant" "" { target *-*-* } 18 } */
+
+
+void f4(int a[_Atomic 10]); /* { dg-error "qualifier in array size is a C99 feature" } */
+/* { dg-error "expected '\\]' before '_Atomic'" "" { target *-*-* } 24 } */
+/* { dg-error "expected '\\)' before '_Atomic'" "" { target *-*-* } 24 } */
+/* { dg-error "expected initializer before '_Atomic'" "" { target *-*-* } 24 } */
+
+
+void f5(int a[*]); /* { dg-error "variable-length array size is a C99 feature" } */
+/* { dg-error "expected ']' before '*' token" "" { target *-*-* } 24 } */
+/* { dg-error "expected '\\)' before '*' token" "" { target *-*-* } 24 } */
+/* { dg-error "expected initializer before '*' token" "" { target *-*-* } 24 } */