On Sun, 1 Nov 2020, Uecker, Martin wrote:
> @@ -5693,27 +5692,54 @@ c_parser_compound_statement_nostart (c_parser *parser)
> last_label = true;
> last_stmt = false;
> mark_valid_location_for_stdc_pragma (false);
> - c_parser_label (parser);
> + c_parser_label (parser, std_attrs);
> +
> + /* Allow '__attribute__((fallthrough));'. */
> + if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
> + {
> + location_t loc = c_parser_peek_token (parser)->location;
> + tree attrs = c_parser_gnu_attributes (parser);
> + if (attribute_fallthrough_p (attrs))
> + {
> + if (c_parser_next_token_is (parser, CPP_SEMICOLON))
> + {
> + tree fn = build_call_expr_internal_loc (loc,
> + IFN_FALLTHROUGH,
> + void_type_node,
> 0);
> + add_stmt (fn);
> + }
> + else
> + warning_at (loc, OPT_Wattributes, "%<fallthrough%>
> attribute "
> + "not followed by %<;%>");
> + }
> + else if (attrs != NULL_TREE)
> + warning_at (loc, OPT_Wattributes, "only attribute
> %<fallthrough%>"
> + " can be applied to a null statement");
> + }
I don't think this is necessary or correct here.
This code is being moved from c_parser_label. The syntax that
c_parser_label was supposed to handle, as shown in the comment above the
function, is:
label:
identifier : gnu-attributes[opt]
case constant-expression :
default :
GNU extensions:
label:
case constant-expression ... constant-expression :
That is: attributes on a label that is an identifier were parsed, and
applied to the identifier, by code that's unchanged (beyond applying
standard attributes rather than just warning in the caller that they are
unused) by this patch. So this code could only fire in c_parser_label for
a case or default label, a case in which GNU attributes on the label are
not part of the syntax.
Previously, a case or default label could not be followed by a
declaration, so there was no particular concern about consuming prefix GNU
attributes on such a following declaration; the declaration would still
result in a syntax error (and fallthrough attributes were the only valid
case needing to be handled). Now that such a declaration is valid, I
think the attributes should be be parsed specially here, but left to apply
to that declaration. The code in c_parser_declaration_or_fndef that
handles GNU fallthrough attributes should suffice to handle one in this
case; other warnings for empty declarations should suffice in the case
where there are other attributes but no declaration following.
> @@ -5843,12 +5879,10 @@ c_parser_all_labels (c_parser *parser)
> GNU C accepts any expressions without commas, non-constant
> expressions being rejected later. Any standard
> attribute-specifier-sequence before the first label has been parsed
> - in the caller, to distinguish statements from declarations. Any
> - attribute-specifier-sequence after the label is parsed in this
> - function. */
> + in the caller, to distinguish statements from declarations. */
I don't think this change to the comment is accurate. As shown in the
(unmodified) syntax comment, GNU attributes on a label that is an
identifier (the only kind that has them) are still parsed in this
function. It's only fallthrough attributes after case and default labels
(which weren't part of that syntax comment anyway) that are no longer
parsed here.
> @@ -5898,8 +5932,13 @@ c_parser_label (c_parser *parser)
> if (tlab)
> {
> decl_attributes (&tlab, attrs, 0);
> + decl_attributes (&tlab, std_attrs, 0);
> label = add_stmt (build_stmt (loc1, LABEL_EXPR, tlab));
> }
> + if (attrs
> + && c_parser_next_tokens_start_declaration (parser))
> + warning_at (loc2, OPT_Wattributes, "GNU-style attribute between"
> + " label and declaration appertains to the label.");
No trailing '.' on diagnostics.
--
Joseph S. Myers
jos...@codesourcery.com
