On Wed, Jun 26, 2024 at 08:01:57PM +0200, Martin Uecker wrote:
> 
> Thanks Marek, here is the second version which should
> implement all your suggestions.  

Thanks!
 
> (BTW: Without the newline of the end, the test case has
> undefined behavior..., not that we need to care.)
> 
> 
> Bootstrapped and regression tested on x86_64.
> 
> 
>     [PATCH] 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 6a3f96d5b61..db60507252b 100644
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -4715,8 +4715,6 @@ c_parser_direct_declarator_inner (c_parser *parser, 
> bool id_present,
>        location_t brace_loc = c_parser_peek_token (parser)->location;
>        struct c_declarator *declarator;
>        struct c_declspecs *quals_attrs = build_null_declspecs ();
> -      bool static_seen;
> -      bool star_seen;
>        struct c_expr dimen;
>        dimen.value = NULL_TREE;
>        dimen.original_code = ERROR_MARK;
> @@ -4724,7 +4722,8 @@ c_parser_direct_declarator_inner (c_parser *parser, 
> bool id_present,
>        c_parser_consume_token (parser);
>        c_parser_declspecs (parser, quals_attrs, false, false, true,
>                         false, false, false, false, cla_prefer_id);
> -      static_seen = c_parser_next_token_is_keyword (parser, RID_STATIC);
> +      const bool static_seen = c_parser_next_token_is_keyword (parser,
> +                                                            RID_STATIC);
>        if (static_seen)
>       c_parser_consume_token (parser);
>        if (static_seen && !quals_attrs->declspecs_seen_p)
> @@ -4735,38 +4734,34 @@ c_parser_direct_declarator_inner (c_parser *parser, 
> bool id_present,
>        /* If "static" is present, there must be an array dimension.
>        Otherwise, there may be a dimension, "*", or no
>        dimension.  */
> -      if (static_seen)
> +      bool 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)
>       {
> -       if (c_parser_next_token_is (parser, CPP_CLOSE_SQUARE))
> +       if (star_seen)
>           {
> -           dimen.value = NULL_TREE;
> +           error_at (c_parser_peek_token (parser)->location,

Now I realize that the location is not ideal here, it points to the
closing ], but we can easily get the location of "static".  So perhaps:

  location_t static_loc = UNKNOWN_LOCATION;
  if (c_parser_next_token_is_keyword (parser, RID_STATIC))
    {
      static_loc = c_parser_peek_token (parser)->location;
      c_parser_consume_token (parser);
    }

and then use "seen_loc != UNKNOWN_LOCATION" instead of the bool, or
do
  const bool static_seen = (static_loc != UNKNOWN_LOCATION);
if you prefer.

> +                     "%<static%> may not be used with an unspecified "
> +                     "variable length array size");
> +           /* Prevent further errors.  */
>             star_seen = false;
> +           dimen.value = error_mark_node;
>           }
> -       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
> +       else if (!dimen.value)
>           {
> -           star_seen = false;
> -           dimen = c_parser_expr_no_commas (parser, NULL);
> +           error_at (c_parser_peek_token (parser)->location,
> +                     "%<static%> may not be used without an array size");
>           }

No need to have { } around a single statement.

Marek

Reply via email to