Bug 66605

I should note that removing "-Wunused-parameter" allows compilation without error.

The problem is that 
0x6b9191 gfc_generate_function_code(gfc_namespace*)
        /home/manuel/test1/pristine/gcc/fortran/trans-decl.c:6021

invokes code that may generate warnings from the middle-end (like -Wunused-parameter), however, gfc_diagnostics_finish has not been called yet.

I don't know the Fortran FE well enough to decide whether gfc_diagnostics_finish should be called earlier (or are fortran diagnostics expected after this point?), or we should special case this call by using something like:

Index: error.c
===================================================================
--- error.c     (revision 224467)
+++ error.c     (working copy)
@@ -1461,21 +1461,27 @@ gfc_errors_to_warnings (bool f)
 {
   warnings_not_errors = f;
 }
 
 void
-gfc_diagnostics_init (void)
+gfc_diagnostics_defaults (void)
 {
   diagnostic_starter (global_dc) = gfc_diagnostic_starter;
   diagnostic_finalizer (global_dc) = gfc_diagnostic_finalizer;
   diagnostic_format_decoder (global_dc) = gfc_format_decoder;
   global_dc->caret_chars[0] = '1';
   global_dc->caret_chars[1] = '2';
+}
+
+void
+gfc_diagnostics_init (void)
+{
   pp_warning_buffer = new (XNEW (output_buffer)) output_buffer ();
   pp_warning_buffer->flush_p = false;
   pp_error_buffer = &(error_buffer.buffer);
   pp_error_buffer->flush_p = false;
+  gfc_diagnostics_defaults ();
 }
 
 void
 gfc_diagnostics_finish (void)
 {
Index: trans-decl.c
===================================================================
--- trans-decl.c        (revision 224467)
+++ trans-decl.c        (working copy)
@@ -6016,11 +6016,15 @@ gfc_generate_function_code (gfc_namespac
         the cgraph node for this function.  */
       if (!has_coarray_vars || flag_coarray != GFC_FCOARRAY_LIB)
        (void) cgraph_node::create (fndecl);
     }
   else
-    cgraph_node::finalize_function (fndecl, true);
+    {
+      gfc_diagnostics_finish ();
+      cgraph_node::finalize_function (fndecl, true);
+      gfc_diagnostics_defaults ();
+    }
 
   gfc_trans_use_stmts (ns);
   gfc_traverse_ns (ns, gfc_emit_parameter_debug_info);
 
   if (sym->attr.is_main_program)

BTW, is the resulting warning actually correct?

This is the warning we get when "fixed":

/home/manuel/test3/src/gcc/testsuite/gfortran.dg/wunused-parameter.f90:8:0:

 SUBROUTINE sub (neq, time, y, dydt)
^
Warning: unused parameter ‘time’ [-Wunused-parameter]

There is no such warning with 4.3.1

> BTW, is the resulting warning actually correct?

According the gfortran manual

-Wunused-parameter
Contrary to gcc's meaning of -Wunused-parameter, gfortran's implementation of this option does not warn about unused dummy arguments (see -Wunused-dummy-argument), but about unused PARAMETER values. -Wunused-parameter is implied by -Wextra if also -Wunused or -Wall is used. 

If I understand correctly the above statement, -Wunused-parameter should not emit a warning for the test in comment 0: there is no parameter. The warning should be issued with -Wunused-dummy-argument

pr66605.f90:5:25:

 SUBROUTINE sub (neq, time, y, dydt)
                         1
Warning: Unused dummy argument 'time' at (1) [-Wunused-dummy-argument]

Another alternative is to add support for printing %D to gfc_format_decoder, it is a matter of adding something like:

case 'D':
if (DECL_NAME (t))
{
          pp_string (pp, lang_hooks.decl_printable_name (t, 2));
          return true;
}

(In reply to Dominique d'Humieres from comment #5)
> If I understand correctly the above statement, -Wunused-parameter should not
> emit a warning for the test in comment 0: there is no parameter. The warning
> should be issued with -Wunused-dummy-argument

If so, then the ICE was not caused by my diagnostic changes, it just exposes a problem that has been latent or introduced later. Thus, I'm not planning to investigate further.

If it is possible to generate a valid warning that triggers the same ICE, then we should consider one of the options I have proposed above. From your explanation above, it seems likely that Fortran doesn't ever want to get the middle-end warning (which makes me wonder why this warning is given in the middle-end to begin with!). Thus another (ugly?) fix may be to disable the warning (all warnings?) completely around calls to finalize_function.

> If so, then the ICE was not caused by my diagnostic changes, it just exposes a
> problem that has been latent or introduced later. Thus, I'm not planning to
> investigate further.

Well, I think that is caused by your diagnostic changes, but is due to a conflicting definition of -Wunused-parameter in C* and gfortran which now share the same code (?).

I am not sure if it is too late or not to change the name of the option in one of the front end. If it is too late, is it possible for the fortran FE to rename internally the option before passing it to the ME?

(In reply to Dominique d'Humieres from comment #8)
> > If so, then the ICE was not caused by my diagnostic changes, it just exposes a
> > problem that has been latent or introduced later. Thus, I'm not planning to
> > investigate further.
> 
> Well, I think that is caused by your diagnostic changes, but is due to a
> conflicting definition of -Wunused-parameter in C* and gfortran which now
> share the same code (?).

It could be when we changed Fortran options to use the common options machinery. But that sounds strange anyway, because -Wunused-parameter would have enabled the middle-end warning in any case. I cannot find any code that disabled the warning before my changes. In fact, this same testcase in GCC 4.5.1 (way before my changes!) had the middle-end option enabled but it didn't give any warning.
 
> I am not sure if it is too late or not to change the name of the option in
> one of the front end. If it is too late, is it possible for the fortran FE
> to rename internally the option before passing it to the ME?

It could simply set:

{
 int tmp_warn_unused_parameter = warn_unused_parameter;
 warn_unused_parameter = 0
 cgraph_node::finalize_function (fndecl, true);
 warn_unused_parameter= tmp_warn_unused_parameter;
}

But it seems quite ugly to me. It would be better to understand why the middle-end thinks it is unused (perhaps Fortran needs to mark it as used or mark it with TREE_NO_WARNING?).

In fact, the exact same code in GCC 4.5.1 has TREE_NO_WARNING set in the PARAM_DECL corresponding to time, but I'm not sure where it is set.

The middle-end warning also shows that the location given by Fortran to the PARAM_DECL is very poor.

I still think this is a latent bug in the way that Fortran is generating the PARAM_DECL for time, but if the Fortran maintainers are not interested in investigating that, then I could prepare a patch with any of the work-arounds I have proposed above as soon as a Fortran maintainer (someone that will approve the patch) chooses one.

(In reply to Manuel López-Ibáñez from comment #9)
> I still think this is a latent bug in the way that Fortran is generating the
> PARAM_DECL for time, but if the Fortran maintainers are not interested in
> investigating that, then I could prepare a patch with any of the
> work-arounds I have proposed above as soon as a Fortran maintainer (someone
> that will approve the patch) chooses one.

Scratch that. Since the goal is to bluntly silence the warning, then the only work-around I see is the one proposed in the previous comment. 

(It would be interesting to know at which GCC version or revision the warning started appearing).

> (It would be interesting to know at which GCC version or revision
> the warning started appearing).

The warning for unused parameters appeared at r126486 (pr31129) and -Wunused-parameter at r126486.

-Wunused-dummy-argument appeared at r159641 (pr38407). This option corresponds to -Wunused-parameter for other FEs.

AFAICT these options are handled by the fortran FE and should not propagate to the ME.

(In reply to Dominique d'Humieres from comment #11)
> > (It would be interesting to know at which GCC version or revision
> > the warning started appearing).
> 
> The warning for unused parameters appeared at r126486 (pr31129) and
> -Wunused-parameter at r126486.

Sorry, perhaps I was not clear. I meant: when did this testcase start triggering a warning (or an ICE, whatever happened first) with -Wunused-parameter? 

As I said in comment #4, GCC 4.3.1 had this warning and the warning option was enabled for the testcase but the warning did not trigger. When did it start triggering?

> AFAICT these options are handled by the fortran FE and should not propagate
> to the ME.

Wunused-parameter is both a Fortran and a middle-end option. It is unfortunate it has a different meaning. One could argue that it should not be a middle-end option and the warning should be given only by the FEs. My intuition is that if you found a clean way to move the ME warning from cgraphunit.c to somewhere in c-family/ , such a patch is likely to be approved and it will fix this problem also.

However, all this has nothing to do with the warning triggering now, since this has been the status quo since at least GCC 4.3.1. The reason it did not warn before is that somehow the Fortran FE marked the TREE_DECL as TREE_NO_WARNING and it doesn't do this anymore.

> As I said in comment #4, GCC 4.3.1 had this warning and the warning option
> was enabled for the testcase but the warning did not trigger. When did
> it start triggering?

I don't see the warning with 4.5.4, but I see it with 4.6.4:

[macbook] f90/bug% gfortran-fsf-4.5 /opt/gcc/_clean/gcc/testsuite/gfortran.dg/warn_unused_dummy_argument_2.f90 -Wunused-parameter -c
[macbook] f90/bug% gfortran-fsf-4.6 /opt/gcc/_clean/gcc/testsuite/gfortran.dg/warn_unused_dummy_argument_2.f90 -Wunused-parameter -c
/opt/gcc/_clean/gcc/testsuite/gfortran.dg/warn_unused_dummy_argument_2.f90:7:0: warning: unused parameter 'dummy' [-Wunused-parameter]

Compiling the test gfortran.dg/warn_unused_dummy_argument_2.f90 with -Wunused-parameter gives the ICE with 5.1/6.0.

To be more precise, I dont't get the warning for 4.7 with r182107 (2011-12-08), but I get it with r182980 (2012-01-07), back ported to 4.6 between r179116 (2011-09-23) and r182981 (2012-01-07), possibly r182211 and r182213 (pr50923)

I'm testing a patch that seems to fix this in a nice way.

Author: manu
Date: Mon Jun 29 16:25:26 2015
New Revision: 225135

URL: https://gcc.gnu.org/viewcvs?rev=225135&root=gcc&view=rev
Log:
Wunused-parameter warnings are given from cgraph::finalize_function,
which is the middle-end. This is an oddity compared to other
-Wunused-* warnings. Moreover, Fortran has its own definition of
-Wunused-parameter that conflicts with the middle-end definition.

This patch moves the middle-end part of Wunused-parameter to the C/C++
FEs. I'm not sure if other FEs expected this warning to work. If so,
they do not seem to test for it. Ada, for example, explicitly disables
it.

gcc/ChangeLog:

2015-06-29  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR fortran/66605
	* cgraphunit.c (cgraph_node::finalize_function): Do not call
	do_warn_unused_parameter.
	* function.c (do_warn_unused_parameter): Move from here.
	* function.h (do_warn_unused_parameter): Do not declare.

gcc/c-family/ChangeLog:

2015-06-29  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR fortran/66605
	* c-common.c (do_warn_unused_parameter): Move here.
	* c-common.h (do_warn_unused_parameter): Declare.

gcc/ada/ChangeLog:

2015-06-29  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR fortran/66605
	* gcc-interface/misc.c (gnat_post_options): No need to disable
	warn_unused_parameter anymore.

gcc/cp/ChangeLog:

2015-06-29  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR fortran/66605
	* decl.c (finish_function): Call do_warn_unused_parameter.

gcc/testsuite/ChangeLog:

2015-06-29  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR fortran/66605
	* gfortran.dg/wunused-parameter.f90: New test.

gcc/c/ChangeLog:

2015-06-29  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR fortran/66605
	* c-decl.c (finish_function): Call do_warn_unused_parameter.

Added:
    trunk/gcc/testsuite/gfortran.dg/wunused-parameter.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ada/ChangeLog
    trunk/gcc/ada/gcc-interface/misc.c
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/c-family/c-common.h
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-decl.c
    trunk/gcc/cgraphunit.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/function.c
    trunk/gcc/function.h
    trunk/gcc/testsuite/ChangeLog

FIXED in GCC 6.

This is probably a regression, but I'm not sure if a backport would be accepted. Otherwise, one of the brute-force fixes, like the one in comment #9, should be enough to fix the ICE.

*** Bug 67467 has been marked as a duplicate of this bug. ***

*** Bug 69063 has been marked as a duplicate of this bug. ***

> FIXED in GCC 6.
>
> This is probably a regression, but I'm not sure if a backport would be
> accepted. Otherwise, one of the brute-force fixes, like the one
> in comment #9, should be enough to fix the ICE.

I am in favor to close the PR as FIXED.

Fixed on trunk (6.0), closing.