public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C PATCH]  C: Error message for incorrect use of static in array declarations
@ 2024-06-23 18:42 Martin Uecker
  2024-06-24 20:41 ` Marek Polacek
  0 siblings, 1 reply; 2+ messages in thread
From: Martin Uecker @ 2024-06-23 18:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: Joseph Myers


This adds an explicit error message for [static] and [static*] 
(the same as clang has) instead of the generic "error: expected
expression before ']' token", which is not entirely accurate.
For function definitions the subsequent error "[*] can not be
used outside function prototype scope" is then suppressed.


Bootstrapped and regression tested on x86_64.



commit 1157d04764eeeb51fa1098727813dbc092e11dd2
Author: Martin Uecker <uecker@tugraz.at>
Date:   Sat Nov 4 14:39:19 2023 +0100

    C: Error message for incorrect use of static in array declarations.
    
    Add an explicit error messages when c99's static is
    used without a size expression in an array declarator.
    
    gcc/:
            c/c-parser.cc (c_parser_direct_declarator_inner): Add
            error message.
    
    gcc/testsuite:
            gcc.dg/c99-arraydecl-4.c: New test.

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index e83e9c683f7..91b8d24ca78 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -4732,41 +4732,29 @@ c_parser_direct_declarator_inner (c_parser *parser, bool id_present,
 			    false, false, false, false, cla_prefer_id);
       if (!quals_attrs->declspecs_seen_p)
 	quals_attrs = NULL;
-      /* If "static" is present, there must be an array dimension.
-	 Otherwise, there may be a dimension, "*", or no
-	 dimension.  */
-      if (static_seen)
+
+      star_seen = false;
+      if (c_parser_next_token_is (parser, CPP_MULT)
+	  && c_parser_peek_2nd_token (parser)->type == CPP_CLOSE_SQUARE)
 	{
-	  star_seen = false;
-	  dimen = c_parser_expr_no_commas (parser, NULL);
+	  star_seen = true;
+	  c_parser_consume_token (parser);
 	}
-      else
+      else if (!c_parser_next_token_is (parser, CPP_CLOSE_SQUARE))
+	dimen = c_parser_expr_no_commas (parser, NULL);
+
+      if (static_seen && star_seen)
 	{
-	  if (c_parser_next_token_is (parser, CPP_CLOSE_SQUARE))
-	    {
-	      dimen.value = NULL_TREE;
-	      star_seen = false;
-	    }
-	  else if (c_parser_next_token_is (parser, CPP_MULT))
-	    {
-	      if (c_parser_peek_2nd_token (parser)->type == CPP_CLOSE_SQUARE)
-		{
-		  dimen.value = NULL_TREE;
-		  star_seen = true;
-		  c_parser_consume_token (parser);
-		}
-	      else
-		{
-		  star_seen = false;
-		  dimen = c_parser_expr_no_commas (parser, NULL);
-		}
-	    }
-	  else
-	    {
-	      star_seen = false;
-	      dimen = c_parser_expr_no_commas (parser, NULL);
-	    }
+	  error_at (c_parser_peek_token (parser)->location,
+	    "%<static%> may not be used with an unspecified "
+	    "variable length array size");
+	  /* Prevent further errors. */
+	  star_seen = false;
 	}
+      else if (static_seen && NULL_TREE == dimen.value)
+	error_at (c_parser_peek_token (parser)->location,
+		  "%<static%> may not be used without an array size");
+
       if (c_parser_next_token_is (parser, CPP_CLOSE_SQUARE))
 	c_parser_consume_token (parser);
       else
diff --git a/gcc/testsuite/gcc.dg/c99-arraydecl-4.c b/gcc/testsuite/gcc.dg/c99-arraydecl-4.c
new file mode 100644
index 00000000000..bfc26196433
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c99-arraydecl-4.c
@@ -0,0 +1,15 @@
+/* { dg-do "compile" } */
+/* { dg-options "-std=c99 -pedantic-errors" } */
+
+void fo(char buf[static]);	/* { dg-error "'static' may not be used without an array size" } */
+void fo(char buf[static]) { }	/* { dg-error "'static' may not be used without an array size" } */
+
+void fu(char buf[static *]);	/* { dg-error "'static' may not be used with an unspecified variable length array size" } */
+void fu(char buf[static *]) { }	/* { dg-error "'static' may not be used with an unspecified variable length array size" } */
+
+void fe(int n, char buf[static n]);
+void fe(int n, char buf[static *]) { }	/* { dg-error "'static' may not be used with an unspecified variable length array size" } */
+
+void fa(int *n, char buf[static *n]);
+void fa(int *n, char buf[static *n]) { }
+






^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [C PATCH]  C: Error message for incorrect use of static in array declarations
  2024-06-23 18:42 [C PATCH] C: Error message for incorrect use of static in array declarations Martin Uecker
@ 2024-06-24 20:41 ` Marek Polacek
  0 siblings, 0 replies; 2+ messages in thread
From: Marek Polacek @ 2024-06-24 20:41 UTC (permalink / raw)
  To: Martin Uecker; +Cc: gcc-patches, Joseph Myers

On Sun, Jun 23, 2024 at 08:42:36PM +0200, Martin Uecker wrote:
> 
> This adds an explicit error message for [static] and [static*] 
> (the same as clang has) instead of the generic "error: expected
> expression before ']' token", which is not entirely accurate.
> For function definitions the subsequent error "[*] can not be
> used outside function prototype scope" is then suppressed.
> 
> 
> Bootstrapped and regression tested on x86_64.
> 
> 
> 
> commit 1157d04764eeeb51fa1098727813dbc092e11dd2
> Author: Martin Uecker <uecker@tugraz.at>
> Date:   Sat Nov 4 14:39:19 2023 +0100
> 
>     C: Error message for incorrect use of static in array declarations.

Please use "[PATCH] c: ..." for C patches.
     
>     Add an explicit error messages when c99's static is
>     used without a size expression in an array declarator.
>     
>     gcc/:
>             c/c-parser.cc (c_parser_direct_declarator_inner): Add
>             error message.

No "c/" here.

>     
>     gcc/testsuite:
>             gcc.dg/c99-arraydecl-4.c: New test.
> 
> diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
> index e83e9c683f7..91b8d24ca78 100644
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -4732,41 +4732,29 @@ c_parser_direct_declarator_inner (c_parser *parser, bool id_present,
>  			    false, false, false, false, cla_prefer_id);
>        if (!quals_attrs->declspecs_seen_p)
>  	quals_attrs = NULL;

FWIW, I'd prefer to use

  const bool static_seen = c_parser_next_token_is_keyword (parser, RID_STATIC);

> -      /* If "static" is present, there must be an array dimension.
> -	 Otherwise, there may be a dimension, "*", or no
> -	 dimension.  */

Why remove the comment?

> -      if (static_seen)
> +
> +      star_seen = false;

bool star_seen = false; would be nicer IMHO.

> +      if (c_parser_next_token_is (parser, CPP_MULT)
> +	  && c_parser_peek_2nd_token (parser)->type == CPP_CLOSE_SQUARE)
>  	{
> -	  star_seen = false;
> -	  dimen = c_parser_expr_no_commas (parser, NULL);
> +	  star_seen = true;
> +	  c_parser_consume_token (parser);
>  	}
> -      else
> +      else if (!c_parser_next_token_is (parser, CPP_CLOSE_SQUARE))
> +	dimen = c_parser_expr_no_commas (parser, NULL);
> +
> +      if (static_seen && star_seen)
>  	{
> -	  if (c_parser_next_token_is (parser, CPP_CLOSE_SQUARE))
> -	    {
> -	      dimen.value = NULL_TREE;
> -	      star_seen = false;
> -	    }
> -	  else if (c_parser_next_token_is (parser, CPP_MULT))
> -	    {
> -	      if (c_parser_peek_2nd_token (parser)->type == CPP_CLOSE_SQUARE)
> -		{
> -		  dimen.value = NULL_TREE;
> -		  star_seen = true;
> -		  c_parser_consume_token (parser);
> -		}
> -	      else
> -		{
> -		  star_seen = false;
> -		  dimen = c_parser_expr_no_commas (parser, NULL);
> -		}
> -	    }
> -	  else
> -	    {
> -	      star_seen = false;
> -	      dimen = c_parser_expr_no_commas (parser, NULL);
> -	    }
> +	  error_at (c_parser_peek_token (parser)->location,
> +	    "%<static%> may not be used with an unspecified "
> +	    "variable length array size");

The last two lines are not indented enough.

> +	  /* Prevent further errors. */

Two spaces after a '.'.

> +	  star_seen = false;
>  	}
> +      else if (static_seen && NULL_TREE == dimen.value)

Please let's use !dimen.value or dimen.value == NULL_TREE instead.

I think it'd be better to do:

  if (static_seen)
    {
      if (star_seen)
	// ...
      else if (!dimen.value)
	// ...
    }

> +	error_at (c_parser_peek_token (parser)->location,
> +		  "%<static%> may not be used without an array size");
> +
>        if (c_parser_next_token_is (parser, CPP_CLOSE_SQUARE))
>  	c_parser_consume_token (parser);
>        else
> diff --git a/gcc/testsuite/gcc.dg/c99-arraydecl-4.c b/gcc/testsuite/gcc.dg/c99-arraydecl-4.c
> new file mode 100644
> index 00000000000..bfc26196433
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/c99-arraydecl-4.c
> @@ -0,0 +1,15 @@
> +/* { dg-do "compile" } */

It's unusual to quote compile here.

> +/* { dg-options "-std=c99 -pedantic-errors" } */
> +
> +void fo(char buf[static]);	/* { dg-error "'static' may not be used without an array size" } */
> +void fo(char buf[static]) { }	/* { dg-error "'static' may not be used without an array size" } */
> +
> +void fu(char buf[static *]);	/* { dg-error "'static' may not be used with an unspecified variable length array size" } */
> +void fu(char buf[static *]) { }	/* { dg-error "'static' may not be used with an unspecified variable length array size" } */
> +
> +void fe(int n, char buf[static n]);
> +void fe(int n, char buf[static *]) { }	/* { dg-error "'static' may not be used with an unspecified variable length array size" } */

With -Wvla-parameter we get:
c99-arraydecl-4.c:11:21: warning: argument 2 of type 'char[]' declared as an ordinary array
c99-arraydecl-4.c:10:21: note: previously declared as a variable length array 'char[static  n]'
which seems wrong; should we set

  dimen.value = error_mark_node;
  
in the static_seen && star_seen case?

> +void fa(int *n, char buf[static *n]);
> +void fa(int *n, char buf[static *n]) { }
> +

Spurious newline.

Marek


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-06-24 20:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-23 18:42 [C PATCH] C: Error message for incorrect use of static in array declarations Martin Uecker
2024-06-24 20:41 ` Marek Polacek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).