Re: [PATCH] mixing of labels and code in C2X

[Joseph Myers <joseph@codesourcery.com>]

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
joseph@codesourcery.com