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