[patch, fortran] error.c: Cleanup, refactoring, column numbers in filename lines.

Brooks Moses brooks.moses@codesourcery.com
Mon Nov 6 07:35:00 GMT 2006


(Why?  Well, since I was looking at error.c already from the last patch, 
I decided to read through it to get a better idea how it worked, and ... 
well, it kind of get refactored a bit and grew some extra comments while 
I was understanding it.  And people seem to think the column numbers are 
handy.)

The attached set of patches are all modifications to error.c, 
implementing a sort of general refactoring of some of the error 
processing in order to fix some minor bugs, and make the code notably 
more legible and maintainable (at least IMHO).  In addition, once I'd 
done all this, implementing column numbers in the "[file]:[line]:" line 
was fairly simple.

I've broken the overall changes up into a set of patches that can be 
applied sequentially; I'll explain them in order:

------------------------------------------------------------------
error3.diff:

When an error occurs in a file that is included in another file, 
GFortran prints out a "[file]:[line]:" line for the source file with the 
error, and follows this with "Included in:" lines for the stack of 
including files.  Currently, there is a useless blank line before each 
"Included in:" line.  This patch removes that blank lines.  It also adds 
some comments to nearby code.

Interestingly, this particular error functionality does not show up 
anywhere in the testsuite.

------------------------------------------------------------------
error4.diff:

When there are two error loci on the same line at the same column, 
GFortran prints out '12' such that the '2' is one column to the right of 
the locus, rather than only printing the '1'.  This cleans up the code 
that accomplishes this.

The cleanup, incidentally, means that this motion of the '2' can be 
accounted for when the line is offset.

------------------------------------------------------------------
error5.diff:

When a line contains only a single locus, and the locus is too far to 
the right to fit onscreen without offsetting the offending source-code 
line, GFortran does some rather strange calculations to figure out how 
to offset the line.

Specifically, it places the locus on the first line 5 columns from the 
left margin, and the locus on the second line 20 columns from the left 
margin.  This is rather bizarre -- since the error is known to be near 
the end of the line, it means that very little of the line is actually 
shown.

This patch changes the offset calculation to one that makes much more 
sense -- namely, setting the loci 5 and 20 columns, respectively, from 
the _right_ margin.

In addition, it adds some more comments, including a TODO that I was 
meaning to come back and address later.

------------------------------------------------------------------
error6.diff:

Now that the offset calculations are all nearly the same, there's no 
point in putting them (and the printing of the column-marker lines) in 
three nearly-identical bits of code in show_loci, when it conceptually 
fits much better in show_locus anyway.  Thus, this patch passes the 
column locations of the markers to show_locus, and calculates the offset 
there.

------------------------------------------------------------------
error7.diff:

A somewhat trivial patch.  This adds a couple of comments, and does a 
minor cleanup of the code I introduced to remove the blank space at the 
beginning of the "[file]:[line]:" line.

------------------------------------------------------------------
error8.diff:

Also a pretty trivial patch.  Aside from the comments, this adds some 
cases to the part of error_print that handles '%' characters in the 
format, so that an unescaped '%' at the end of the format string doesn't 
create a buffer overflow -- instead, the pointer is backed up a
character so that only the '%', and not the trailing '\0', is gobbled.
Ideally, we'll never need this code, but it seems better to have it.

I would have made this an ICE, but having even the vague possibility of 
an infinite recursion in the ICE code seemed a bad idea.  Maybe it
would make more sense to put in code that emulates gfc_internal_error, 
rather than just silently ignoring the problem, though.

Also, this should perhaps be a "default:" rather than "case '\0':", if 
we error out.

------------------------------------------------------------------
error9.diff:

This modifies the "[file]:[line]:" line to look like one of the 
following, depending on whether or not the loci have column information:

   [file]:[line]:
   [file]:[line].[column]:
   [file]:[line].[column]-[column]:

This is in accordance with the GNU error-message coding standards.

A subsidiary change to this was that it was easier to _not_ use 
error_print to print these lines, since the format varies.  However, the 
integer-printing code from error_print was useful, so I moved it off 
into its own function, error_integer (by analogy to error_char and 
error_string), where it probably should have been anyway.

------------------------------------------------------------------
error10.diff:

Needless to say, error9.diff breaks the testsuite.  This fixes it.

------------------------------------------------------------------

fortran/ChangeLog:

2006-11-05  Brooks Moses  <brooks.moses@codesourcery.com>

	* error.c (show_loci): Move column-offset calculation to
	show_locus.
	(show_locus): Remove blank lines before "Included in"
	lines, clean up code, calculate column-offsets, print
	column number is error-header lines as appropriate.
	(error_integer): (new function) Print integer to error
	buffer.
	(error_print): Use error_integer, avoid possible buffer
	overflows from buggy error formats.

testsuite/ChangeLog:

2006-11-05  Brooks Moses  <brooks.moses@codesourcery.com>

	* lib/gfortran-dg.exp (gfortran-dg-test): Ignore column
	numbers in error message headers.

------------------------------------------------------------------

Since trunk bootstrap seems to be broken, I'm regression testing this 
against 4.2 at present, and it seems to be working fine so far.  (It's 
not quite done with the tests, but has gone quite far enough to confirm 
that I haven't broken the basic error-message handling.)

Ok for trunk, once someone fixes the bootstrap, and I get a chance to 
regression test it there?

- Brooks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error3.diff
Type: text/x-patch
Size: 1768 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error4.diff
Type: text/x-patch
Size: 1237 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error5.diff
Type: text/x-patch
Size: 1939 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error6.diff
Type: text/x-patch
Size: 5176 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error7.diff
Type: text/x-patch
Size: 1176 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error8.diff
Type: text/x-patch
Size: 1201 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error9.diff
Type: text/x-patch
Size: 3198 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: error10.diff
Type: text/x-patch
Size: 1915 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20061106/7737e6d2/attachment-0007.bin>


More information about the Gcc-patches mailing list