| Message ID | 8d1702a5-e217-e469-f7ff-c5d97cde2fe5@net-b.de |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,Fortran] %C error diagnostic location | expand |
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 >
On Wed, Oct 02, 2019 at 11:26:17AM +0200, Tobias Burnus wrote: > 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? > Looks good.
* error (error_print, gfc_format_decoder): Fix off-by one issue with %C. 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); }
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