This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/4] c/c++, asm: Use nicer error for duplicate asm qualifiers


On Mon, 2018-12-10 at 22:47 +0000, Segher Boessenkool wrote:

[...]

> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index 121a91c..652e53c 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -6360,41 +6360,54 @@ c_parser_for_statement (c_parser *parser,
> bool ivdep, unsigned short unroll,
>  static tree
>  c_parser_asm_statement (c_parser *parser)
>  {
> -  tree quals, str, outputs, inputs, clobbers, labels, ret;
> -  bool simple, is_volatile, is_inline, is_goto;
> +  tree str, outputs, inputs, clobbers, labels, ret;
> +  bool simple;
>    location_t asm_loc = c_parser_peek_token (parser)->location;
>    int section, nsections;
>  
>    gcc_assert (c_parser_next_token_is_keyword (parser, RID_ASM));
>    c_parser_consume_token (parser);
>  
> -  quals = NULL_TREE;
> -  is_volatile = false;
> -  is_inline = false;
> -  is_goto = false;
> +  /* Handle the asm-qualifier-list.  */
> +  location_t volatile_loc = UNKNOWN_LOCATION;
> +  location_t inline_loc = UNKNOWN_LOCATION;
> +  location_t goto_loc = UNKNOWN_LOCATION;
>    for (;;)
>      {
> -      switch (c_parser_peek_token (parser)->keyword)
> +      c_token *token = c_parser_peek_token (parser);
> +      location_t loc = token->location;
> +      switch (token->keyword)
>  	{
>  	case RID_VOLATILE:
> -	  if (is_volatile)
> -	    break;
> -	  is_volatile = true;
> -	  quals = c_parser_peek_token (parser)->value;
> +	  if (volatile_loc)
> +	    {
> +	      error_at (loc, "duplicate asm qualifier %qE", token-
> >value);
> +	      inform (volatile_loc, "first seen here");
> +	    }

Thanks for the improvements.

Is there test coverage for these errors and notes?

A diagnostic nit (new with gcc 9): please add an:
    auto_diagnostic_group d;
to the start of the guarded block, so that the "error" and "note" are
known to be related.

See:	
https://gcc.gnu.org/onlinedocs/gccint/Guidelines-for-Diagnostics.html#Group-logically-related-diagnostics


> +	  else
> +	    volatile_loc = loc;
>  	  c_parser_consume_token (parser);
>  	  continue;
>  
>  	case RID_INLINE:
> -	  if (is_inline)
> -	    break;
> -	  is_inline = true;
> +	  if (inline_loc)
> +	    {
> +	      error_at (loc, "duplicate asm qualifier %qE", token-
> >value);
> +	      inform (inline_loc, "first seen here");

Likewise.

> +	    }
> +	  else
> +	    inline_loc = loc;
>  	  c_parser_consume_token (parser);
>  	  continue;
>  
>  	case RID_GOTO:
> -	  if (is_goto)
> -	    break;
> -	  is_goto = true;
> +	  if (goto_loc)
> +	    {
> +	      error_at (loc, "duplicate asm qualifier %qE", token-
> >value);
> +	      inform (goto_loc, "first seen here");
> +	    }

Likewise.

> +	  else
> +	    goto_loc = loc;
>  	  c_parser_consume_token (parser);
>  	  continue;

[...]
 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 1cc34ba..06a6bb0 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -19649,29 +19646,50 @@ cp_parser_asm_definition (cp_parser*
> parser)
>      }
>  
>    /* Handle the asm-qualifier-list.  */
> +  location_t volatile_loc = UNKNOWN_LOCATION;
> +  location_t inline_loc = UNKNOWN_LOCATION;
> +  location_t goto_loc = UNKNOWN_LOCATION;
>    if (cp_parser_allow_gnu_extensions_p (parser))
>      for (;;)
>        {
> +	cp_token *token = cp_lexer_peek_token (parser->lexer);
> +	location_t loc = token->location;
>  	switch (cp_lexer_peek_token (parser->lexer)->keyword)
>  	  {
>  	  case RID_VOLATILE:
> -	    if (volatile_p)
> -	      break;
> -	    volatile_p = true;
> +	    if (volatile_loc)
> +	      {
> +		error_at (loc, "duplicate asm qualifier %qT", token-
> >u.value);
> +		inform (volatile_loc, "first seen here");

Likewise.

> +	      }
> +	    else
> +	      volatile_loc = loc;
>  	    cp_lexer_consume_token (parser->lexer);
>  	    continue;
>  
>  	  case RID_INLINE:
> -	    if (inline_p || !parser->in_function_body)
> +	    if (!parser->in_function_body)
>  	      break;
> -	    inline_p = true;
> +	    if (inline_loc)
> +	      {
> +		error_at (loc, "duplicate asm qualifier %qT", token-
> >u.value);
> +		inform (inline_loc, "first seen here");

Likewise.

> +	      }
> +	    else
> +	      inline_loc = loc;
>  	    cp_lexer_consume_token (parser->lexer);
>  	    continue;
>  
>  	  case RID_GOTO:
> -	    if (goto_p || !parser->in_function_body)
> +	    if (!parser->in_function_body)
>  	      break;
> -	    goto_p = true;
> +	    if (goto_loc)
> +	      {
> +		error_at (loc, "duplicate asm qualifier %qT", token-
> >u.value);
> +		inform (goto_loc, "first seen here");

Likewise.

> +	      }
> +	    else
> +	      goto_loc = loc;
>  	    cp_lexer_consume_token (parser->lexer);
>  	    continue;
>  

[...]


Dave


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]