Hi Tobias!
On 2020-10-30T12:16:05+0100, Tobias Burnus <tob...@codesourcery.com> wrote:
> On 30.10.20 11:47, Thomas Schwinge wrote:
>>>> Fixed by introducing a new function; now one only needs to make sure
>>>> that no new code will re-introduce "lb->location":-)
>> ... another*existing instance* of this problem.
> ...
>> gfc_set_backend_locus (locus * loc)
>> {
>> gfc_current_backend_file = loc->lb->file;
>> - input_location = loc->lb->location;
>> + input_location = gfc_get_location (loc);
>> }
>
> In bare usage, it seems to be fine – which are 23 callers.
>
> However, there is additionally:
>
> gfc_save_backend_locus (locus * loc)
> {
> loc->lb = XCNEW (gfc_linebuf);
> loc->lb->location = input_location;
> loc->lb->file = gfc_current_backend_file;
> }
>
> which is used together with:
>
> gfc_restore_backend_locus (locus * loc)
> {
> gfc_set_backend_locus (loc);
> free (loc->lb);
> }
>
> I think the latter needs to be replaced by the previous
> version of "gfc_save_backend_locus" for two related reasons:
>
> * gfc_save_backend_locus operates with incomplete data,
> i.e. loc->nextc (used by gfc_get_location) might not
> be set.
> * input_location might/should already contain the column
> offset – and you do not want to add some random offset
> to it.
>
> Hence: LGTM – if you update 'gfc_restore_backend_locus'
> by inlining the previous version of 'gfc_set_backend_locus'.
Thanks for the review; absolutely right, sorry for not realizing that on
my own.
Thusly changed, see attached, pushed "Further improve Fortran column
location information [PR92793]" to master branch in commit
5677444f7e7ca15557030902c3d09dab4852fa90, and backported to
releases/gcc-10 branch in commit
a5c5f9e181c1f7548930f1cab91002b9d460cc92.
Grüße
Thomas
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
>From 5677444f7e7ca15557030902c3d09dab4852fa90 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Fri, 30 Oct 2020 13:13:51 +0100
Subject: [PATCH] Further improve Fortran column location information [PR92793]
Building on top of commit 9c81750c5bedd7883182ee2684a012c6210ebe1d "Fortran] PR
92793 - fix column used for error diagnostic", there is another place where we
have to use 'gfc_get_location' returning column-corrected locations.
For example, this improves column location information for OMP constructs.
gcc/fortran/
PR fortran/92793
* trans.c (gfc_set_backend_locus): Use 'gfc_get_location'.
(gfc_restore_backend_locus): Adjust.
gcc/testsuite/
PR fortran/92793
* gfortran.dg/goacc/pr92793-1.f90: Adjust.
---
gcc/fortran/trans.c | 7 ++++--
gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 | 24 +++++++++----------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index 8caa625ab0e8..025abe389853 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -1829,7 +1829,7 @@ void
gfc_set_backend_locus (locus * loc)
{
gfc_current_backend_file = loc->lb->file;
- input_location = loc->lb->location;
+ input_location = gfc_get_location (loc);
}
@@ -1839,7 +1839,10 @@ gfc_set_backend_locus (locus * loc)
void
gfc_restore_backend_locus (locus * loc)
{
- gfc_set_backend_locus (loc);
+ /* This only restores the information captured by gfc_save_backend_locus,
+ intentionally does not use gfc_get_location. */
+ input_location = loc->lb->location;
+ gfc_current_backend_file = loc->lb->file;
free (loc->lb);
}
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90
index a572c6b3437b..72dd6b7b8f81 100644
--- a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90
@@ -13,22 +13,22 @@ subroutine check ()
integer :: i, j, sum, diff
!$acc parallel &
- !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information.
-!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma acc parallel" 1 "original" } }
- !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma omp target oacc_parallel" 1 "gimple" } }
+ !$acc & & ! Fortran location information points to the last line, and last character of the directive.
+!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma acc parallel" 1 "original" } }
+ !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma omp target oacc_parallel" 1 "gimple" } }
!$acc loop &
- !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information.
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "original" } }
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "gimple" } }
+ !$acc & & ! Fortran location information points to the last line, and last character of the directive.
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "original" } }
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "gimple" } }
!$acc& reduction ( + : sum ) & ! { dg-line sum1 }
!$acc && ! Fortran location information points to the ':' in 'reduction(+:sum)'.
!$acc & & ! { dg-message "36: location of the previous reduction for 'sum'" "" { target *-*-* } sum1 }
!$acc& independent
do i = 1, 10
!$acc loop &
-!$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information.
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "original" } }
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "gimple" } }
+!$acc & & ! Fortran location information points to the last line, and last character of the directive.
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "original" } }
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "gimple" } }
!$acc & reduction(-: diff ) &
!$acc&reduction(- : sum) & ! { dg-line sum2 }
!$acc & & ! Fortran location information points to the ':' in 'reduction(-:sum)'.
@@ -38,9 +38,9 @@ subroutine check ()
sum &
& = &
& 1
- ! Fortran location information points to the last line of the statement, and there is no column location information.
- ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "original" } }
- ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "gimple" } }
+ ! Fortran location information points to the last line, and last character of the statement.
+ ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "original" } }
+ ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "gimple" } }
end do
end do
!$acc end parallel
--
2.17.1
>From a5c5f9e181c1f7548930f1cab91002b9d460cc92 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Fri, 30 Oct 2020 13:13:51 +0100
Subject: [PATCH] Further improve Fortran column location information [PR92793]
Building on top of commit 9c81750c5bedd7883182ee2684a012c6210ebe1d "Fortran] PR
92793 - fix column used for error diagnostic", there is another place where we
have to use 'gfc_get_location' returning column-corrected locations.
For example, this improves column location information for OMP constructs.
gcc/fortran/
PR fortran/92793
* trans.c (gfc_set_backend_locus): Use 'gfc_get_location'.
(gfc_restore_backend_locus): Adjust.
gcc/testsuite/
PR fortran/92793
* gfortran.dg/goacc/pr92793-1.f90: Adjust.
(cherry picked from commit 5677444f7e7ca15557030902c3d09dab4852fa90)
---
gcc/fortran/trans.c | 7 ++++--
gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 | 24 +++++++++----------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index ed0542614523..e3043d499943 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -1808,7 +1808,7 @@ void
gfc_set_backend_locus (locus * loc)
{
gfc_current_backend_file = loc->lb->file;
- input_location = loc->lb->location;
+ input_location = gfc_get_location (loc);
}
@@ -1818,7 +1818,10 @@ gfc_set_backend_locus (locus * loc)
void
gfc_restore_backend_locus (locus * loc)
{
- gfc_set_backend_locus (loc);
+ /* This only restores the information captured by gfc_save_backend_locus,
+ intentionally does not use gfc_get_location. */
+ input_location = loc->lb->location;
+ gfc_current_backend_file = loc->lb->file;
free (loc->lb);
}
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90 b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90
index a572c6b3437b..72dd6b7b8f81 100644
--- a/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90
+++ b/gcc/testsuite/gfortran.dg/goacc/pr92793-1.f90
@@ -13,22 +13,22 @@ subroutine check ()
integer :: i, j, sum, diff
!$acc parallel &
- !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information.
-!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma acc parallel" 1 "original" } }
- !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:0\\\] #pragma omp target oacc_parallel" 1 "gimple" } }
+ !$acc & & ! Fortran location information points to the last line, and last character of the directive.
+!$acc && ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma acc parallel" 1 "original" } }
+ !$acc & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:18:123\\\] #pragma omp target oacc_parallel" 1 "gimple" } }
!$acc loop &
- !$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information.
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "original" } }
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:0\\\] #pragma acc loop" 1 "gimple" } }
+ !$acc & & ! Fortran location information points to the last line, and last character of the directive.
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "original" } }
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:26:22\\\] #pragma acc loop" 1 "gimple" } }
!$acc& reduction ( + : sum ) & ! { dg-line sum1 }
!$acc && ! Fortran location information points to the ':' in 'reduction(+:sum)'.
!$acc & & ! { dg-message "36: location of the previous reduction for 'sum'" "" { target *-*-* } sum1 }
!$acc& independent
do i = 1, 10
!$acc loop &
-!$acc & & ! Fortran location information points to the last line of the directive, and there is no column location information.
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "original" } }
- !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:0\\\] #pragma acc loop" 1 "gimple" } }
+!$acc & & ! Fortran location information points to the last line, and last character of the directive.
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "original" } }
+ !$acc & & ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:36:34\\\] #pragma acc loop" 1 "gimple" } }
!$acc & reduction(-: diff ) &
!$acc&reduction(- : sum) & ! { dg-line sum2 }
!$acc & & ! Fortran location information points to the ':' in 'reduction(-:sum)'.
@@ -38,9 +38,9 @@ subroutine check ()
sum &
& = &
& 1
- ! Fortran location information points to the last line of the statement, and there is no column location information.
- ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "original" } }
- ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:0\\\] sum = 1" 1 "gimple" } }
+ ! Fortran location information points to the last line, and last character of the statement.
+ ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "original" } }
+ ! { dg-final { scan-tree-dump-times "pr92793-1\\\.f90:40:9\\\] sum = 1" 1 "gimple" } }
end do
end do
!$acc end parallel
--
2.17.1
