C/C++ OpenACC routine directive, undeclared name error: try to help the user, once

Hi!

On Tue, 31 May 2016 10:26:14 -0400, Nathan Sidwell <nathan@acm.org> wrote:
> 'lexically following' is implementor-speak.  [...]

Thanks for the review, and wording suggestion.

OK for trunk, as follows?

commit 3289032bf7fd7e4a0cce37e7acd71e3330729d83
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Tue May 31 17:46:26 2016 +0200

    C/C++ OpenACC routine directive, undeclared name error: try to help the user, once
    
    	gcc/c/
    	* c-parser.c (c_parser_oacc_routine): If running into an
    	undeclared name error, try to help the user, once.
    	gcc/cp/
    	* parser.c (cp_parser_oacc_routine): If running into an undeclared
    	name error, try to help the user, once.
    	gcc/testsuite/
    	* c-c++-common/goacc/routine-5.c: Update.
---
 gcc/c/c-parser.c                             | 16 ++++++++++++++--
 gcc/cp/parser.c                              | 16 ++++++++++++++--
 gcc/testsuite/c-c++-common/goacc/routine-5.c | 15 ++++++++++++++-
 3 files changed, 42 insertions(+), 5 deletions(-)



Grüße
 Thomas

Hi!

Ping.

On Tue, 31 May 2016 17:49:49 +0200, I wrote:
> OK for trunk, as follows?
> 
> commit 3289032bf7fd7e4a0cce37e7acd71e3330729d83
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date:   Tue May 31 17:46:26 2016 +0200
> 
>     C/C++ OpenACC routine directive, undeclared name error: try to help the user, once
>     
>     	gcc/c/
>     	* c-parser.c (c_parser_oacc_routine): If running into an
>     	undeclared name error, try to help the user, once.
>     	gcc/cp/
>     	* parser.c (cp_parser_oacc_routine): If running into an undeclared
>     	name error, try to help the user, once.
>     	gcc/testsuite/
>     	* c-c++-common/goacc/routine-5.c: Update.
> ---
>  gcc/c/c-parser.c                             | 16 ++++++++++++++--
>  gcc/cp/parser.c                              | 16 ++++++++++++++--
>  gcc/testsuite/c-c++-common/goacc/routine-5.c | 15 ++++++++++++++-
>  3 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index 993c0a0..d3cab69 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -14003,8 +14003,20 @@ c_parser_oacc_routine (c_parser *parser, enum pragma_context context)
>  	{
>  	  decl = lookup_name (token->value);
>  	  if (!decl)
> -	    error_at (token->location, "%qE has not been declared",
> -		      token->value);
> +	    {
> +	      error_at (token->location, "%qE has not been declared",
> +			token->value);
> +	      static bool informed_once = false;
> +	      if (!informed_once)
> +		{
> +		  inform (token->location,
> +			  "omit the %<(%E)%>, if you want to mark the"
> +			  " immediately following function, or place this"
> +			  " pragma after a declaration of the function to be"
> +			  " marked", token->value);
> +		  informed_once = true;
> +		}
> +	    }
>  	  c_parser_consume_token (parser);
>  	}
>        else
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 8841666..0c67608 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -36528,8 +36528,20 @@ cp_parser_oacc_routine (cp_parser *parser, cp_token *pragma_tok,
>  					 /*optional_p=*/false);
>        decl = cp_parser_lookup_name_simple (parser, id, token->location);
>        if (id != error_mark_node && decl == error_mark_node)
> -	cp_parser_name_lookup_error (parser, id, decl, NLE_NULL,
> -				     token->location);
> +	{
> +	  cp_parser_name_lookup_error (parser, id, decl, NLE_NULL,
> +				       token->location);
> +	  static bool informed_once = false;
> +	  if (!informed_once)
> +	    {
> +	      inform (token->location,
> +		      "omit the %<(%E)%>, if you want to mark the"
> +		      " immediately following function, or place this"
> +		      " pragma after a declaration of the function to be"
> +		      " marked", id);
> +	      informed_once = true;
> +	    }
> +	}
>  
>        if (decl == error_mark_node
>  	  || !cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN))
> diff --git gcc/testsuite/c-c++-common/goacc/routine-5.c gcc/testsuite/c-c++-common/goacc/routine-5.c
> index 1efd154..def78cd 100644
> --- gcc/testsuite/c-c++-common/goacc/routine-5.c
> +++ gcc/testsuite/c-c++-common/goacc/routine-5.c
> @@ -71,7 +71,20 @@ void Foo ()
>  
>  #pragma acc routine (Foo) gang // { dg-error "must be applied before definition" }
>  
> -#pragma acc routine (Baz) // { dg-error "not been declared" }
> +#pragma acc routine (Baz) worker
> +/* { dg-error ".Baz. has not been declared" "" { target *-*-* } 74 }
> +   Try to help the user:
> +   { dg-message "note: omit the .\\(Baz\\)., if" "" { target *-*-* } 74 } */
> +
> +#pragma acc routine (Baz) vector
> +/* { dg-error ".Baz. has not been declared" "" { target *-*-* } 79 }
> +   Don't try to help the user again:
> +   { dg-bogus "note: omit the .\\(Baz\\)., if" "" { target *-*-* } 79 } */
> +
> +#pragma acc routine (Qux) seq
> +/* { dg-error ".Qux. has not been declared" "" { target *-*-* } 84 }
> +   Don't try to help the user again:
> +   { dg-bogus "note: omit the .\\(Qux\\)., if" "" { target *-*-* } 84 } */
>  
>  
>  int vb1;		/* { dg-error "directive for use" } */


Grüße
 Thomas

On Tue, Jun 07, 2016 at 08:44:16AM +0200, Thomas Schwinge wrote:
> On Tue, 31 May 2016 17:49:49 +0200, I wrote:
> > OK for trunk, as follows?
> > 
> > commit 3289032bf7fd7e4a0cce37e7acd71e3330729d83
> > Author: Thomas Schwinge <thomas@codesourcery.com>
> > Date:   Tue May 31 17:46:26 2016 +0200
> > 
> >     C/C++ OpenACC routine directive, undeclared name error: try to help the user, once
> >     
> >     	gcc/c/
> >     	* c-parser.c (c_parser_oacc_routine): If running into an
> >     	undeclared name error, try to help the user, once.
> >     	gcc/cp/
> >     	* parser.c (cp_parser_oacc_routine): If running into an undeclared
> >     	name error, try to help the user, once.
> >     	gcc/testsuite/
> >     	* c-c++-common/goacc/routine-5.c: Update.

I don't like hinting user something that very often doesn't make any sense.

Wouldn't it be better to defer the inform call until we parse the next decl?

E.g. in the C++ FE arrange to have parser->oacc_routine set after the
directive, but with some new flag in there, and just throw it away silently
if it is not followed by function declaration, and emit the inform only if
the next declaration actually is the symbol mentioned in there?

In your testcase it is not followed by Baz declaration or definition, so
emitting the hint is just confusing.

	Jakub