Re: [PATCH 1/2] asm qualifiers (PR55681)

[Segher Boessenkool <segher@kernel.crashing.org>]

+cc: C and C++ maintainers.  Sorry I forgot before :-/

On Tue, Oct 30, 2018 at 05:30:33PM +0000, Segher Boessenkool wrote:
> PR55681 observes that currently only one qualifier is allowed for
> inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
> okay (with a warning), but "const volatile asm" gives an error.  Also
> "const const asm" is an error (while "const const int" is okay for C),
> "goto" has to be last, and "_Atomic" isn't handled at all.
> 
> This patch fixes all these.  It allows any order of qualifiers (and
> goto), allows them all for C, allows duplications for C.  For C++ it
> still allows only a single volatile and single goto, but in any order.
> 
> 
> 2018-10-30  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> gcc/c/
> 	PR inline-asm/55681
> 	* c-parser.c (c_parser_for_statement): Update grammar.  Allow any
> 	combination of type-qualifiers and goto in any order, with repetitions
> 	allowed.
> 
> gcc/cp/
> 	PR inline-asm/55681
> 	* parser.c (cp_parser_using_directive): Update grammar.  Allow any
> 	combination of volatile and goto in any order, without repetitions.
> 
> ---
>  gcc/c/c-parser.c | 66 +++++++++++++++++++++++++++---------------------
>  gcc/cp/parser.c  | 77 +++++++++++++++++++++++++++++++++++++-------------------
>  2 files changed, 89 insertions(+), 54 deletions(-)
> 
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index ee66ce8..ce9921e 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -6283,23 +6283,31 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
>  }
>  
>  /* Parse an asm statement, a GNU extension.  This is a full-blown asm
> -   statement with inputs, outputs, clobbers, and volatile tag
> +   statement with inputs, outputs, clobbers, and volatile and goto tag
>     allowed.
>  
> +   asm-qualifier:
> +     type-qualifier
> +     goto
> +
> +   asm-qualifier-list:
> +     asm-qualifier-list asm-qualifier
> +     asm-qualifier
> +
>     asm-statement:
> -     asm type-qualifier[opt] ( asm-argument ) ;
> -     asm type-qualifier[opt] goto ( asm-goto-argument ) ;
> +     asm asm-qualifier-list[opt] ( asm-argument ) ;
>  
>     asm-argument:
>       asm-string-literal
>       asm-string-literal : asm-operands[opt]
>       asm-string-literal : asm-operands[opt] : asm-operands[opt]
> -     asm-string-literal : asm-operands[opt] : asm-operands[opt] : asm-clobbers[opt]
> -
> -   asm-goto-argument:
> +     asm-string-literal : asm-operands[opt] : asm-operands[opt] \
> +       : asm-clobbers[opt]
>       asm-string-literal : : asm-operands[opt] : asm-clobbers[opt] \
>         : asm-goto-operands
>  
> +   The form with asm-goto-operands is valid if and only if the
> +   asm-qualifier-list contains goto, and is the only allowed form in that case.
>     Qualifiers other than volatile are accepted in the syntax but
>     warned for.  */
>  
> @@ -6313,30 +6321,32 @@ c_parser_asm_statement (c_parser *parser)
>  
>    gcc_assert (c_parser_next_token_is_keyword (parser, RID_ASM));
>    c_parser_consume_token (parser);
> -  if (c_parser_next_token_is_keyword (parser, RID_VOLATILE))
> -    {
> -      quals = c_parser_peek_token (parser)->value;
> -      c_parser_consume_token (parser);
> -    }
> -  else if (c_parser_next_token_is_keyword (parser, RID_CONST)
> -	   || c_parser_next_token_is_keyword (parser, RID_RESTRICT))
> -    {
> -      warning_at (c_parser_peek_token (parser)->location,
> -		  0,
> -		  "%E qualifier ignored on asm",
> -		  c_parser_peek_token (parser)->value);
> -      quals = NULL_TREE;
> -      c_parser_consume_token (parser);
> -    }
> -  else
> -    quals = NULL_TREE;
>  
> +  quals = NULL_TREE;
>    is_goto = false;
> -  if (c_parser_next_token_is_keyword (parser, RID_GOTO))
> -    {
> -      c_parser_consume_token (parser);
> -      is_goto = true;
> -    }
> +  for (bool done = false; !done; )
> +    switch (c_parser_peek_token (parser)->keyword)
> +      {
> +      case RID_VOLATILE:
> +	quals = c_parser_peek_token (parser)->value;
> +	c_parser_consume_token (parser);
> +	break;
> +      case RID_CONST:
> +      case RID_RESTRICT:
> +      case RID_ATOMIC:
> +	warning_at (c_parser_peek_token (parser)->location,
> +		    0,
> +		    "%E qualifier ignored on asm",
> +		    c_parser_peek_token (parser)->value);
> +	c_parser_consume_token (parser);
> +	break;
> +      case RID_GOTO:
> +	is_goto = true;
> +	c_parser_consume_token (parser);
> +	break;
> +      default:
> +	done = true;
> +      }
>  
>    /* ??? Follow the C++ parser rather than using the
>       lex_untranslated_string kludge.  */
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index ebe326e..d44fd4d 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -19196,22 +19196,34 @@ cp_parser_using_directive (cp_parser* parser)
>  
>  /* Parse an asm-definition.
>  
> +  asm-qualifier:
> +    volatile
> +    goto
> +
> +  asm-qualifier-list:
> +    asm-qualifier
> +    asm-qualifier-list asm-qualifier
> +
>     asm-definition:
>       asm ( string-literal ) ;
>  
>     GNU Extension:
>  
>     asm-definition:
> -     asm volatile [opt] ( string-literal ) ;
> -     asm volatile [opt] ( string-literal : asm-operand-list [opt] ) ;
> -     asm volatile [opt] ( string-literal : asm-operand-list [opt]
> -			  : asm-operand-list [opt] ) ;
> -     asm volatile [opt] ( string-literal : asm-operand-list [opt]
> -			  : asm-operand-list [opt]
> +     asm asm-qualifier-list [opt] ( string-literal ) ;
> +     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt] ) ;
> +     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt]
> +				    : asm-operand-list [opt] ) ;
> +     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt]
> +				    : asm-operand-list [opt]
>  			  : asm-clobber-list [opt] ) ;
> -     asm volatile [opt] goto ( string-literal : : asm-operand-list [opt]
> -			       : asm-clobber-list [opt]
> -			       : asm-goto-list ) ;  */
> +     asm asm-qualifier-list [opt] ( string-literal : : asm-operand-list [opt]
> +				    : asm-clobber-list [opt]
> +				    : asm-goto-list ) ;
> +
> +  The form with asm-goto-list is valid if and only if the asm-qualifier-list
> +  contains goto, and is the only allowed form in that case.  No duplicates are
> +  allowed in an asm-qualifier-list.  */
>  
>  static void
>  cp_parser_asm_definition (cp_parser* parser)
> @@ -19240,23 +19252,36 @@ cp_parser_asm_definition (cp_parser* parser)
>      }
>  
>    /* See if the next token is `volatile'.  */
> -  if (cp_parser_allow_gnu_extensions_p (parser)
> -      && cp_lexer_next_token_is_keyword (parser->lexer, RID_VOLATILE))
> -    {
> -      /* Remember that we saw the `volatile' keyword.  */
> -      volatile_p = true;
> -      /* Consume the token.  */
> -      cp_lexer_consume_token (parser->lexer);
> -    }
> -  if (cp_parser_allow_gnu_extensions_p (parser)
> -      && parser->in_function_body
> -      && cp_lexer_next_token_is_keyword (parser->lexer, RID_GOTO))
> -    {
> -      /* Remember that we saw the `goto' keyword.  */
> -      goto_p = true;
> -      /* Consume the token.  */
> -      cp_lexer_consume_token (parser->lexer);
> -    }
> +  if (cp_parser_allow_gnu_extensions_p (parser))
> +    for (bool done = false; !done ; )
> +      switch (cp_lexer_peek_token (parser->lexer)->keyword)
> +	{
> +	case RID_VOLATILE:
> +	  if (!volatile_p)
> +	    {
> +	      /* Remember that we saw the `volatile' keyword.  */
> +	      volatile_p = true;
> +	      /* Consume the token.  */
> +	      cp_lexer_consume_token (parser->lexer);
> +	    }
> +	  else
> +	    done = true;
> +	  break;
> +	case RID_GOTO:
> +	  if (!goto_p && parser->in_function_body)
> +	    {
> +	      /* Remember that we saw the `goto' keyword.  */
> +	      goto_p = true;
> +	      /* Consume the token.  */
> +	      cp_lexer_consume_token (parser->lexer);
> +	    }
> +	  else
> +	    done = true;
> +	  break;
> +	default:
> +	  done = true;
> +	}
> +
>    /* Look for the opening `('.  */
>    if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
>      return;
> -- 
> 1.8.3.1