Hi all,
I looked at error messages – and I like it. – Please review.
There is actually a "fallout": One testsuite case actually checks for
the row location – I didn't even know that one can do it that way.
That's fixed by the attached patch (I also included the original patch).
OK for the trunk?
Tobias
PS: The effect of the patch can also be seen at the patch description
at: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg00110.html
On 10/2/19 12:26 AM, Tobias Burnus wrote:
Hi all,
my feeling is that %C locations are always off by one, e.g., showing
the (1) under the last white-space character before the place where
the error occurred – the match starts at the character after the
gfc_current_location.
That bothered my for a while – but today, I was wondering whether one
shouldn't simply bump the %C location by one – such that it shows at
the first wrong character and not at the last okay character.
What do you think?
Another observation (unfixed): If gfortran buffers the error, the %C
does not seem to get resolved at gfc_{error,warning} time but at the
time when the buffer is flushed – which will have a reset error location.
Cheers,
Tobias
* error (error_print, gfc_format_decoder): Fix off-by one issue with %C.
* gfortran.dg/use_without_only_1.f90: Update column num in dg-warning.
diff --git a/gcc/fortran/error.c b/gcc/fortran/error.c
index a0ce7a6b190..815cae9d7e7 100644
--- a/gcc/fortran/error.c
+++ b/gcc/fortran/error.c
@@ -618,12 +618,18 @@ error_print (const char *type, const char *format0, va_list argp)
{
l2 = loc;
arg[pos].u.stringval = "(2)";
+ /* Point %C first offending character not the last good one. */
+ if (arg[pos].type == TYPE_CURRENTLOC)
+ l2->nextc++;
}
else
{
l1 = loc;
have_l1 = 1;
arg[pos].u.stringval = "(1)";
+ /* Point %C first offending character not the last good one. */
+ if (arg[pos].type == TYPE_CURRENTLOC)
+ l1->nextc++;
}
break;
@@ -963,6 +969,9 @@ gfc_format_decoder (pretty_printer *pp, text_info *text, const char *spec,
loc = va_arg (*text->args_ptr, locus *);
gcc_assert (loc->nextc - loc->lb->line >= 0);
unsigned int offset = loc->nextc - loc->lb->line;
+ if (*spec == 'C')
+ /* Point %C first offending character not the last good one. */
+ offset++;
/* If location[0] != UNKNOWN_LOCATION means that we already
processed one of %C/%L. */
int loc_num = text->get_location (0) == UNKNOWN_LOCATION ? 0 : 1;
@@ -1400,7 +1409,7 @@ gfc_internal_error (const char *gmsgid, ...)
void
gfc_clear_error (void)
{
- error_buffer.flag = 0;
+ error_buffer.flag = false;
warnings_not_errors = false;
gfc_clear_pp_buffer (pp_error_buffer);
}
diff --git a/gcc/testsuite/gfortran.dg/use_without_only_1.f90 b/gcc/testsuite/gfortran.dg/use_without_only_1.f90
index 06af9853933..ad64b206b76 100644
--- a/gcc/testsuite/gfortran.dg/use_without_only_1.f90
+++ b/gcc/testsuite/gfortran.dg/use_without_only_1.f90
@@ -6,16 +6,16 @@ MODULE foo
END MODULE
MODULE testmod
- USE foo ! { dg-warning "6:has no ONLY qualifier" }
+ USE foo ! { dg-warning "7:has no ONLY qualifier" }
IMPLICIT NONE
CONTAINS
SUBROUTINE S1
- USE foo ! { dg-warning "9:has no ONLY qualifier" }
+ USE foo ! { dg-warning "10:has no ONLY qualifier" }
END SUBROUTINE S1
SUBROUTINE S2
USE foo, ONLY: bar
END SUBROUTINE
SUBROUTINE S3
- USE ISO_C_BINDING ! { dg-warning "9:has no ONLY qualifier" }
+ USE ISO_C_BINDING ! { dg-warning "10:has no ONLY qualifier" }
END SUBROUTINE S3
END MODULE
