[Bug fortran/54224] Warn for unused internal procedures

[dominiq at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #31 from dominiq at gcc dot gnu.org ---
Author: dominiq
Date: Fri Nov  6 21:49:18 2015
New Revision: 229894

URL: https://gcc.gnu.org/viewcvs?rev=229894=gcc=rev
Log:
2015-11-06  Dominique d'Humieres 

PR fortran/54224
* gfortran.dg/warn_unused_function_2.f90: Add two new 
"defined but not used" subroutines.


Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gfortran.dg/warn_unused_function_2.f90

[Bug fortran/54224] Warn for unused internal procedures

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

Dominique d'Humieres  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #32 from Dominique d'Humieres  ---
Patch committed and column position tracked by pr63327. Closing as FIXED.

[Bug fortran/54224] Warn for unused internal procedures

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #30 from Dominique d'Humieres  ---
> so I give up. I am planning to submit the following patch, open a new PR
> for the bad locus, then close this PR as fixed.

The new PR won't be necessary: it is already pr63327.

[Bug fortran/54224] Warn for unused internal procedures

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #25 from Dominique d'Humieres  ---
> Nonetheless, as a work-around, you could use:
>
>unsigned int offset = loc->nextc - loc->lb->line;
>location = linemap_position_for_loc_and_offset (line_table,
>loc->lb->location,
>offset));
>
> to create a new location_t with the correct column number and use that
> for the tree it creates.

Where should this be done?

[Bug fortran/54224] Warn for unused internal procedures

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #29 from Dominique d'Humieres  ---
I have tried the following patch

--- ../_clean/gcc/fortran/trans-decl.c  2015-10-29 18:20:14.0 +0100
+++ gcc/fortran/trans-decl.c2015-11-03 19:39:39.0 +0100
@@ -285,7 +285,11 @@ gfc_build_label_decl (tree label_id)
 void
 gfc_set_decl_location (tree decl, locus * loc)
 {
-  DECL_SOURCE_LOCATION (decl) = loc->lb->location;
+  unsigned int offset = loc->nextc - loc->lb->line;
+  DECL_SOURCE_LOCATION (decl) = linemap_position_for_loc_and_offset
(line_table,
+   
loc->lb->location,
+offset);
+  /* DECL_SOURCE_LOCATION (decl) = loc->lb->location; */
 }


but it does not fix the issue and makes
gfortran.dg/array_constructor_type_14.f03 and
gfortran.dg/realloc_on_assign_18.f90 regress

/opt/gcc/work/gcc/testsuite/gfortran.dg/array_constructor_type_14.f03:7:0:

 PROGRAM test
1
internal compiler error: in linemap_position_for_loc_and_offset, at
libcpp/line-map.c:714

so I give up. I am planning to submit the following patch, open a new PR for
the bad locus, then close this PR as fixed.

--- ../_clean/gcc/testsuite/gfortran.dg/warn_unused_function_2.f90 
2014-03-26 17:49:50.0 +0100
+++ gcc/testsuite/gfortran.dg/warn_unused_function_2.f902015-11-03
12:47:30.0 +0100
@@ -2,6 +2,7 @@
 ! { dg-options "-Wall" }
 !
 ! [4.8 Regression] PR 54997: -Wunused-function gives false warnings
+! PR 54224: missing warnings with -Wunused-function
 !
 ! Contributed by Janus Weil 

@@ -14,6 +15,9 @@ contains

   subroutine s1! { dg-warning "defined but not used" }
 call s2(s3)
+contains
+  subroutine s4! { dg-warning "defined but not used" }
+  end subroutine
   end subroutine

   subroutine s2(dummy) ! { dg-warning "Unused dummy argument" }
@@ -30,5 +34,10 @@ subroutine sub
 entry en
 end subroutine

+program test
+contains
+  subroutine s5! { dg-warning "defined but not used" }
+  end subroutine
+end

 ! { dg-final { cleanup-modules "m" } }

[Bug fortran/54224] Warn for unused internal procedures

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #28 from Manuel López-Ibáñez  ---
(In reply to Dominique d'Humieres from comment #27)
> > At some moment, the Fortran FE sets DECL_SOURCE_LOCATION() on a tree,
> > probably using loc->lb->line. Finding out where this happens probably
> > requires a bit of debugging.
> 
> /* Set the backend source location of a decl.  */
> 
> void
> gfc_set_decl_location (tree decl, locus * loc)
> {
>   DECL_SOURCE_LOCATION (decl) = loc->lb->location;
> }
> 
> in gcc/fortran/trans-decl.c?

I guess you can always try, but yes it looks one place that could be fixed. I
suggest to add a wrapper function. Something like:

location_t
gfc_location_from_locus (const locus * loc)
{
unsigned int offset = loc->nextc - loc->lb->line;
return linemap_position_for_loc_and_offset (line_table,
loc->lb->location,
offset));
}

I think there is at least one place already that could use this wrapper.

[Bug fortran/54224] Warn for unused internal procedures

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #27 from Dominique d'Humieres  ---
> At some moment, the Fortran FE sets DECL_SOURCE_LOCATION() on a tree,
> probably using loc->lb->line. Finding out where this happens probably
> requires a bit of debugging.

/* Set the backend source location of a decl.  */

void
gfc_set_decl_location (tree decl, locus * loc)
{
  DECL_SOURCE_LOCATION (decl) = loc->lb->location;
}

in gcc/fortran/trans-decl.c?

[Bug fortran/54224] Warn for unused internal procedures

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #26 from Manuel López-Ibáñez  ---
(In reply to Dominique d'Humieres from comment #25)
> > Nonetheless, as a work-around, you could use:
> >
> >unsigned int offset = loc->nextc - loc->lb->line;
> >location = linemap_position_for_loc_and_offset (line_table,
> >loc->lb->location,
> >offset));
> >
> > to create a new location_t with the correct column number and use that
> > for the tree it creates.
> 
> Where should this be done?

At some moment, the Fortran FE sets DECL_SOURCE_LOCATION() on a tree, probably
using loc->lb->line. Finding out where this happens probably requires a bit of
debugging.

[Bug fortran/54224] Warn for unused internal procedures

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

Dominique d'Humieres  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #23 from Dominique d'Humieres  ---
> I'm not a Fortran dev, but I would humbly suggest to add this testcase
> to the regression testsuite before closing the bug.

OK. REOPENED.

> Also:
>
> > pr54224_3.f90:6:0:
> > 
> > subroutine s2
> > ^
>
> You will get a more precise column info (and better location for '^')
> if Fortran gives a more precise DECL_SOURCE_LOCATION() when generating this 
> tree.

Could you please elaborate?

[Bug fortran/54224] Warn for unused internal procedures

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

Manuel López-Ibáñez  changed:

   What|Removed |Added

 CC||manu at gcc dot gnu.org

--- Comment #22 from Manuel López-Ibáñez  ---
(In reply to Dominique d'Humieres from comment #21)
> Closing as FIXED. Please file new PR(s) for new issue(s).

I'm not a Fortran dev, but I would humbly suggest to add this testcase to the
regression testsuite before closing the bug. Also:

> pr54224_3.f90:6:0:
> 
> subroutine s2
> ^

You will get a more precise column info (and better location for '^') if
Fortran gives a more precise DECL_SOURCE_LOCATION() when generating this tree.

[Bug fortran/54224] Warn for unused internal procedures

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

Dominique d'Humieres  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #21 from Dominique d'Humieres  ---
> Still open: Comment 13.

Confirmed up to 5.2, but with trunk (6.0) I get

pr54224_3.f90:13:0:

   subroutine s3
^
Warning: 's3' defined but not used [-Wunused-function]
pr54224_3.f90:4:0:

  subroutine s1
^
Warning: 's1' defined but not used [-Wunused-function]
pr54224_3.f90:6:0:

subroutine s2
^
Warning: 's2' defined but not used [-Wunused-function]

This occurred between revisions r224160 (2015-06-05, s1 only) and r224647
(2015-06-19, above warnings).

Closing as FIXED. Please file new PR(s) for new issue(s).

[Bug fortran/54224] Warn for unused internal procedures

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

--- Comment #24 from Manuel López-Ibáñez  ---
(In reply to Dominique d'Humieres from comment #23)
> > I'm not a Fortran dev, but I would humbly suggest to add this testcase
> > to the regression testsuite before closing the bug.
> 
> OK. REOPENED.
> 
> > Also:
> >
> > > pr54224_3.f90:6:0:
> > > 
> > > subroutine s2
> > > ^
> >
> > You will get a more precise column info (and better location for '^')
> > if Fortran gives a more precise DECL_SOURCE_LOCATION() when generating this 
> > tree.
> 
> Could you please elaborate?

I don't know the Fortran FE that well, but at some moment it creates this tree
and it sets DECL_SOURCE_LOCATION(). Probably based on some loc->lb->location.
However, that only contains line number info because Fortran does not use
libcpp/line-map.c for tracking column numbers (it only creates new locations
for line changes).

Fully using line-map.c in Fortran is likely to be a lot of work, but it could
mean getting precise macro locations
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53934#c2) and remove some of the
duplication between libcpp and Fortran (tokenizer, file inclusion, etc.)

Nonetheless, as a work-around, you could use:

unsigned int offset = loc->nextc - loc->lb->line;
location = linemap_position_for_loc_and_offset (line_table,
loc->lb->location,
offset));

to create a new location_t with the correct column number and use that for the
tree it creates.

[Bug fortran/54224] Warn for unused internal procedures

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

Richard Biener rguenth at gcc dot gnu.org changed:

   What|Removed |Added

   Target Milestone|4.8.3   |---

[Bug fortran/54224] Warn for unused internal procedures

[janus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54224

janus at gcc dot gnu.org changed:

   What|Removed |Added

Summary|Warn for unused (private)   |Warn for unused internal
   |module variables and|procedures
   |internal procedures |

--- Comment #19 from janus at gcc dot gnu.org ---
Still open: Comment 13.