diff mbox series

[RFC,Fortran] %C error diagnostic location

Message ID 8d1702a5-e217-e469-f7ff-c5d97cde2fe5@net-b.de
State New
Headers show
Series [RFC,Fortran] %C error diagnostic location | expand

Commit Message

Tobias Burnus Oct. 1, 2019, 10:26 p.m. UTC
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

Comments

Tobias Burnus Oct. 2, 2019, 9:26 a.m. UTC | #1
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
>
Steve Kargl Oct. 2, 2019, 5:24 p.m. UTC | #2
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.
diff mbox series

Patch


	* 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);
 }