Hi! On Thu, 4 Aug 2016 11:03:25 -0600, Jeff Law <l...@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 <tho...@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 <clt...@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 <tho...@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 <ja...@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:
--
2.17.1
signature.asc
Description: PGP signature
