Fix column information for omp_clauses in Fortran code

Message ID 0624ce85-2ee7-38eb-d115-509d600d3f20@codesourcery.com
State New
Headers show
Series
  • Fix column information for omp_clauses in Fortran code
Related show

Commit Message

Frederik Harwath Dec. 9, 2019, 3:58 p.m.
Hi,
Tobias has recently fixed a problem with the column information in gfortran locations
("PR 92793 - fix column used for error diagnostic"). Diagnostic messages for OpenMP/OpenACC
clauses do not contain the right column information yet. The reason is that the location
information of the first clause is used for all clauses on a line and hence the columns
are wrong for all but the first clause. The attached patch fixes this problem.

I have tested the patch manually by adapting the validity check for nested OpenACC reductions (see omp-low.c)
to include the location of clauses in warnings instead of the location of the loop to which the clause belongs.
I can add a regression test based on this later on after adapting the code in omp-low.c.

Is it ok to include the patch in trunk?

Best regards,
Frederik


On 04.12.19 14:37, Tobias Burnus wrote:
> As reported internally by Frederik, gfortran currently passes LOCATION_COLUMN == 0 to the middle end. The reason for that is how parsing works – gfortran reads the input line by line.

> 

> For internal error diagnostic (fortran/error.c), the column location was corrected –  but not for locations passed to the middle end. Hence, the diagnostic there wasn't optimal.

> 

> Fixed by introducing a new function; now one only needs to make sure that no new code will re-introduce "lb->location" :-)

> 

> Build and regtested on x86-64-gnu-linux.

> OK for the trunk?

> 

> Tobias

Comments

Tobias Burnus Dec. 9, 2019, 4:02 p.m. | #1
LGTM. Thanks for the patch!

Tobias

On 12/9/19 4:58 PM, Harwath, Frederik wrote:
> Hi,

> Tobias has recently fixed a problem with the column information in gfortran locations

> ("PR 92793 - fix column used for error diagnostic"). Diagnostic messages for OpenMP/OpenACC

> clauses do not contain the right column information yet. The reason is that the location

> information of the first clause is used for all clauses on a line and hence the columns

> are wrong for all but the first clause. The attached patch fixes this problem.

>

> I have tested the patch manually by adapting the validity check for nested OpenACC reductions (see omp-low.c)

> to include the location of clauses in warnings instead of the location of the loop to which the clause belongs.

> I can add a regression test based on this later on after adapting the code in omp-low.c.

>

> Is it ok to include the patch in trunk?

>

> Best regards,

> Frederik

>

>

> On 04.12.19 14:37, Tobias Burnus wrote:

>> As reported internally by Frederik, gfortran currently passes LOCATION_COLUMN == 0 to the middle end. The reason for that is how parsing works – gfortran reads the input line by line.

>>

>> For internal error diagnostic (fortran/error.c), the column location was corrected –  but not for locations passed to the middle end. Hence, the diagnostic there wasn't optimal.

>>

>> Fixed by introducing a new function; now one only needs to make sure that no new code will re-introduce "lb->location" :-)

>>

>> Build and regtested on x86-64-gnu-linux.

>> OK for the trunk?

>>

>> Tobias

Patch

From af3a63b64f38d522b0091a123a919d1f20f5a8b1 Mon Sep 17 00:00:00 2001
From: Frederik Harwath <frederik@codesourcery.com>
Date: Mon, 9 Dec 2019 15:07:53 +0100
Subject: [PATCH] Fix column information for omp_clauses in Fortran code

The location of all OpenMP/OpenACC clauses on any given line in Fortran code
always points to the first clause on that line. Hence, the column information
is wrong for all clauses but the first one.

Use the correct location for each clause instead.

2019-12-09  Frederik Harwath  <frederik@codesourcery.com>

/gcc/fortran/
	* trans-openmp (gfc_trans_omp_reduction_list): Pass correct location for each
	clause to build_omp_clause.
---
 gcc/fortran/trans-openmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index d07ff86fc0b..356fd04e6c3 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -1982,7 +1982,7 @@  gfc_trans_omp_reduction_list (gfc_omp_namelist *namelist, tree list,
 	tree t = gfc_trans_omp_variable (namelist->sym, false);
 	if (t != error_mark_node)
 	  {
-	    tree node = build_omp_clause (gfc_get_location (&where),
+	    tree node = build_omp_clause (gfc_get_location (&namelist->where),
 					  OMP_CLAUSE_REDUCTION);
 	    OMP_CLAUSE_DECL (node) = t;
 	    if (mark_addressable)
-- 
2.17.1