* [patch,openacc] C, C++ OpenACC wait diagnostic change
@ 2018-09-26 18:21 Cesar Philippidis
2018-09-26 20:28 ` Joseph Myers
0 siblings, 1 reply; 9+ messages in thread
From: Cesar Philippidis @ 2018-09-26 18:21 UTC (permalink / raw)
To: gcc-patches, Thomas Schwinge
[-- Attachment #1: Type: text/plain, Size: 301 bytes --]
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
[-- Attachment #2: 0001-C-C-OpenACC-wait-diagnostic-change.patch --]
[-- Type: text/x-patch, Size: 2626 bytes --]
[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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-09-26 18:21 [patch,openacc] C, C++ OpenACC wait diagnostic change Cesar Philippidis
@ 2018-09-26 20:28 ` Joseph Myers
2018-09-26 21:14 ` Cesar Philippidis
0 siblings, 1 reply; 9+ messages in thread
From: Joseph Myers @ 2018-09-26 20:28 UTC (permalink / raw)
To: Cesar Philippidis; +Cc: gcc-patches, Thomas Schwinge
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-09-26 20:28 ` Joseph Myers
@ 2018-09-26 21:14 ` Cesar Philippidis
2018-09-28 13:33 ` Julian Brown
0 siblings, 1 reply; 9+ messages in thread
From: Cesar Philippidis @ 2018-09-26 21:14 UTC (permalink / raw)
To: Joseph Myers, Julian Brown; +Cc: gcc-patches, Thomas Schwinge
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-09-26 21:14 ` Cesar Philippidis
@ 2018-09-28 13:33 ` Julian Brown
2018-11-29 21:22 ` Julian Brown
2018-11-30 15:25 ` Thomas Schwinge
0 siblings, 2 replies; 9+ messages in thread
From: Julian Brown @ 2018-09-28 13:33 UTC (permalink / raw)
To: Cesar Philippidis; +Cc: Joseph Myers, gcc-patches, Thomas Schwinge
[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]
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.
[-- Attachment #2: diagnostic-change-2.diff --]
[-- Type: text/x-patch, Size: 2411 bytes --]
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" } */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-09-28 13:33 ` Julian Brown
@ 2018-11-29 21:22 ` Julian Brown
2018-11-29 21:28 ` Joseph Myers
2018-11-30 15:25 ` Thomas Schwinge
1 sibling, 1 reply; 9+ messages in thread
From: Julian Brown @ 2018-11-29 21:22 UTC (permalink / raw)
To: Cesar Philippidis; +Cc: Joseph Myers, gcc-patches, Thomas Schwinge
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-11-29 21:22 ` Julian Brown
@ 2018-11-29 21:28 ` Joseph Myers
0 siblings, 0 replies; 9+ messages in thread
From: Joseph Myers @ 2018-11-29 21:28 UTC (permalink / raw)
To: Julian Brown; +Cc: Cesar Philippidis, gcc-patches, Thomas Schwinge
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-09-28 13:33 ` Julian Brown
2018-11-29 21:22 ` Julian Brown
@ 2018-11-30 15:25 ` Thomas Schwinge
2018-12-03 21:11 ` Julian Brown
1 sibling, 1 reply; 9+ messages in thread
From: Thomas Schwinge @ 2018-11-30 15:25 UTC (permalink / raw)
To: Julian Brown; +Cc: Joseph Myers, gcc-patches
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" } */
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-11-30 15:25 ` Thomas Schwinge
@ 2018-12-03 21:11 ` Julian Brown
2018-12-04 11:46 ` Thomas Schwinge
0 siblings, 1 reply; 9+ messages in thread
From: Julian Brown @ 2018-12-03 21:11 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: Joseph Myers, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]
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>
[-- Attachment #2: wait-list-diagnostic-change-2.diff --]
[-- Type: text/x-patch, Size: 3166 bytes --]
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" } */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch,openacc] C, C++ OpenACC wait diagnostic change
2018-12-03 21:11 ` Julian Brown
@ 2018-12-04 11:46 ` Thomas Schwinge
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Schwinge @ 2018-12-04 11:46 UTC (permalink / raw)
To: Julian Brown; +Cc: Joseph Myers, gcc-patches
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" } */
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-12-04 11:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 18:21 [patch,openacc] C, C++ OpenACC wait diagnostic change Cesar Philippidis
2018-09-26 20:28 ` Joseph Myers
2018-09-26 21:14 ` Cesar Philippidis
2018-09-28 13:33 ` Julian Brown
2018-11-29 21:22 ` Julian Brown
2018-11-29 21:28 ` Joseph Myers
2018-11-30 15:25 ` Thomas Schwinge
2018-12-03 21:11 ` Julian Brown
2018-12-04 11:46 ` Thomas Schwinge
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).