This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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(-)

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
@@ -11623,12 +11623,12 @@ c_parser_omp_clause_num_workers (c_parser *parser, tree list)
 */
 
 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";
@@ -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)
 {
   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;
@@ -13089,7 +13089,7 @@ 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 = c_parser_oacc_simple_clause (parser, here, OMP_CLAUSE_AUTO,
 						clauses);
 	  c_name = "auto";
 	  break;
@@ -13139,7 +13139,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:
@@ -13151,8 +13151,9 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  c_name = "if";
 	  break;
 	case PRAGMA_OACC_CLAUSE_INDEPENDENT:
-	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_INDEPENDENT,
-						clauses);
+	  clauses = c_parser_oacc_simple_clause (parser, here,
+						 OMP_CLAUSE_INDEPENDENT,
+						 clauses);
 	  c_name = "independent";
 	  break;
 	case PRAGMA_OACC_CLAUSE_LINK:
@@ -13200,7 +13201,7 @@ c_parser_oacc_all_clauses (c_parser *parser, omp_clause_mask mask,
 	  c_name = "self";
 	  break;
 	case PRAGMA_OACC_CLAUSE_SEQ:
-	  clauses = c_parser_oacc_simple_clause (parser, OMP_CLAUSE_SEQ,
+	  clauses = c_parser_oacc_simple_clause (parser, here, OMP_CLAUSE_SEQ,
 						clauses);
 	  c_name = "seq";
 	  break;
@@ -13214,7 +13215,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:
@@ -13227,7 +13228,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 gcc/cp/parser.c gcc/cp/parser.c
index 2797ec4..b78c3de 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -30372,13 +30372,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";
@@ -32248,7 +32248,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:
@@ -32330,7 +32330,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:
@@ -32345,7 +32346,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:


Grüße
 Thomas


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]