On Wed, 26 Sep 2018 14:08:37 -0700
Cesar Philippidis <ce...@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  <jnor...@codesourcery.com>
            Cesar Philippidis  <ce...@codesourcery.com>
            Julian Brown  <jul...@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 <jul...@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" } */
 

Reply via email to