Use correct location information for OpenACC shape and simple clauses in C/C++

Hi!

I found that for a lot of OpenACC (and potentially also OpenMP) clauses
(in C/C++ front ends; didn't look at Fortran), we use wrong location
information.  The problem is that
c_parser_oacc_all_clauses/c_parser_omp_all_clauses calls
cp_parser_omp_clause_name to determine the pragma_omp_clause c_kind, and
that function (as documented) consumes the clause token before returning.
So, when we then do "c_parser_peek_token (parser)->location" or similar
in some clause parsing function, that will return the location
information of the token _after_ the clause token, which -- at least very
often -- is not desirable, in particular if that location information is
used then in a build_omp_clause call, which should point to the clause
token itself, and not whatever follows after that.

Probably that all went unnoticed for so long, because the GCC testsuite
largely is running with -fno-diagnostics-show-caret, so we don't visually
see the wrong location information (and nobody pays attention to the
colum information as given, for example, as line 2, column 32 in
"[...]/c-c++-common/goacc/routine-2.c:2:32: error: [...]".

There seems to be a lot of inconsistency in that in all the clause
parsing; here is just a first patch to fix the immediate problem I've
been observing.  OK for trunk already, or need to clean this all up in
one go?  Do we need this on release branches, as a "quality of
implementation" fix (wrong diagnostic locations)?

commit bac4c04ca1d52c56a3583f5958e116c62b889d5a
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Wed Jul 27 16:55:56 2016 +0200

    Use correct location information for OpenACC shape and simple clauses in C/C++
    
    	gcc/c/
    	* c-parser.c (c_parser_oacc_shape_clause)
    	(c_parser_oacc_simple_clause): Add loc formal parameter.  Adjust
    	all users.
    	gcc/cp/
    	* parser.c (cp_parser_oacc_shape_clause): Add loc formal
    	parameter.  Adjust all users.
---
 gcc/c/c-parser.c | 25 +++++++++++++------------
 gcc/cp/parser.c  | 12 +++++++-----
 2 files changed, 20 insertions(+), 17 deletions(-)



Grüße
 Thomas

On Wed, 2016-07-27 at 17:17 +0200, Thomas Schwinge wrote:
> Hi!
> 
> I found that for a lot of OpenACC (and potentially also OpenMP)
> clauses
> (in C/C++ front ends; didn't look at Fortran), we use wrong location
> information.  The problem is that
> c_parser_oacc_all_clauses/c_parser_omp_all_clauses calls
> cp_parser_omp_clause_name to determine the pragma_omp_clause c_kind,
> and
> that function (as documented) consumes the clause token before
> returning.
> So, when we then do "c_parser_peek_token (parser)->location" or
> similar
> in some clause parsing function, that will return the location
> information of the token _after_ the clause token, which -- at least
> very
> often -- is not desirable, in particular if that location information
> is
> used then in a build_omp_clause call, which should point to the
> clause
> token itself, and not whatever follows after that.
> 
> Probably that all went unnoticed for so long, because the GCC
> testsuite
> largely is running with -fno-diagnostics-show-caret, so we don't
> visually
> see the wrong location information (and nobody pays attention to the
> colum information as given, for example, as line 2, column 32 in
> "[...]/c-c++-common/goacc/routine-2.c:2:32: error: [...]".

> There seems to be a lot of inconsistency in that in all the clause
> parsing; here is just a first patch to fix the immediate problem I've
> been observing.  OK for trunk already, or need to clean this all up
> in
> one go?  Do we need this on release branches, as a "quality of
> implementation" fix (wrong diagnostic locations)?

[...]

I'm not a reviewer for the C/C++ FEs so I can't really review this
patch, but it might be nice in this (or in a followup) to add some test
cases for this that explicitly test the caret information for some of
these errors.

Put this at the top of a new test file:
/* { dg-options "-fdiagnostics-show-caret" } */
to override the default -fno-diagnostics-show-caret, and use this to
quote what we expect to see after each dg-error:

 /* { dg-begin-multiline-output "" }
EXPECTED CARET TEXT GOES HERE, with trailing dg- stuff trimmed off
    { dg-end-multiline-output "" } */


Hope this is constructive
Dave

Hi!

On Wed, 27 Jul 2016 17:09:38 -0400, David Malcolm <dmalcolm@redhat.com> wrote:
> On Wed, 2016-07-27 at 17:17 +0200, Thomas Schwinge wrote:
> > I found that for a lot of OpenACC (and potentially also OpenMP)
> > clauses
> > (in C/C++ front ends; didn't look at Fortran), we use wrong location
> > information.  The problem is that
> > c_parser_oacc_all_clauses/c_parser_omp_all_clauses calls
> > cp_parser_omp_clause_name to determine the pragma_omp_clause c_kind,
> > and
> > that function (as documented) consumes the clause token before
> > returning.
> > So, when we then do "c_parser_peek_token (parser)->location" or
> > similar
> > in some clause parsing function, that will return the location
> > information of the token _after_ the clause token, which -- at least
> > very
> > often -- is not desirable, in particular if that location information
> > is
> > used then in a build_omp_clause call, which should point to the
> > clause
> > token itself, and not whatever follows after that.
> > 
> > Probably that all went unnoticed for so long, because the GCC
> > testsuite
> > largely is running with -fno-diagnostics-show-caret, so we don't
> > visually
> > see the wrong location information (and nobody pays attention to the
> > colum information as given, for example, as line 2, column 32 in
> > "[...]/c-c++-common/goacc/routine-2.c:2:32: error: [...]".
> 
> > There seems to be a lot of inconsistency in that in all the clause
> > parsing; here is just a first patch to fix the immediate problem I've
> > been observing.  OK for trunk already, or need to clean this all up
> > in
> > one go?  Do we need this on release branches, as a "quality of
> > implementation" fix (wrong diagnostic locations)?

> > [initial patch]

Ping for that one.


> I'm not a reviewer for the C/C++ FEs so I can't really review this
> patch

I think in your position as a maintainer for "diagnostic messages", you
should be qualified to exercise that status to approve such a patch.  :-)


> but it might be nice in this (or in a followup) to add some test
> cases for this that explicitly test the caret information for some of
> these errors.  [...]

> Hope this is constructive

It certainly is, thanks!  In fact, I had planned to look up how to do
that, which you've now made simpler by providing a specific receipe.


Grüße
 Thomas

On Thu, 2016-08-04 at 16:54 +0200, Thomas Schwinge wrote:
> Hi!
> 
> On Wed, 27 Jul 2016 17:09:38 -0400, David Malcolm <
> dmalcolm@redhat.com> wrote:
> > On Wed, 2016-07-27 at 17:17 +0200, Thomas Schwinge wrote:
> > > I found that for a lot of OpenACC (and potentially also OpenMP)
> > > clauses
> > > (in C/C++ front ends; didn't look at Fortran), we use wrong
> > > location
> > > information.  The problem is that
> > > c_parser_oacc_all_clauses/c_parser_omp_all_clauses calls
> > > cp_parser_omp_clause_name to determine the pragma_omp_clause
> > > c_kind,
> > > and
> > > that function (as documented) consumes the clause token before
> > > returning.
> > > So, when we then do "c_parser_peek_token (parser)->location" or
> > > similar
> > > in some clause parsing function, that will return the location
> > > information of the token _after_ the clause token, which -- at
> > > least
> > > very
> > > often -- is not desirable, in particular if that location
> > > information
> > > is
> > > used then in a build_omp_clause call, which should point to the
> > > clause
> > > token itself, and not whatever follows after that.
> > > 
> > > Probably that all went unnoticed for so long, because the GCC
> > > testsuite
> > > largely is running with -fno-diagnostics-show-caret, so we don't
> > > visually
> > > see the wrong location information (and nobody pays attention to
> > > the
> > > colum information as given, for example, as line 2, column 32 in
> > > "[...]/c-c++-common/goacc/routine-2.c:2:32: error: [...]".
> > 
> > > There seems to be a lot of inconsistency in that in all the
> > > clause
> > > parsing; here is just a first patch to fix the immediate problem
> > > I've
> > > been observing.  OK for trunk already, or need to clean this all
> > > up
> > > in
> > > one go?  Do we need this on release branches, as a "quality of
> > > implementation" fix (wrong diagnostic locations)?
> 
> > > [initial patch]
> 
> Ping for that one.
> 
> 
> > I'm not a reviewer for the C/C++ FEs so I can't really review this
> > patch
> 
> I think in your position as a maintainer for "diagnostic messages",
> you
> should be qualified to exercise that status to approve such a patch. 
>  :-)

I don't know exactly where the boundaries of that role are; I've been
assuming it means anything relating to the diagnostic subsystem itself
(and location-tracking), as opposed to *usage* of the system.  The
patch in question is more about the latter.  That said, your patch
looks very reasonable to me (but as I mentioned, a test case
demonstrating the improved caret locations would be good).

Steering committee: am I being too conservative in my interpretation of
that role?

> 
> > but it might be nice in this (or in a followup) to add some test
> > cases for this that explicitly test the caret information for some
> > of
> > these errors.  [...]
> 
> > Hope this is constructive
> 
> It certainly is, thanks!  In fact, I had planned to look up how to do
> that, which you've now made simpler by providing a specific receipe.
> 
> 
> Grüße
>  Thomas

On 07/27/2016 09:17 AM, Thomas Schwinge wrote:
> Hi!
>
> I found that for a lot of OpenACC (and potentially also OpenMP) clauses
> (in C/C++ front ends; didn't look at Fortran), we use wrong location
> information.  The problem is that
> c_parser_oacc_all_clauses/c_parser_omp_all_clauses calls
> cp_parser_omp_clause_name to determine the pragma_omp_clause c_kind, and
> that function (as documented) consumes the clause token before returning.
> So, when we then do "c_parser_peek_token (parser)->location" or similar
> in some clause parsing function, that will return the location
> information of the token _after_ the clause token, which -- at least very
> often -- is not desirable, in particular if that location information is
> used then in a build_omp_clause call, which should point to the clause
> token itself, and not whatever follows after that.
>
> Probably that all went unnoticed for so long, because the GCC testsuite
> largely is running with -fno-diagnostics-show-caret, so we don't visually
> see the wrong location information (and nobody pays attention to the
> colum information as given, for example, as line 2, column 32 in
> "[...]/c-c++-common/goacc/routine-2.c:2:32: error: [...]".
>
> There seems to be a lot of inconsistency in that in all the clause
> parsing; here is just a first patch to fix the immediate problem I've
> been observing.  OK for trunk already, or need to clean this all up in
> one go?  Do we need this on release branches, as a "quality of
> implementation" fix (wrong diagnostic locations)?
>
> commit bac4c04ca1d52c56a3583f5958e116c62b889d5a
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date:   Wed Jul 27 16:55:56 2016 +0200
>
>     Use correct location information for OpenACC shape and simple clauses in C/C++
>
>     	gcc/c/
>     	* c-parser.c (c_parser_oacc_shape_clause)
>     	(c_parser_oacc_simple_clause): Add loc formal parameter.  Adjust
>     	all users.
>     	gcc/cp/
>     	* parser.c (cp_parser_oacc_shape_clause): Add loc formal
>     	parameter.  Adjust all users.
> ---
>  gcc/c/c-parser.c | 25 +++++++++++++------------
>  gcc/cp/parser.c  | 12 +++++++-----
>  2 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index 0031481..82ac855 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -11758,12 +11758,12 @@ c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind,
>     seq */
>
>  static tree
> -c_parser_oacc_simple_clause (c_parser *parser, enum omp_clause_code code,
> -			     tree list)
> +c_parser_oacc_simple_clause (c_parser * /* parser */, location_t loc,
> +			     enum omp_clause_code code, tree list)
Any reason not to just drop the parser argument entirely?  If we must 
have it to match an API, but don't need it, then just drop the argument 
name entirely rather than commenting it out.  This kind of comment, IMHO 
serves no useful purpose.

With that change and some tests (presumably using David recipe) this is 
will be fine.

jeff

On Thu, 4 Aug 2016, David Malcolm wrote:
> On Thu, 2016-08-04 at 16:54 +0200, Thomas Schwinge wrote:
>> I think in your position as a maintainer for "diagnostic messages", 
>> you should be qualified to exercise that status to approve such a 
>> patch. :-)
> I don't know exactly where the boundaries of that role are; I've been
> assuming it means anything relating to the diagnostic subsystem itself
> (and location-tracking), as opposed to *usage* of the system.  The
> patch in question is more about the latter.  That said, your patch
> looks very reasonable to me (but as I mentioned, a test case
> demonstrating the improved caret locations would be good).
> 
> Steering committee: am I being too conservative in my interpretation of
> that role?

Somehow your question seems to have been missed?  Sorry about that.

For the sake of full disclosure, on the steering committee we
usually do not dive super deeply into what a maintainer's domain
entails and what not.  

What I generally see is that being a bit liberal, especially when 
you know that adjacent area and are confident, tends to work for 
everyone.  (It's not that we have excessive review capacity.)

Gerald

Hi!

On Thu, 4 Aug 2016 11:03:25 -0600, Jeff Law <law@redhat.com> wrote:
> On 07/27/2016 09:17 AM, Thomas Schwinge wrote:
> > I found that for a lot of OpenACC (and potentially also OpenMP) clauses
> > (in C/C++ front ends; didn't look at Fortran), we use wrong location
> > information.  The problem is that
> > c_parser_oacc_all_clauses/c_parser_omp_all_clauses calls
> > cp_parser_omp_clause_name to determine the pragma_omp_clause c_kind, and
> > that function (as documented) consumes the clause token before returning.
> > So, when we then do "c_parser_peek_token (parser)->location" or similar
> > in some clause parsing function, that will return the location
> > information of the token _after_ the clause token, which -- at least very
> > often -- is not desirable, in particular if that location information is
> > used then in a build_omp_clause call, which should point to the clause
> > token itself, and not whatever follows after that.
> >
> > Probably that all went unnoticed for so long, because the GCC testsuite
> > largely is running with -fno-diagnostics-show-caret, so we don't visually
> > see the wrong location information (and nobody pays attention to the
> > colum information as given, for example, as line 2, column 32 in
> > "[...]/c-c++-common/goacc/routine-2.c:2:32: error: [...]".
> >
> > There seems to be a lot of inconsistency in that in all the clause
> > parsing; here is just a first patch to fix the immediate problem I've
> > been observing.  OK for trunk already, or need to clean this all up in
> > one go?  Do we need this on release branches, as a "quality of
> > implementation" fix (wrong diagnostic locations)?

> > --- gcc/c/c-parser.c
> > +++ gcc/c/c-parser.c
> > @@ -11758,12 +11758,12 @@ c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind,
> >     seq */
> >
> >  static tree
> > -c_parser_oacc_simple_clause (c_parser *parser, enum omp_clause_code code,
> > -			     tree list)
> > +c_parser_oacc_simple_clause (c_parser * /* parser */, location_t loc,
> > +			     enum omp_clause_code code, tree list)

> Any reason not to just drop the parser argument entirely?  If we must 
> have it to match an API, but don't need it, then just drop the argument 
> name entirely rather than commenting it out.  This kind of comment, IMHO 
> serves no useful purpose.

OK.  I had only left it in, as all the parser functions passed it in, but
yeah, that's not actually necessary (and can easily be restored should it
ever be needed).

> With that change and some tests (presumably using David recipe) this is 
> will be fine.

With test cases still deferred for proper inspection of all such clauses,
I have now at least committed the agreed-on code changes to fix things
for the cases already identified: committed to trunk in r269102 "[C, C++]
Use correct location information for OpenACC shape and simple clauses",
as attached.


Grüße
 Thomas
From 51391c1d37a8111492a5c5ea2e17654c4fa29d03 Mon Sep 17 00:00:00 2001
From: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Fri, 22 Feb 2019 10:49:43 +0000
Subject: [PATCH 1/9] [C, C++] Use correct location information for OpenACC
 shape and simple clauses

	gcc/c/
	* c-parser.c (c_parser_oacc_shape_clause): Add loc formal
	parameter.  Adjust all users.
	(c_parser_oacc_simple_clause): Replace parser with loc formal
	parameter.  Adjust all users.
	gcc/cp/
	* parser.c (cp_parser_oacc_simple_clause): Remove parser formal
	parameter, move loc formal parameter to the front.  Adjust all
	users.
	(cp_parser_oacc_shape_clause): Add loc formal parameter.  Adjust
	all users.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@269102 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/c/ChangeLog  |  7 +++++++
 gcc/c/c-parser.c | 28 ++++++++++++++--------------
 gcc/cp/ChangeLog |  8 ++++++++
 gcc/cp/parser.c  | 45 +++++++++++++++++++++++----------------------
 4 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index b76f5b1fae6..6b26ca0f9b3 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-22  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* c-parser.c (c_parser_oacc_shape_clause): Add loc formal
+	parameter.  Adjust all users.
+	(c_parser_oacc_simple_clause): Replace parser with loc formal
+	parameter.  Adjust all users.
+
 2019-02-19  Chung-Lin Tang <cltang@codesourcery.com>
 
 	PR c/87924
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 6c1f3076241..22c7416ac94 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -13159,12 +13159,12 @@ c_parser_oacc_single_int_clause (c_parser *parser, omp_clause_code code,
 */
 
 static tree
-c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind,
+c_parser_oacc_shape_clause (c_parser *parser, location_t loc,
+			    omp_clause_code kind,
 			    const char *str, tree list)
 {
   const char *id = "num";
   tree ops[2] = { NULL_TREE, NULL_TREE }, c;
-  location_t loc = c_parser_peek_token (parser)->location;
 
   if (kind == OMP_CLAUSE_VECTOR)
     id = "length";
@@ -13296,12 +13296,12 @@ c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind,
    seq */
 
 static tree
-c_parser_oacc_simple_clause (c_parser *parser, enum omp_clause_code code,
+c_parser_oacc_simple_clause (location_t loc, enum omp_clause_code code,
 			     tree list)
 {
   check_no_duplicate_clause (list, code, omp_clause_code_name[code]);
 
-  tree c = build_omp_clause (c_parser_peek_token (parser)->location, code);
+  tree c = build_omp_clause (loc, code);
   OMP_CLAUSE_CHAIN (c) = list;
 
   return c;
@@ -14807,8 +14807,8 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  c_name = "async";
 	  break;
 	case PRAGMA_OACC_CLAUSE_AUTO:
-	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_AUTO,
-						clauses);
+	  clauses = c_parser_oacc_simple_clause (here, OMP_CLAUSE_AUTO,
+						 clauses);
 	  c_name = "auto";
 	  break;
 	case PRAGMA_OACC_CLAUSE_COLLAPSE:
@@ -14852,7 +14852,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  c_name = "device_resident";
 	  break;
 	case PRAGMA_OACC_CLAUSE_FINALIZE:
-	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_FINALIZE,
+	  clauses = c_parser_oacc_simple_clause (here, OMP_CLAUSE_FINALIZE,
 						 clauses);
 	  c_name = "finalize";
 	  break;
@@ -14862,7 +14862,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_GANG:
 	  c_name = "gang";
-	  clauses = c_parser_oacc_shape_clause (parser, OMP_CLAUSE_GANG,
+	  clauses = c_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_GANG,
 						c_name, clauses);
 	  break;
 	case PRAGMA_OACC_CLAUSE_HOST:
@@ -14874,13 +14874,13 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  c_name = "if";
 	  break;
 	case PRAGMA_OACC_CLAUSE_IF_PRESENT:
-	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_IF_PRESENT,
+	  clauses = c_parser_oacc_simple_clause (here, OMP_CLAUSE_IF_PRESENT,
 						 clauses);
 	  c_name = "if_present";
 	  break;
 	case PRAGMA_OACC_CLAUSE_INDEPENDENT:
-	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_INDEPENDENT,
-						clauses);
+	  clauses = c_parser_oacc_simple_clause (here, OMP_CLAUSE_INDEPENDENT,
+						 clauses);
 	  c_name = "independent";
 	  break;
 	case PRAGMA_OACC_CLAUSE_LINK:
@@ -14914,7 +14914,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  c_name = "reduction";
 	  break;
 	case PRAGMA_OACC_CLAUSE_SEQ:
-	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_SEQ,
+	  clauses = c_parser_oacc_simple_clause (here, OMP_CLAUSE_SEQ,
 						 clauses);
 	  c_name = "seq";
 	  break;
@@ -14928,7 +14928,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_VECTOR:
 	  c_name = "vector";
-	  clauses = c_parser_oacc_shape_clause (parser, OMP_CLAUSE_VECTOR,
+	  clauses = c_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_VECTOR,
 						c_name,	clauses);
 	  break;
 	case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH:
@@ -14943,7 +14943,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_WORKER:
 	  c_name = "worker";
-	  clauses = c_parser_oacc_shape_clause (parser, OMP_CLAUSE_WORKER,
+	  clauses = c_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_WORKER,
 						c_name, clauses);
 	  break;
 	default:
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index e7780e7eb12..0858646d19e 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2019-02-22  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* parser.c (cp_parser_oacc_simple_clause): Remove parser formal
+	parameter, move loc formal parameter to the front.  Adjust all
+	users.
+	(cp_parser_oacc_shape_clause): Add loc formal parameter.  Adjust
+	all users.
+
 2019-02-21  Jason Merrill  <jason@redhat.com>
 
 	PR c++/87685 - generic lambda 'this' capture error.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index f8d44e06ec3..545047c91af 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -32595,13 +32595,14 @@ cp_parser_oacc_data_clause_deviceptr (cp_parser *parser, tree list)
    seq */
 
 static tree
-cp_parser_oacc_simple_clause (cp_parser * /* parser  */,
-			      enum omp_clause_code code,
-			      tree list, location_t location)
+cp_parser_oacc_simple_clause (location_t loc, enum omp_clause_code code,
+			      tree list)
 {
-  check_no_duplicate_clause (list, code, omp_clause_code_name[code], location);
-  tree c = build_omp_clause (location, code);
+  check_no_duplicate_clause (list, code, omp_clause_code_name[code], loc);
+
+  tree c = build_omp_clause (loc, code);
   OMP_CLAUSE_CHAIN (c) = list;
+
   return c;
 }
 
@@ -32657,13 +32658,13 @@ cp_parser_oacc_single_int_clause (cp_parser *parser, omp_clause_code code,
 */
 
 static tree
-cp_parser_oacc_shape_clause (cp_parser *parser, omp_clause_code kind,
+cp_parser_oacc_shape_clause (cp_parser *parser, location_t loc,
+			     omp_clause_code kind,
 			     const char *str, tree list)
 {
   const char *id = "num";
   cp_lexer *lexer = parser->lexer;
   tree ops[2] = { NULL_TREE, NULL_TREE }, c;
-  location_t loc = cp_lexer_peek_token (lexer)->location;
 
   if (kind == OMP_CLAUSE_VECTOR)
     id = "length";
@@ -34884,8 +34885,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  c_name = "async";
 	  break;
 	case PRAGMA_OACC_CLAUSE_AUTO:
-	  clauses = cp_parser_oacc_simple_clause (parser, OMP_CLAUSE_AUTO,
-						 clauses, here);
+	  clauses = cp_parser_oacc_simple_clause (here, OMP_CLAUSE_AUTO,
+						  clauses);
 	  c_name = "auto";
 	  break;
 	case PRAGMA_OACC_CLAUSE_COLLAPSE:
@@ -34929,8 +34930,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  c_name = "device_resident";
 	  break;
 	case PRAGMA_OACC_CLAUSE_FINALIZE:
-	  clauses = cp_parser_oacc_simple_clause (parser, OMP_CLAUSE_FINALIZE,
-						  clauses, here);
+	  clauses = cp_parser_oacc_simple_clause (here, OMP_CLAUSE_FINALIZE,
+						  clauses);
 	  c_name = "finalize";
 	  break;
 	case PRAGMA_OACC_CLAUSE_FIRSTPRIVATE:
@@ -34940,7 +34941,7 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_GANG:
 	  c_name = "gang";
-	  clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_GANG,
+	  clauses = cp_parser_oacc_shape_clause (parser, here, OMP_CLAUSE_GANG,
 						 c_name, clauses);
 	  break;
 	case PRAGMA_OACC_CLAUSE_HOST:
@@ -34952,15 +34953,13 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  c_name = "if";
 	  break;
 	case PRAGMA_OACC_CLAUSE_IF_PRESENT:
-	  clauses = cp_parser_oacc_simple_clause (parser,
-						  OMP_CLAUSE_IF_PRESENT,
-						  clauses, here);
+	  clauses = cp_parser_oacc_simple_clause (here, OMP_CLAUSE_IF_PRESENT,
+						  clauses);
 	  c_name = "if_present";
 	  break;
 	case PRAGMA_OACC_CLAUSE_INDEPENDENT:
-	  clauses = cp_parser_oacc_simple_clause (parser,
-						  OMP_CLAUSE_INDEPENDENT,
-						  clauses, here);
+	  clauses = cp_parser_oacc_simple_clause (here, OMP_CLAUSE_INDEPENDENT,
+						  clauses);
 	  c_name = "independent";
 	  break;
 	case PRAGMA_OACC_CLAUSE_LINK:
@@ -34995,8 +34994,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  c_name = "reduction";
 	  break;
 	case PRAGMA_OACC_CLAUSE_SEQ:
-	  clauses = cp_parser_oacc_simple_clause (parser, OMP_CLAUSE_SEQ,
-						 clauses, here);
+	  clauses = cp_parser_oacc_simple_clause (here, OMP_CLAUSE_SEQ,
+						  clauses);
 	  c_name = "seq";
 	  break;
 	case PRAGMA_OACC_CLAUSE_TILE:
@@ -35010,7 +35009,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_VECTOR:
 	  c_name = "vector";
-	  clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_VECTOR,
+	  clauses = cp_parser_oacc_shape_clause (parser, here,
+						 OMP_CLAUSE_VECTOR,
 						 c_name, clauses);
 	  break;
 	case PRAGMA_OACC_CLAUSE_VECTOR_LENGTH:
@@ -35025,7 +35025,8 @@ cp_parser_oacc_all_clauses (cp_parser *parser, omp_clause_mask mask,
 	  break;
 	case PRAGMA_OACC_CLAUSE_WORKER:
 	  c_name = "worker";
-	  clauses = cp_parser_oacc_shape_clause (parser, OMP_CLAUSE_WORKER,
+	  clauses = cp_parser_oacc_shape_clause (parser, here,
+						 OMP_CLAUSE_WORKER,
 						 c_name, clauses);
 	  break;
 	default: