[Fortran] Optionally suppress no-automatic overwrites recursive warning - for review

Message ID 0ddfc9df-384a-98eb-f950-57705d1a5faf@codethink.co.uk
State New
Headers show
Series
  • [Fortran] Optionally suppress no-automatic overwrites recursive warning - for review
Related show

Commit Message

Mark Eggleston Sept. 19, 2019, 1:40 p.m.
The following warning is produced when -fno-automatic and -frecursive 
are used at the same time:

f951: Warning: Flag '-fno-automatic' overwrites '-frecursive'

This is a reasonable warning, however, recursion can be used with the 
-fno-automatic flag where recursion is enabled using -frecusive instead 
of explicitly using the recursive keyword.

If all relevant subroutines local variables are explicitly declared 
automatic then recursion will work and the warning becomes a nuisance 
when -Werror is used in the build environment.

This patch allows the warning to be switched off using a new option, 
-Woverwrite-recursive, initialised to on.

I don't have a test case for this as I don't know how to test for a 
warning that isn't related to a line of code.

It is a very simple patch, can the test case be omitted?

ChangeLog

gcc/fortran

     Mark Eggleston  <mark.eggleston@codethink.com>

     * lang.opt: Add option -Woverwrite-recursive initialised as on.
     * option.c (gfc_post_options): Output warning only if it is enabled.

-- 
https://www.codethink.co.uk/privacy.html

Comments

Tobias Burnus Sept. 19, 2019, 3:46 p.m. | #1
Hi Mark,

On 9/19/19 3:40 PM, Mark Eggleston wrote:
> The following warning is produced when -fno-automatic and -frecursive 

> are used at the same time:

>

> f951: Warning: Flag '-fno-automatic' overwrites '-frecursive'

>

> This patch allows the warning to be switched off using a new option, 

> -Woverwrite-recursive, initialised to on.

>

> I don't have a test case for this as I don't know how to test for a 

> warning that isn't related to a line of code.


Try:

! { dg-warning "Flag .-fno-automatic. overwrites .-frecursive." "" { 
target *-*-* } 0 }

The syntax is { dg-warning "message", "label" {target ...} linenumber }, 
where linenumber = 0 means it can be on any line.

If the output doesn't match (but I think it does with "Warning:"), 
general messages can be caught with "dg-message".


Thanks,

Tobias
Bernhard Reutner-Fischer Sept. 20, 2019, 6:46 a.m. | #2
On Thu, 19 Sep 2019 17:46:29 +0200
Tobias Burnus <tobias@codesourcery.com> wrote:

> Hi Mark,

> 

> On 9/19/19 3:40 PM, Mark Eggleston wrote:

> > The following warning is produced when -fno-automatic and -frecursive 

> > are used at the same time:

> >

> > f951: Warning: Flag '-fno-automatic' overwrites '-frecursive'

> >

> > This patch allows the warning to be switched off using a new option, 

> > -Woverwrite-recursive, initialised to on.

> >

> > I don't have a test case for this as I don't know how to test for a 

> > warning that isn't related to a line of code.  

> 

> Try:

> 

> ! { dg-warning "Flag .-fno-automatic. overwrites .-frecursive." "" { 

> target *-*-* } 0 }

> 

> The syntax is { dg-warning "message", "label" {target ...} linenumber }, 

> where linenumber = 0 means it can be on any line.

> 

> If the output doesn't match (but I think it does with "Warning:"), 

> general messages can be caught with "dg-message".


Also:

> @@ -411,7 +411,7 @@ gfc_post_options (const char **pfilename)

>        && flag_max_stack_var_size != 0)

>      gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-fmax-stack-var-size=%d%>",

>  		     flag_max_stack_var_size);

> -  else if (!flag_automatic && flag_recursive)

> +  else if (!flag_automatic && flag_recursive && warn_overwrite_recursive)

>      gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%>");

>    else if (!flag_automatic && flag_openmp)

>      gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> implied by "

> 


Doesn't look right to me. Do you want
gfc_warning_now (OPT_Woverwrite_recursive, "Flag ...
instead?

thanks,
Mark Eggleston Sept. 24, 2019, 11:12 a.m. | #3
On 20/09/2019 07:46, Bernhard Reutner-Fischer wrote:
> On Thu, 19 Sep 2019 17:46:29 +0200

> Tobias Burnus <tobias@codesourcery.com> wrote:

>

>> Hi Mark,

>>

>> On 9/19/19 3:40 PM, Mark Eggleston wrote:

>>> The following warning is produced when -fno-automatic and -frecursive

>>> are used at the same time:

>>>

>>> f951: Warning: Flag '-fno-automatic' overwrites '-frecursive'

>>>

>>> This patch allows the warning to be switched off using a new option,

>>> -Woverwrite-recursive, initialised to on.

>>>

>>> I don't have a test case for this as I don't know how to test for a

>>> warning that isn't related to a line of code.

>> Try:

>>

>> ! { dg-warning "Flag .-fno-automatic. overwrites .-frecursive." "" {

>> target *-*-* } 0 }

>>

>> The syntax is { dg-warning "message", "label" {target ...} linenumber },

>> where linenumber = 0 means it can be on any line.

Thanks that was the bit I was missing. Test cases now added.
>>

>> If the output doesn't match (but I think it does with "Warning:"),

>> general messages can be caught with "dg-message".

> Also:

>

>> @@ -411,7 +411,7 @@ gfc_post_options (const char **pfilename)

>>         && flag_max_stack_var_size != 0)

>>       gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-fmax-stack-var-size=%d%>",

>>   		     flag_max_stack_var_size);

>> -  else if (!flag_automatic && flag_recursive)

>> +  else if (!flag_automatic && flag_recursive && warn_overwrite_recursive)

>>       gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%>");

>>     else if (!flag_automatic && flag_openmp)

>>       gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> implied by "

>>

> Doesn't look right to me. Do you want

> gfc_warning_now (OPT_Woverwrite_recursive, "Flag ...

> instead?

Done.
>

> thanks,


Additionally I realised that I hadn't updated the manual.

Updated patch is attached.

Updated change log:

gcc/fortran

     Mark Eggleston <mark.eggleston@codethink.com>

     * invoke.texi: Add -Wno-overwrite-recursive to list of options. Add
     description of -Wno-overwrite-recursive. Fix typo in description
     of -Winteger-division.
     * lang.opt: Add option -Woverwrite-recursive initialised as on.
     * option.c (gfc_post_options): Output warning only if it is enabled.

gcc/testsuite

     Mark Eggleston <mark.eggleston@codethink.com>

     * gfortran.dg/no_overwrite_recursive_1.f90: New test.
     * gfortran.dg/no_overwrite_recursive_2.f90: New test.

OK to commit?

Mark



-- 
https://www.codethink.co.uk/privacy.html
From e3c40212a648d9fbf8ea33525a943cc85e0652b0 Mon Sep 17 00:00:00 2001
From: Mark Eggleston <markeggleston@codethink.com>

Date: Tue, 16 Apr 2019 09:09:12 +0100
Subject: [PATCH] Suppress warning with -Wno-overwrite-recursive

The message "Warning: Flag '-fno-automatic' overwrites '-frecursive'" is
output by default when -fno-automatic and -frecursive are used together.
It warns that recursion may be broken, however if all the relavent variables
in the recursive procedure have automatic attributes the warning is
unnecessary so -Wno-overwrite-recursive can be used to suppress it. This
will allow compilation when warnings are regarded as errors.

Suppress warning with -Wno-overwrite-recursive
---
 gcc/fortran/invoke.texi                              | 20 +++++++++++++++-----
 gcc/fortran/lang.opt                                 |  4 ++++
 gcc/fortran/options.c                                |  5 +++--
 .../gfortran.dg/no_overwrite_recursive_1.f90         | 11 +++++++++++
 .../gfortran.dg/no_overwrite_recursive_2.f90         | 10 ++++++++++
 5 files changed, 43 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/no_overwrite_recursive_1.f90
 create mode 100644 gcc/testsuite/gfortran.dg/no_overwrite_recursive_2.f90

diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
index 40eeadd00db..b60749b3755 100644
--- a/gcc/fortran/invoke.texi
+++ b/gcc/fortran/invoke.texi
@@ -149,10 +149,11 @@ and warnings}.
 -Wc-binding-type -Wcharacter-truncation -Wconversion @gol
 -Wdo-subscript -Wfunction-elimination -Wimplicit-interface @gol
 -Wimplicit-procedure -Wintrinsic-shadow -Wuse-without-only @gol
--Wintrinsics-std -Wline-truncation -Wno-align-commons -Wno-tabs @gol
--Wreal-q-constant -Wsurprising -Wunderflow -Wunused-parameter @gol
--Wrealloc-lhs -Wrealloc-lhs-all -Wfrontend-loop-interchange @gol
--Wtarget-lifetime -fmax-errors=@var{n} -fsyntax-only -pedantic @gol
+-Wintrinsics-std -Wline-truncation -Wno-align-commons @gol
+-Wno-overwrite-recursive -Wno-tabs -Wreal-q-constant -Wsurprising @gol
+-Wunderflow -Wunused-parameter -Wrealloc-lhs -Wrealloc-lhs-all @gol
+-Wfrontend-loop-interchange -Wtarget-lifetime -fmax-errors=@var{n} @gol
+-fsyntax-only -pedantic @gol
 -pedantic-errors @gol
 }
 
@@ -997,7 +998,7 @@ nor has been declared as @code{EXTERNAL}.
 @opindex @code{Winteger-division}
 @cindex warnings, integer division
 @cindex warnings, division of integers
-Warn if a constant integer division truncates it result.
+Warn if a constant integer division truncates its result.
 As an example, 3/5 evaluates to 0.
 
 @item -Wintrinsics-std
@@ -1010,6 +1011,15 @@ it as @code{EXTERNAL} procedure because of this.  @option{-fall-intrinsics} can
 be used to never trigger this behavior and always link to the intrinsic
 regardless of the selected standard.
 
+@item -Wno-overwrite-recursive
+@opindex @code{Woverwrite-recursive}
+@cindex  warnings, overwrite recursive
+Do not warn when @option{-fno-automatic} is used with @option{-frecursive}. Recursion
+will be broken if the relevant local variables do not have the attribute
+@code{AUTOMATIC} explicitly declared. This option can be used to suppress the warning
+when it is known that recursion is not broken. Useful for build environment that use
+@option{-Werror}.
+
 @item -Wreal-q-constant
 @opindex @code{Wreal-q-constant}
 @cindex warnings, @code{q} exponent-letter
diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index 767725c5801..015da85a4a4 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -293,6 +293,10 @@ Wopenmp-simd
 Fortran
 ; Documented in C
 
+Woverwrite-recursive
+Fortran Warning Var(warn_overwrite_recursive) Init(1)
+Warn that -fno-automatic may break recursion.
+
 Wpedantic
 Fortran
 ; Documented in common.opt
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index a0f95fa8832..8c38a493dc8 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -418,8 +418,9 @@ gfc_post_options (const char **pfilename)
       && flag_max_stack_var_size != 0)
     gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-fmax-stack-var-size=%d%>",
 		     flag_max_stack_var_size);
-  else if (!flag_automatic && flag_recursive)
-    gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%>");
+  else if (!flag_automatic && flag_recursive && warn_overwrite_recursive)
+    gfc_warning_now (OPT_Woverwrite_recursive, "Flag %<-fno-automatic%> "
+		     "overwrites %<-frecursive%>");
   else if (!flag_automatic && flag_openmp)
     gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> implied by "
 		     "%<-fopenmp%>");
diff --git a/gcc/testsuite/gfortran.dg/no_overwrite_recursive_1.f90 b/gcc/testsuite/gfortran.dg/no_overwrite_recursive_1.f90
new file mode 100644
index 00000000000..f12c4b8a4c7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/no_overwrite_recursive_1.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! { dg-options "-fno-automatic -frecursive" }
+!
+! Test case contributed by Mark Eggleston  <mark.eggleston@codethink.com>
+!
+
+program test
+  ! do nothing
+end program
+
+! { dg-warning "Flag '-fno-automatic' overwrites '-frecursive'" "warning" { target *-*-* } 0 } 
diff --git a/gcc/testsuite/gfortran.dg/no_overwrite_recursive_2.f90 b/gcc/testsuite/gfortran.dg/no_overwrite_recursive_2.f90
new file mode 100644
index 00000000000..09445375481
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/no_overwrite_recursive_2.f90
@@ -0,0 +1,10 @@
+! { dg-do compile }
+! { dg-options "-fno-automatic -frecursive -Wno-overwrite-recursive" }
+!
+! Test case contributed by Mark Eggleston  <mark.eggleston@codethink.com>
+!
+
+program test
+  ! do nothing
+end program
+
-- 
2.11.0
Bernhard Reutner-Fischer Sept. 24, 2019, 1:53 p.m. | #4
On Tue, 24 Sep 2019 12:12:04 +0100
Mark Eggleston <mark.eggleston@codethink.co.uk> wrote:

> >> @@ -411,7 +411,7 @@ gfc_post_options (const char **pfilename)

> >>         && flag_max_stack_var_size != 0)

> >>       gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-fmax-stack-var-size=%d%>",

> >>   		     flag_max_stack_var_size);

> >> -  else if (!flag_automatic && flag_recursive)

> >> +  else if (!flag_automatic && flag_recursive && warn_overwrite_recursive)

> >>       gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%>");

> >>     else if (!flag_automatic && flag_openmp)

> >>       gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> implied by "

> >>

> > Doesn't look right to me. Do you want

> > gfc_warning_now (OPT_Woverwrite_recursive, "Flag ...

> > instead?

> Done.


by "instead" i mean you to leave the if unchanged.

thanks,
Mark Eggleston Sept. 24, 2019, 2:11 p.m. | #5
On 24/09/2019 14:53, Bernhard Reutner-Fischer wrote:
> On Tue, 24 Sep 2019 12:12:04 +0100

> Mark Eggleston <mark.eggleston@codethink.co.uk> wrote:

>

>>>> @@ -411,7 +411,7 @@ gfc_post_options (const char **pfilename)

>>>>          && flag_max_stack_var_size != 0)

>>>>        gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-fmax-stack-var-size=%d%>",

>>>>    		     flag_max_stack_var_size);

>>>> -  else if (!flag_automatic && flag_recursive)

>>>> +  else if (!flag_automatic && flag_recursive && warn_overwrite_recursive)

>>>>        gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%>");

>>>>      else if (!flag_automatic && flag_openmp)

>>>>        gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> implied by "

>>>>

>>> Doesn't look right to me. Do you want

>>> gfc_warning_now (OPT_Woverwrite_recursive, "Flag ...

>>> instead?

>> Done.

> by "instead" i mean you to leave the if unchanged.

I didn't realise that's how it worked. That's cleaner. Once fixed OK for 
commit?
>

> thanks,

>

-- 
https://www.codethink.co.uk/privacy.html
Bernhard Reutner-Fischer Sept. 24, 2019, 6:39 p.m. | #6
On Tue, 24 Sep 2019 15:11:43 +0100
Mark Eggleston <mark.eggleston@codethink.co.uk> wrote:

> I didn't realise that's how it worked. That's cleaner. Once fixed OK for 

> commit?


> +@item -Wno-overwrite-recursive

> +@opindex @code{Woverwrite-recursive}

> +@cindex  warnings, overwrite recursive

> +Do not warn when @option{-fno-automatic} is used with @option{-frecursive}. Recursion

> +will be broken if the relevant local variables do not have the attribute

> +@code{AUTOMATIC} explicitly declared. This option can be used to suppress the warning

> +when it is known that recursion is not broken. Useful for build environment that use

> +@option{-Werror}.


I'm not a native speaker, but i would expect environment in plural.
With that nit fixed i have no further comments, i.e. looks fine but i
cannot approve it.

thanks,

Patch

From cfdcde8229ebed15304c1adb8085365f8ad5970a Mon Sep 17 00:00:00 2001
From: Mark Eggleston <markeggleston@codethink.com>
Date: Tue, 16 Apr 2019 09:09:12 +0100
Subject: [PATCH 09/12] Suppress warning with -Wno-overwrite-recursive

The message "Warning: Flag '-fno-automatic' overwrites '-frecursive'" is
output by default when -fno-automatic and -frecursive are used together.
It warns that recursion may be broken, however if all the relavent variables
in the recursive procedure have automatic attributes the warning is
unnecessary so -Wno-overwrite-recursive can be used to suppress it. This
will allow compilation when warnings are regarded as errors.

Suppress warning with -Wno-overwrite-recursive
---
 gcc/fortran/lang.opt  | 4 ++++
 gcc/fortran/options.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index 2aba57d00b5..7d9fd3e048c 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -289,6 +289,10 @@  Wopenmp-simd
 Fortran
 ; Documented in C
 
+Woverwrite-recursive
+Fortran Warning Var(warn_overwrite_recursive) Init(1)
+Warn that -fno-automatic may break recursion.
+
 Wpedantic
 Fortran
 ; Documented in common.opt
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index 03e14338578..1721f93f673 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -411,7 +411,7 @@  gfc_post_options (const char **pfilename)
       && flag_max_stack_var_size != 0)
     gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-fmax-stack-var-size=%d%>",
 		     flag_max_stack_var_size);
-  else if (!flag_automatic && flag_recursive)
+  else if (!flag_automatic && flag_recursive && warn_overwrite_recursive)
     gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%>");
   else if (!flag_automatic && flag_openmp)
     gfc_warning_now (0, "Flag %<-fno-automatic%> overwrites %<-frecursive%> implied by "
-- 
2.11.0