[openacc] C, C++ OpenACC wait diagnostic change

Message ID 38bae4a6-8ef1-b1df-95e2-54f03e07f173@codesourcery.com
State New
Headers show
Series
  • [openacc] C, C++ OpenACC wait diagnostic change
Related show

Commit Message

Cesar Philippidis Sept. 26, 2018, 6:17 p.m.
Attached is an old patch which updated the C and C++ FEs to use %<)%>
for the right ')' symbol. It's mostly a cosmetic change. All of the
changes are self-contained to the OpenACC code path.

Is this OK for trunk? I bootstrapped and regtested it for x86_64 Linux
with nvptx offloading.

Thanks,
Cesar

Comments

Joseph Myers Sept. 26, 2018, 7:50 p.m. | #1
On Wed, 26 Sep 2018, Cesar Philippidis wrote:

> Attached is an old patch which updated the C and C++ FEs to use %<)%>

> for the right ')' symbol. It's mostly a cosmetic change. All of the

> changes are self-contained to the OpenACC code path.


Why is the "before ')'" included in the call to c_parser_error at all?  
c_parser_error calls c_parse_error which adds its own " before " and token 
description or expansion, so I'd expect the current error to result in a 
message ending in something of the form "before X before Y".

-- 
Joseph S. Myers
joseph@codesourcery.com
Cesar Philippidis Sept. 26, 2018, 9:08 p.m. | #2
On 09/26/2018 12:50 PM, Joseph Myers wrote:
> On Wed, 26 Sep 2018, Cesar Philippidis wrote:

> 

>> Attached is an old patch which updated the C and C++ FEs to use %<)%>

>> for the right ')' symbol. It's mostly a cosmetic change. All of the

>> changes are self-contained to the OpenACC code path.

> 

> Why is the "before ')'" included in the call to c_parser_error at all?  

> c_parser_error calls c_parse_error which adds its own " before " and token 

> description or expansion, so I'd expect the current error to result in a 

> message ending in something of the form "before X before Y".


On closer inspection

 #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1 /* {
dg-error "expected '\\\)' before end of line" } */
-    /* { dg-error "expected integer expression before '\\\)'" "" {
target c++ } .-1 } */
+    /* { dg-error "expected integer expression list before" "" { target
c++ } .-1 } */

so this is only applicable to c++. But in C++ I see duplicate errors
like this

wait.c:29:29: error: expected ‘)’ before end of line
 #pragma acc parallel wait (1
                           ~ ^
                             )
wait.c:29:29: error: expected integer expression list before ‘)’ before
end of line

I suppose for C++ that's an improvement over

wait.c:29:29: error: expected integer expression before ')' before end
of line

Julian, I need to start working on deep copy in OpenACC. Can you take
over this patch? The error handling code in the C FE needs to be removed
because it's dead.

Thanks,
Cesar
Julian Brown Sept. 28, 2018, 1:17 p.m. | #3
On Wed, 26 Sep 2018 14:08:37 -0700
Cesar Philippidis <cesar@codesourcery.com> wrote:

> On 09/26/2018 12:50 PM, Joseph Myers wrote:

> > On Wed, 26 Sep 2018, Cesar Philippidis wrote:

> >   

> >> Attached is an old patch which updated the C and C++ FEs to use

> >> %<)%> for the right ')' symbol. It's mostly a cosmetic change. All

> >> of the changes are self-contained to the OpenACC code path.  

> > 

> > Why is the "before ')'" included in the call to c_parser_error at

> > all? c_parser_error calls c_parse_error which adds its own " before

> > " and token description or expansion, so I'd expect the current

> > error to result in a message ending in something of the form

> > "before X before Y".  


> Julian, I need to start working on deep copy in OpenACC. Can you take

> over this patch? The error handling code in the C FE needs to be

> removed because it's dead.


I agree that the error-handling path in question in the C FE is dead.
The difference is that in C, c_parser_oacc_wait_list parses the open
parenthesis, the list and then the close parenthesis separately, and so
a token sequence like:

   (1

will return an expression list of length 1. In the C++ FE rather, a
cp_parser_parenthesized_expression_list is parsed all in one go, and if
the input is not that well-formed sequence then NULL is returned (or a
zero-length vector for an empty list).

But for C, it does not appear that c_parser_expr_list has a code path
that can return a zero-length list at all. So, we can elide the
diagnostic with no change to compiler behaviour. This patch does that,
and also changes the C++ diagnostic, leading to errors being reported
like:

diag.c: In function 'int main(int, char*)':
diag.c:6:59: error: expected ')' before end of line
6 | #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1
  |                                                         ~ ^
  |                                                           )
diag.c:6:59: error: expected integer expression list before end of line 

Actually I'm not too sure how useful the second error line is. Maybe we
should just remove it to improve consistency between C & C++?

The attached has been tested with offloading to nvptx and bootstrapped.
OK?

Thanks,

Julian

2018-XX-YY  James Norris  <jnorris@codesourcery.com>
	    Cesar Philippidis  <cesar@codesourcery.com>
            Julian Brown  <julian@codesourcery.com>

	gcc/c/
	* c-parser.c (c_parser_oacc_wait_list): Remove dead diagnostic
	code.

        gcc/cp/
        * parser.c (cp_parser_oacc_wait_list): Change error message.

	gcc/testsuite/
        * c-c++-common/goacc/asyncwait-1: Update expected errors.
commit 3a59bdbccc3c2383c0056c74797d698c7d81dce2
Author: Julian Brown <julian@codesourcery.com>
Date:   Fri Sep 28 05:52:55 2018 -0700

    OpenACC wait list diagnostic change

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 1f173fc..92a8089 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -11597,14 +11597,6 @@ c_parser_oacc_wait_list (c_parser *parser, location_t clause_loc, tree list)
     return list;
 
   args = c_parser_expr_list (parser, false, true, NULL, NULL, NULL, NULL);
-
-  if (args->length () == 0)
-    {
-      c_parser_error (parser, "expected integer expression before ')'");
-      release_tree_vector (args);
-      return list;
-    }
-
   args_tree = build_tree_list_vec (args);
 
   for (t = args_tree; t; t = TREE_CHAIN (t))
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6696f17..43128e0 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -32086,7 +32086,7 @@ cp_parser_oacc_wait_list (cp_parser *parser, location_t clause_loc, tree list)
 
   if (args == NULL || args->length () == 0)
     {
-      cp_parser_error (parser, "expected integer expression before ')'");
+      cp_parser_error (parser, "expected integer expression list");
       if (args != NULL)
 	release_tree_vector (args);
       return list;
diff --git a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
index e1840af..2fc8948 100644
--- a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
+++ b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
@@ -116,7 +116,7 @@ f (int N, float *a, float *b)
     }
 
 #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1 /* { dg-error "expected '\\\)' before end of line" } */
-    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */
+    /* { dg-error "expected integer expression list before" "" { target c++ } .-1 } */
     {
         for (ii = 0; ii < N; ii++)
             b[ii] = a[ii];
@@ -171,7 +171,7 @@ f (int N, float *a, float *b)
 #pragma acc wait (1,2,,) /* { dg-error "expected (primary-|)expression before" } */
 
 #pragma acc wait (1 /* { dg-error "expected '\\\)' before end of line" } */
-    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */
+    /* { dg-error "expected integer expression list before" "" { target c++ } .-1 } */
 
 #pragma acc wait (1,*) /* { dg-error "expected (primary-|)expression before" } */
Julian Brown Nov. 29, 2018, 9:22 p.m. | #4
On Fri, 28 Sep 2018 14:17:42 +0100
Julian Brown <julian@codesourcery.com> wrote:

> On Wed, 26 Sep 2018 14:08:37 -0700

> Cesar Philippidis <cesar@codesourcery.com> wrote:

> 

> > On 09/26/2018 12:50 PM, Joseph Myers wrote:  

> > > On Wed, 26 Sep 2018, Cesar Philippidis wrote:

> > >     

> > >> Attached is an old patch which updated the C and C++ FEs to use

> > >> %<)%> for the right ')' symbol. It's mostly a cosmetic change.

> > >> All of the changes are self-contained to the OpenACC code

> > >> path.    

> > > 

> > > Why is the "before ')'" included in the call to c_parser_error at

> > > all? c_parser_error calls c_parse_error which adds its own "

> > > before " and token description or expansion, so I'd expect the

> > > current error to result in a message ending in something of the

> > > form "before X before Y".    

> 

> > Julian, I need to start working on deep copy in OpenACC. Can you

> > take over this patch? The error handling code in the C FE needs to

> > be removed because it's dead.  

> 

> I agree that the error-handling path in question in the C FE is dead.

> The difference is that in C, c_parser_oacc_wait_list parses the open

> parenthesis, the list and then the close parenthesis separately, and

> so a token sequence like:

> 

>    (1

> 

> will return an expression list of length 1. In the C++ FE rather, a

> cp_parser_parenthesized_expression_list is parsed all in one go, and

> if the input is not that well-formed sequence then NULL is returned

> (or a zero-length vector for an empty list).

> 

> But for C, it does not appear that c_parser_expr_list has a code path

> that can return a zero-length list at all. So, we can elide the

> diagnostic with no change to compiler behaviour. This patch does that,

> and also changes the C++ diagnostic, leading to errors being reported

> like:

> 

> diag.c: In function 'int main(int, char*)':

> diag.c:6:59: error: expected ')' before end of line

> 6 | #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1

>   |                                                         ~ ^

>   |                                                           )

> diag.c:6:59: error: expected integer expression list before end of

> line 

> 

> Actually I'm not too sure how useful the second error line is. Maybe

> we should just remove it to improve consistency between C & C++?

> 

> The attached has been tested with offloading to nvptx and

> bootstrapped. OK?


Ping?

Thanks,

Julian
Joseph Myers Nov. 29, 2018, 9:28 p.m. | #5
On Thu, 29 Nov 2018, Julian Brown wrote:

> > But for C, it does not appear that c_parser_expr_list has a code path

> > that can return a zero-length list at all. So, we can elide the

> > diagnostic with no change to compiler behaviour. This patch does that,

> > and also changes the C++ diagnostic, leading to errors being reported

> > like:


The c-parser.c change is OK.

-- 
Joseph S. Myers
joseph@codesourcery.com
Thomas Schwinge Nov. 30, 2018, 3:25 p.m. | #6
Hi Julian!

On Fri, 28 Sep 2018 14:17:42 +0100, Julian Brown <julian@codesourcery.com> wrote:
> On Wed, 26 Sep 2018 14:08:37 -0700

> Cesar Philippidis <cesar@codesourcery.com> wrote:

> 

> > On 09/26/2018 12:50 PM, Joseph Myers wrote:

> > > On Wed, 26 Sep 2018, Cesar Philippidis wrote:

> > >   

> > >> Attached is an old patch which updated the C and C++ FEs to use

> > >> %<)%> for the right ')' symbol. It's mostly a cosmetic change. All

> > >> of the changes are self-contained to the OpenACC code path.  

> > > 

> > > Why is the "before ')'" included in the call to c_parser_error at

> > > all? c_parser_error calls c_parse_error which adds its own " before

> > > " and token description or expansion, so I'd expect the current

> > > error to result in a message ending in something of the form

> > > "before X before Y".  

> 

> > Julian, I need to start working on deep copy in OpenACC. Can you take

> > over this patch? The error handling code in the C FE needs to be

> > removed because it's dead.

> 

> I agree that the error-handling path in question in the C FE is dead.

> The difference is that in C, c_parser_oacc_wait_list parses the open

> parenthesis, the list and then the close parenthesis separately, and so

> a token sequence like:

> 

>    (1

> 

> will return an expression list of length 1. In the C++ FE rather, a

> cp_parser_parenthesized_expression_list is parsed all in one go, and if

> the input is not that well-formed sequence then NULL is returned (or a

> zero-length vector for an empty list).

> 

> But for C, it does not appear that c_parser_expr_list has a code path

> that can return a zero-length list at all.


In addition to your "(1" token sequence (and similar ones), I suppose
what these code paths in C and C++ are supposed to catch the "wait ()"
case (see line 149 of gcc/testsuite/c-c++-common/goacc/asyncwait-1.c).

I suppose in C, we do diagnose an "error: expected expression before ')'
token" in "c_parser_expr_list"/"c_parser_expr_no_commas", and then return
a list with an "error_mark_node", right?  (I have not verified that.)

> So, we can elide the

> diagnostic with no change to compiler behaviour.


In that case, yes.

> This patch does that,

> and also changes the C++ diagnostic, leading to errors being reported

> like:

> 

> diag.c: In function 'int main(int, char*)':

> diag.c:6:59: error: expected ')' before end of line

> 6 | #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1

>   |                                                         ~ ^

>   |                                                           )

> diag.c:6:59: error: expected integer expression list before end of line 

> 

> Actually I'm not too sure how useful the second error line is. Maybe we

> should just remove it to improve consistency between C & C++?


Right, one single error diagnostic is enough.

But please make sure that the "wait ()" case continues to be diagnosed
correctly -- similarly to C, I suggest "expected expression before ')'
token" (or whatever is natural to the C++ parser), and then accordingly
tidy up that "dg-error" regular expression on line 149 of
gcc/testsuite/c-c++-common/goacc/asyncwait-1.c.

In C++, this is the case that: "args != NULL && args->length () == 0", I
suppose?  (I have not verified that.)


Oh, and next to "wait ()" please also add test coverage for "wait (".


Grüße
 Thomas


> The attached has been tested with offloading to nvptx and bootstrapped.

> OK?

> 

> Thanks,

> 

> Julian

> 

> 2018-XX-YY  James Norris  <jnorris@codesourcery.com>

> 	    Cesar Philippidis  <cesar@codesourcery.com>

>             Julian Brown  <julian@codesourcery.com>

> 

> 	gcc/c/

> 	* c-parser.c (c_parser_oacc_wait_list): Remove dead diagnostic

> 	code.

> 

>         gcc/cp/

>         * parser.c (cp_parser_oacc_wait_list): Change error message.

> 

> 	gcc/testsuite/

>         * c-c++-common/goacc/asyncwait-1: Update expected errors.

> commit 3a59bdbccc3c2383c0056c74797d698c7d81dce2

> Author: Julian Brown <julian@codesourcery.com>

> Date:   Fri Sep 28 05:52:55 2018 -0700

> 

>     OpenACC wait list diagnostic change

> 

> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c

> index 1f173fc..92a8089 100644

> --- a/gcc/c/c-parser.c

> +++ b/gcc/c/c-parser.c

> @@ -11597,14 +11597,6 @@ c_parser_oacc_wait_list (c_parser *parser, location_t clause_loc, tree list)

>      return list;

>  

>    args = c_parser_expr_list (parser, false, true, NULL, NULL, NULL, NULL);

> -

> -  if (args->length () == 0)

> -    {

> -      c_parser_error (parser, "expected integer expression before ')'");

> -      release_tree_vector (args);

> -      return list;

> -    }

> -

>    args_tree = build_tree_list_vec (args);

>  

>    for (t = args_tree; t; t = TREE_CHAIN (t))

> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

> index 6696f17..43128e0 100644

> --- a/gcc/cp/parser.c

> +++ b/gcc/cp/parser.c

> @@ -32086,7 +32086,7 @@ cp_parser_oacc_wait_list (cp_parser *parser, location_t clause_loc, tree list)

>  

>    if (args == NULL || args->length () == 0)

>      {

> -      cp_parser_error (parser, "expected integer expression before ')'");

> +      cp_parser_error (parser, "expected integer expression list");

>        if (args != NULL)

>  	release_tree_vector (args);

>        return list;

> diff --git a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c

> index e1840af..2fc8948 100644

> --- a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c

> +++ b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c

> @@ -116,7 +116,7 @@ f (int N, float *a, float *b)

>      }

>  

>  #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1 /* { dg-error "expected '\\\)' before end of line" } */

> -    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */

> +    /* { dg-error "expected integer expression list before" "" { target c++ } .-1 } */

>      {

>          for (ii = 0; ii < N; ii++)

>              b[ii] = a[ii];

> @@ -171,7 +171,7 @@ f (int N, float *a, float *b)

>  #pragma acc wait (1,2,,) /* { dg-error "expected (primary-|)expression before" } */

>  

>  #pragma acc wait (1 /* { dg-error "expected '\\\)' before end of line" } */

> -    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */

> +    /* { dg-error "expected integer expression list before" "" { target c++ } .-1 } */

>  

>  #pragma acc wait (1,*) /* { dg-error "expected (primary-|)expression before" } */

>
Julian Brown Dec. 3, 2018, 9:10 p.m. | #7
On Fri, 30 Nov 2018 16:25:42 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:

> In addition to your "(1" token sequence (and similar ones), I suppose

> what these code paths in C and C++ are supposed to catch the "wait ()"

> case (see line 149 of gcc/testsuite/c-c++-common/goacc/asyncwait-1.c).

> 

> I suppose in C, we do diagnose an "error: expected expression before

> ')' token" in "c_parser_expr_list"/"c_parser_expr_no_commas", and

> then return a list with an "error_mark_node", right?  (I have not

> verified that.)

> 

> > So, we can elide the

> > diagnostic with no change to compiler behaviour.  

> 

> In that case, yes.


[...]

> Right, one single error diagnostic is enough.

> 

> But please make sure that the "wait ()" case continues to be diagnosed

> correctly -- similarly to C, I suggest "expected expression before ')'

> token" (or whatever is natural to the C++ parser), and then

> accordingly tidy up that "dg-error" regular expression on line 149 of

> gcc/testsuite/c-c++-common/goacc/asyncwait-1.c.

> 

> In C++, this is the case that: "args != NULL && args->length () ==

> 0", I suppose?  (I have not verified that.)

> 

> Oh, and next to "wait ()" please also add test coverage for "wait (".


I've made those changes in the attached, thank you. OK?

Julian

ChangeLog

2018-XX-YY  James Norris  <jnorris@codesourcery.com>
            Cesar Philippidis  <cesar@codesourcery.com>
            Julian Brown  <julian@codesourcery.com>

        gcc/c/
        * c-parser.c (c_parser_oacc_wait_list): Remove dead diagnostic
        code.

        gcc/cp/
        * parser.c (cp_parser_oacc_wait_list): Fix error message and avoid
        duplicate diagnostic.

        gcc/testsuite/
        * c-c++-common/goacc/asyncwait-1: Update expected errors and add a
        test for "wait (".

Reviewed-by: Thomas Schwinge  <thomas@codesourcery.com>

Reviewed-by: Joseph Myers  <joseph@codesourcery.com>
commit e3f9a5935e9ec3062017602a580139a0bccf1f4c
Author: Julian Brown <julian@codesourcery.com>
Date:   Fri Sep 28 05:52:55 2018 -0700

    OpenACC wait list diagnostic change
    
    2018-XX-YY  James Norris  <jnorris@codesourcery.com>
    	    Cesar Philippidis  <cesar@codesourcery.com>
    	    Julian Brown  <julian@codesourcery.com>
    
    	gcc/c/
    	* c-parser.c (c_parser_oacc_wait_list): Remove dead diagnostic
    	code.
    
    	gcc/cp/
    	* parser.c (cp_parser_oacc_wait_list): Fix error message and avoid
    	duplicate diagnostic.
    
    	gcc/testsuite/
    	* c-c++-common/goacc/asyncwait-1: Update expected errors and add a
    	test for "wait (".

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index afc4071..0d7fcc0 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -11801,14 +11801,6 @@ c_parser_oacc_wait_list (c_parser *parser, location_t clause_loc, tree list)
     return list;
 
   args = c_parser_expr_list (parser, false, true, NULL, NULL, NULL, NULL);
-
-  if (args->length () == 0)
-    {
-      c_parser_error (parser, "expected integer expression before ')'");
-      release_tree_vector (args);
-      return list;
-    }
-
   args_tree = build_tree_list_vec (args);
 
   for (t = args_tree; t; t = TREE_CHAIN (t))
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ab6d237..ac19cb4 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -32605,9 +32605,11 @@ cp_parser_oacc_wait_list (cp_parser *parser, location_t clause_loc, tree list)
 
   if (args == NULL || args->length () == 0)
     {
-      cp_parser_error (parser, "expected integer expression before ')'");
       if (args != NULL)
-	release_tree_vector (args);
+	{
+	  cp_parser_error (parser, "expected integer expression list");
+	  release_tree_vector (args);
+	}
       return list;
     }
 
diff --git a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
index e1840af..2f5d476 100644
--- a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
+++ b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
@@ -116,7 +116,6 @@ f (int N, float *a, float *b)
     }
 
 #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1 /* { dg-error "expected '\\\)' before end of line" } */
-    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */
     {
         for (ii = 0; ii < N; ii++)
             b[ii] = a[ii];
@@ -152,6 +151,12 @@ f (int N, float *a, float *b)
             b[ii] = a[ii];
     }
 
+#pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait ( /* { dg-error "expected (primary-|)expression before" } */
+    {
+        for (ii = 0; ii < N; ii++)
+            b[ii] = a[ii];
+    }
+
 #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait
     {
         for (ii = 0; ii < N; ii++)
@@ -171,7 +176,6 @@ f (int N, float *a, float *b)
 #pragma acc wait (1,2,,) /* { dg-error "expected (primary-|)expression before" } */
 
 #pragma acc wait (1 /* { dg-error "expected '\\\)' before end of line" } */
-    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */
 
 #pragma acc wait (1,*) /* { dg-error "expected (primary-|)expression before" } */
Thomas Schwinge Dec. 4, 2018, 11:45 a.m. | #8
Hi Julian!

On Mon, 3 Dec 2018 21:10:50 +0000, Julian Brown <julian@codesourcery.com> wrote:
> On Fri, 30 Nov 2018 16:25:42 +0100

> Thomas Schwinge <thomas@codesourcery.com> wrote:

> > [...]

> 

> I've made those changes in the attached, thank you. OK?


Yes, thanks!


Grüße
 Thomas


> 2018-XX-YY  James Norris  <jnorris@codesourcery.com>

>             Cesar Philippidis  <cesar@codesourcery.com>

>             Julian Brown  <julian@codesourcery.com>

> 

>         gcc/c/

>         * c-parser.c (c_parser_oacc_wait_list): Remove dead diagnostic

>         code.

> 

>         gcc/cp/

>         * parser.c (cp_parser_oacc_wait_list): Fix error message and avoid

>         duplicate diagnostic.

> 

>         gcc/testsuite/

>         * c-c++-common/goacc/asyncwait-1: Update expected errors and add a

>         test for "wait (".

> 

> Reviewed-by: Thomas Schwinge  <thomas@codesourcery.com>

> Reviewed-by: Joseph Myers  <joseph@codesourcery.com>

> commit e3f9a5935e9ec3062017602a580139a0bccf1f4c

> Author: Julian Brown <julian@codesourcery.com>

> Date:   Fri Sep 28 05:52:55 2018 -0700

> 

>     OpenACC wait list diagnostic change

>     

>     2018-XX-YY  James Norris  <jnorris@codesourcery.com>

>     	    Cesar Philippidis  <cesar@codesourcery.com>

>     	    Julian Brown  <julian@codesourcery.com>

>     

>     	gcc/c/

>     	* c-parser.c (c_parser_oacc_wait_list): Remove dead diagnostic

>     	code.

>     

>     	gcc/cp/

>     	* parser.c (cp_parser_oacc_wait_list): Fix error message and avoid

>     	duplicate diagnostic.

>     

>     	gcc/testsuite/

>     	* c-c++-common/goacc/asyncwait-1: Update expected errors and add a

>     	test for "wait (".

> 

> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c

> index afc4071..0d7fcc0 100644

> --- a/gcc/c/c-parser.c

> +++ b/gcc/c/c-parser.c

> @@ -11801,14 +11801,6 @@ c_parser_oacc_wait_list (c_parser *parser, location_t clause_loc, tree list)

>      return list;

>  

>    args = c_parser_expr_list (parser, false, true, NULL, NULL, NULL, NULL);

> -

> -  if (args->length () == 0)

> -    {

> -      c_parser_error (parser, "expected integer expression before ')'");

> -      release_tree_vector (args);

> -      return list;

> -    }

> -

>    args_tree = build_tree_list_vec (args);

>  

>    for (t = args_tree; t; t = TREE_CHAIN (t))

> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

> index ab6d237..ac19cb4 100644

> --- a/gcc/cp/parser.c

> +++ b/gcc/cp/parser.c

> @@ -32605,9 +32605,11 @@ cp_parser_oacc_wait_list (cp_parser *parser, location_t clause_loc, tree list)

>  

>    if (args == NULL || args->length () == 0)

>      {

> -      cp_parser_error (parser, "expected integer expression before ')'");

>        if (args != NULL)

> -	release_tree_vector (args);

> +	{

> +	  cp_parser_error (parser, "expected integer expression list");

> +	  release_tree_vector (args);

> +	}

>        return list;

>      }

>  

> diff --git a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c

> index e1840af..2f5d476 100644

> --- a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c

> +++ b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c

> @@ -116,7 +116,6 @@ f (int N, float *a, float *b)

>      }

>  

>  #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1 /* { dg-error "expected '\\\)' before end of line" } */

> -    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */

>      {

>          for (ii = 0; ii < N; ii++)

>              b[ii] = a[ii];

> @@ -152,6 +151,12 @@ f (int N, float *a, float *b)

>              b[ii] = a[ii];

>      }

>  

> +#pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait ( /* { dg-error "expected (primary-|)expression before" } */

> +    {

> +        for (ii = 0; ii < N; ii++)

> +            b[ii] = a[ii];

> +    }

> +

>  #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait

>      {

>          for (ii = 0; ii < N; ii++)

> @@ -171,7 +176,6 @@ f (int N, float *a, float *b)

>  #pragma acc wait (1,2,,) /* { dg-error "expected (primary-|)expression before" } */

>  

>  #pragma acc wait (1 /* { dg-error "expected '\\\)' before end of line" } */

> -    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */

>  

>  #pragma acc wait (1,*) /* { dg-error "expected (primary-|)expression before" } */

>

Patch

[OpenACC] C, C++ OpenACC wait diagnostic change

2018-XX-YY  James Norris  <jnorris@codesourcery.com>
	    Cesar Philippidis  <cesar@codesourcery.com>

	gcc/c/
	* c-parser.c (c_parser_oacc_wait_list): Change error message.
	gcc/cp/
        * parser.c (cp_parser_oacc_wait_list): Change error message.
	gcc/testsuite/
        * c-c++-common/goacc/asyncwait-1: Update messages.

(cherry picked from gomp-4_0-branch r223007, e4ea0a3)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 1766a256633..b8fc000b50d 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -11597,7 +11597,8 @@  c_parser_oacc_wait_list (c_parser *parser, location_t clause_loc, tree list)
 
   if (args->length () == 0)
     {
-      c_parser_error (parser, "expected integer expression before ')'");
+      c_parser_error (parser,
+		      "expected integer expression list before %<)%>");
       release_tree_vector (args);
       return list;
     }
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index c6ebc494e59..e80c1fba670 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -32094,7 +32094,8 @@  cp_parser_oacc_wait_list (cp_parser *parser, location_t clause_loc, tree list)
 
   if (args == NULL || args->length () == 0)
     {
-      cp_parser_error (parser, "expected integer expression before ')'");
+      cp_parser_error (parser,
+		       "expected integer expression list before %<)%>");
       if (args != NULL)
 	release_tree_vector (args);
       return list;
diff --git a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
index e1840af5d70..2fc89486ee5 100644
--- a/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
+++ b/gcc/testsuite/c-c++-common/goacc/asyncwait-1.c
@@ -116,7 +116,7 @@  f (int N, float *a, float *b)
     }
 
 #pragma acc parallel copyin (a[0:N]) copy (b[0:N]) wait (1 /* { dg-error "expected '\\\)' before end of line" } */
-    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */
+    /* { dg-error "expected integer expression list before" "" { target c++ } .-1 } */
     {
         for (ii = 0; ii < N; ii++)
             b[ii] = a[ii];
@@ -171,7 +171,7 @@  f (int N, float *a, float *b)
 #pragma acc wait (1,2,,) /* { dg-error "expected (primary-|)expression before" } */
 
 #pragma acc wait (1 /* { dg-error "expected '\\\)' before end of line" } */
-    /* { dg-error "expected integer expression before '\\\)'" "" { target c++ } .-1 } */
+    /* { dg-error "expected integer expression list before" "" { target c++ } .-1 } */
 
 #pragma acc wait (1,*) /* { dg-error "expected (primary-|)expression before" } */
 
-- 
2.17.1