diff mbox series

[RFA,fortran] Fix # <line> <file> handling in the Fortran front-end

Message ID 704e1a86-f096-9418-818b-d0d322a8c1f5@redhat.com
State New
Headers show
Series [RFA,fortran] Fix # <line> <file> handling in the Fortran front-end | expand

Commit Message

Jeff Law May 4, 2018, 7:32 p.m. UTC 
The Fortran front end has its own code to parse # <line> <file>
directives.  We've run into a case where it does not function correctly.
 In particular when the directive changes the current file, subsequent
diagnostics still refer to the original filename.

Concretely take this code and compile it with -Wall:



# 12345 "foo-f"
SUBROUTINE s(dummy)
  INTEGER, INTENT(in) :: dummy
END SUBROUTINE


The "dummy" argument is unused and you'll get a diagnostic like:

-bash-4.3$ gfortran j.f90 -Wall
j.f90:12345:18:

Warning: Unused dummy argument ‘dummy’ at (1) [-Wunused-dummy-argument]

Note how we got the right line #, but the wrong file in the diagnostic.
It should look something like this:

[law@torsion gcc]$ ./gfortran -Wall -B./ j.f90
foo-f:12345:18:

Warning: Unused dummy argument ‘dummy’ at (1) [-Wunused-dummy-argument]

--


AFAICT the Fortran front-end has failed to notify the linemap interface
that the current filename has changed.

This patch (and testcase) fixes the problem by adding the missing call
to linemap_add.

Bootstrapped and regression tested on x86_64-linux-gnu.  OK for the trunk?

Jeff

Comments

Steve Kargl May 4, 2018, 7:55 p.m. UTC | #1 
On Fri, May 04, 2018 at 01:32:00PM -0600, Jeff Law wrote:
> 
> The Fortran front end has its own code to parse # <line> <file>
> directives.  We've run into a case where it does not function correctly.
>  In particular when the directive changes the current file, subsequent
> diagnostics still refer to the original filename.
> 
> Concretely take this code and compile it with -Wall:
> 
> # 12345 "foo-f"
> SUBROUTINE s(dummy)
>   INTEGER, INTENT(in) :: dummy
> END SUBROUTINE

Can you tell us where the above comes from?

If I have the three lines of code in a file 
name 'a.inc', and use either the Fortran
INCLUDE statement or a cpp #include statement
I get what I expect.

% cat a.f90
module bar
  contains
  include 'a.inc'
end module bar
% gfcx -c -Wall a.f90
a.inc:1:18:

 SUBROUTINE s(dummy)
                  1
Warning: Unused dummy argument 'dummy' at (1) [-Wunused-dummy-argument]

% cat a.F90
module bar
  contains
#include "a.inc"
end module bar
% gfcx -c -Wall a.F90
a.inc:1:18:
 
 SUBROUTINE s(dummy)
                  1
Warning: Unused dummy argument 'dummy' at (1) [-Wunused-dummy-argument]

> 
> Bootstrapped and regression tested on x86_64-linux-gnu.  OK for the trunk?
> 

I don't have any objection to the patch, but I would like
to understand where '# 12345 "foo-f"' comes from.
Jeff Law May 4, 2018, 8:05 p.m. UTC | #2 
On 05/04/2018 01:55 PM, Steve Kargl wrote:
> On Fri, May 04, 2018 at 01:32:00PM -0600, Jeff Law wrote:
>>
>> The Fortran front end has its own code to parse # <line> <file>
>> directives.  We've run into a case where it does not function correctly.
>>  In particular when the directive changes the current file, subsequent
>> diagnostics still refer to the original filename.
>>
>> Concretely take this code and compile it with -Wall:
>>
>> # 12345 "foo-f"
>> SUBROUTINE s(dummy)
>>   INTEGER, INTENT(in) :: dummy
>> END SUBROUTINE
> 
> Can you tell us where the above comes from?
Use of # <line> <file> is a standard CPP directive one can use to change
the compiler's notion of file/line.   It's defined by ISO for C/C++ and
appears to be relatively common in other Fortran compilers.  GNU Fortran
tries to handle it and just gets it slightly wrong.

It's most typically used when source code is generated by another program.




> 
> If I have the three lines of code in a file 
> name 'a.inc', and use either the Fortran
> INCLUDE statement or a cpp #include statement
> I get what I expect.
Right.  That uses a slightly different form internally.  It'll get
turned into

# <line> <file> <flags>

Where the flags will indicate entry/exit from included files.  That form
is handled correctly by the GNU Fortran front end.  But this form is not
for external use.


Jeff
Steve Kargl May 4, 2018, 8:08 p.m. UTC | #3 
On Fri, May 04, 2018 at 02:05:11PM -0600, Jeff Law wrote:
> On 05/04/2018 01:55 PM, Steve Kargl wrote:
> > On Fri, May 04, 2018 at 01:32:00PM -0600, Jeff Law wrote:
> >>
> >> The Fortran front end has its own code to parse # <line> <file>
> >> directives.  We've run into a case where it does not function correctly.
> >>  In particular when the directive changes the current file, subsequent
> >> diagnostics still refer to the original filename.
> >>
> >> Concretely take this code and compile it with -Wall:
> >>
> >> # 12345 "foo-f"
> >> SUBROUTINE s(dummy)
> >>   INTEGER, INTENT(in) :: dummy
> >> END SUBROUTINE
> > 
> > Can you tell us where the above comes from?
> Use of # <line> <file> is a standard CPP directive one can use to change
> the compiler's notion of file/line.   It's defined by ISO for C/C++ and
> appears to be relatively common in other Fortran compilers.  GNU Fortran
> tries to handle it and just gets it slightly wrong.
> 
> It's most typically used when source code is generated by another program.
> 
> > If I have the three lines of code in a file 
> > name 'a.inc', and use either the Fortran
> > INCLUDE statement or a cpp #include statement
> > I get what I expect.
> Right.  That uses a slightly different form internally.  It'll get
> turned into
> 
> # <line> <file> <flags>
> 
> Where the flags will indicate entry/exit from included files.  That form
> is handled correctly by the GNU Fortran front end.  But this form is not
> for external use.

Thanks for the explanation.  The patch looks good to me.
diff mbox series

Patch

diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index aab5379..55d6daf 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -2107,6 +2107,10 @@  preprocessor_line (gfc_char_t *c)
           in the linemap.  Alternative could be using GC or updating linemap to
           point to the new name, but there is no API for that currently.  */
       current_file->filename = xstrdup (filename);
+
+      /* We need to tell the linemap API that the filename changed.  Just
+	 changing current_file is insufficient.  */
+      linemap_add (line_table, LC_RENAME, false, current_file->filename, line);
     }
 
   /* Set new line number.  */
diff --git a/gcc/testsuite/gfortran.dg/linefile.f90 b/gcc/testsuite/gfortran.dg/linefile.f90
new file mode 100644
index 0000000..7f1465a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/linefile.f90
@@ -0,0 +1,12 @@ 
+! { dg-do compile }
+! { dg-options "-Wall" }
+
+# 4 "foo-f"
+SUBROUTINE s(dummy)
+  INTEGER, INTENT(in) :: dummy
+END SUBROUTINE
+! We want to check that the # directive changes the filename in the
+! diagnostic.  Nothing else really matters here.  dg-regexp allows us
+! to see the entire diagnostic.  We just have to make sure to consume
+! the entire message.
+! { dg-regexp "foo-f\[^\n]*" }