Argument-mismatch fallout (was: Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval)
Hi Jeff, hi Thomas, hi all, I had a look at Wannier90. (Fedora uses Version 2.0.0 of 2013, 2.0.1 was released 2015; current is 3.0 of Feb 2019 and does build.) I think that problem in Wannier90 it typical for all failing code, although there are likely a few other failures. Namely, That code uses something like: subroutine sub(array, n) integer array(*) integer N end subroutine with the call: integer :: scalar call sub(scalar, 1) That fails with: Error: Rank mismatch in argument ‘array’ at (1) (rank-1 and scalar) That's a scalar actual argument is passed to an array dummy argument. Technically, the error is correct and the code invalid: F2018, 15.5.2.4, paragraph 14 has: "If the actual argument is a … scalar, the corresponding dummy argument shall be scalar unless … the actual argument … is an element … of an array." Fortran 66, https://wg5-fortran.org/ARCHIVE/Fortran66.pdf , [8.4.2, ll. 33-35] "If a dummy argument of an external function is an array name, the corresponding actual argument must be an array name or array element name (10.1.3)" Fortran IV, p. 104, http://web.eah-jena.de/~kleine/history/languages/GC28-6515-10-FORTRAN-IV-Language.pdf "If a dummy argument is an array, the corresponding actual argument must be (1) an array, or (2) an array element." [Fortran IV only permitted literal constants in 'dimension(123)' and 'dimension(N)' for dummy arguments, i.e. the size is variable and provided by the callee. [Number of ranks not specified, the linked specification supports rank-7 arrays.] Fortran 66 seems to be likewise, and specifies only rank-4 arrays. Fortran 77 has finally assumed-size arrays (which seem to be backwards, 'dimension(N)' I regard as better than 'dimension(*)'), supports other lower bounds than 1, has rank-7 arrays and a few additional array extensions to 66.] Current result: * Error with -std=gnu (the default) or -std=f95/f2003/f… * Downgraded to a warning with either -std=legacy or -fallow-argument-mismatch However, passing a scalar instead of an array/array element worked/works with (nearly?) all compilers. Hence, passing a scalar is seemingly common pattern. Thus, I wonder whether we should do something about this. Ideas/possibilities: * Use warning instead of an error with -std=gnu (and only error for std=f…) * Keep the error for most mismatches but downgrade to a warning if -std=gnu for (only) the scalar-to-array case. (a) do this always or (b) only for assumes-size dummy args. * (a) Keep the status quo and require -std=legacy/ -fallow-argument-mismatch to get it compiled. (b) modify the error message such that the user knows about the arg-mismatch flag? * Do something else? Ideas? Comments? Opinions? Cheers, Tobias On 10/25/19 4:48 PM, Jeff Law wrote: […]the function argument stuff broke 30-40 packages, many of which still don't build without -fallow-argument-mismatch. Do you know whether those 30–40 packages have code bugs or could there be gfortran bugs (too strict checking) lurking? I'm not familiar enough with the issue & packages to know if they're cases of source bugs or gfortran being too strict. My plan has always been to extract a few cases and pass them along for that kind of analysis. I've just been too busy lately with other regressions :( A partial list of the affected packages: R-deldir R atlas cgnslib cp2k elk elpa exciting ga getdata grib_api hdf libccp4 mpich hwchem psblas3 qrmumps qrupdate quantum-espresso scalapack scipy scorep wannier90 wsjtx xfoil xrotor There's certainly more, that list just represents those I've locally worked around with -fallow-argument-mismatch. Several more trigger the mismatch error, but I haven't bothered working around yet. That list comes from _after_ the Oct 14 patch to correct issues in the argument mismatch testing. Regarding the BOZ: One difference to the argument mismatch is that the latter has an option to still accept it (-fallow-argument-mismatch) and potentially generates wrong code – depending what the ME does with the mismatches – while the former, once parsed, causes no potential ME problems and as there is no flag, it always requires code changes. (On the other hand, fixing the BOZ issue is straight forward; argument changes are trickier.) Absolutely. That's the primary reason why I haven't contacted the affected package maintainers yet -- I don't want them blindly adding -fallow-argument-mismatch to their flags. Jeff
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
Hi Steve, On 10/25/19 4:17 PM, Steve Kargl wrote: My BOZ patch brought gfortran closer to an actual comforming Fortran compiler while providing an option that would allow quite a few documented and undocumented extensions. If the patch broke some of your code, and -fallow-invalid-boz did not allow the code to compile Actually, it does allow it to compile – but I missed the option; -std=legacy used to be sufficient. For the code at hand, it was actually better to nag the author to change the code to valid Fortran than to fiddle around with some options. But I think that point to a problem: Some compiler flags do exist, but it is are hard to find them (especially, if one does not know that they do exist). … I am not claiming that it should be done in this case, but sometimes something like: "Error: BOZ literal constant at (1) is neither a DATA statement value nor an actual argument of INT/REAL/DBLE/CMPLX intrinsic subprogram; _for legacy code, -fallow-invalid-boz might be an option"_ would have surely helped.** Side note: to me, "the … intrinsic function" is clearer than "… intrinsic subprogram". and you were forced to use INT(, kind=) to get it to compile Well, using INT() was the way in order to get the code standard conforming :-) As you you quoted: C410 (R411) A boz-literal-constant shall appear only as a data-stmt-constant in a DATA statement, as the actual argument associated with the dummy argument A of the numeric intrinsic functions DBLE, REAL or INT, or as the actual argument associated with the X or Y dummy argument of the intrinsic function CMPLX. Cheers, Tobias **This does not mean that users will always read this. (I get this error, what shall I do?) Nor that fixing the code would be usually be the better option. (As in my case.)
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
On 10/25/19 7:54 AM, Tobias Burnus wrote: > Hi Jeff, > > On 10/25/19 3:22 PM, Jeff Law wrote: >> So across Fedora the BOZ stuff tripped 2-3 packages. In comparison the >> function argument stuff broke 30-40 packages, many of which still >> don't build without -fallow-argument-mismatch. > > Regarding the latter: > The initial patch was too strict – an also rejected valid code > (according to the Fortran 2018 standard). That was my understanding from loosely following the threads. That has been fixed.* – Thus, > either some valid cases were missed (gfortran bug) or all those packages > indeed have an argument mismatch. > > *That fix is: 2019-10-14 / r276972 / PR fortran/92004 / > https://gcc.gnu.org/ml/fortran/2019-10/msg00128.html Yea. That patch certainly helped lapack and others. > > Do you know whether those 30–40 packages have code bugs or could there > be gfortran bugs (too strict checking) lurking? I'm not familiar enough with the issue & packages to know if they're cases of source bugs or gfortran being too strict. My plan has always been to extract a few cases and pass them along for that kind of analysis. I've just been too busy lately with other regressions :( A partial list of the affected packages: R-deldir R atlas cgnslib cp2k elk elpa exciting ga getdata grib_api hdf libccp4 mpich hwchem psblas3 qrmumps qrupdate quantum-espresso scalapack scipy scorep wannier90 wsjtx xfoil xrotor There's certainly more, that list just represents those I've locally worked around with -fallow-argument-mismatch. Several more trigger the mismatch error, but I haven't bothered working around yet. That list comes from _after_ the Oct 14 patch to correct issues in the argument mismatch testing. > > > Regarding the BOZ: One difference to the argument mismatch is that the > latter has an option to still accept it (-fallow-argument-mismatch) and > potentially generates wrong code – depending what the ME does with the > mismatches – while the former, once parsed, causes no potential ME > problems and as there is no flag, it always requires code changes. (On > the other hand, fixing the BOZ issue is straight forward; argument > changes are trickier.) Absolutely. That's the primary reason why I haven't contacted the affected package maintainers yet -- I don't want them blindly adding -fallow-argument-mismatch to their flags. Jeff
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
On Fri, Oct 25, 2019 at 10:35:24AM +0200, Tobias Burnus wrote: > On 9/26/19 10:45 AM, Mark Eggleston wrote: > > PS: I was also not that happy about the BOZ changes by Steve, which > broke code here – but, fortunately, adding int( ,kind=) around it was > sufficient and that code was supposed to be F2003 standard conforming. > I could ping the authors and is now fixed. Still, I wonder how much code > broke due to that change; code is not that simple to fix. – But, in > general, I am very much in favour in having valid Fortran 2018 code (can > be fixed form, old and use old features, that's fine). > My BOZ patch brought gfortran closer to an actual comforming Fortran compiler while providing an option that would allow quite a few documented and undocumented extensions. If the patch broke some of your code, and -fallow-invalid-boz did not allow the code to compile, and you were forced to use INT(, kind=) to get it to compile, then, no, the code was not conforming. And since you mention F2003, for the record C410 (R411) A boz-literal-constant shall appear only as a data-stmt-constant in a DATA statement, as the actual argument associated with the dummy argument A of the numeric intrinsic functions DBLE, REAL or INT, or as the actual argument associated with the X or Y dummy argument of the intrinsic function CMPLX. -- Steve
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
Hi Jeff, On 10/25/19 3:22 PM, Jeff Law wrote: So across Fedora the BOZ stuff tripped 2-3 packages. In comparison the function argument stuff broke 30-40 packages, many of which still don't build without -fallow-argument-mismatch. Regarding the latter: The initial patch was too strict – an also rejected valid code (according to the Fortran 2018 standard). That has been fixed.* – Thus, either some valid cases were missed (gfortran bug) or all those packages indeed have an argument mismatch. *That fix is: 2019-10-14 / r276972 / PR fortran/92004 / https://gcc.gnu.org/ml/fortran/2019-10/msg00128.html Do you know whether those 30–40 packages have code bugs or could there be gfortran bugs (too strict checking) lurking? Regarding the BOZ: One difference to the argument mismatch is that the latter has an option to still accept it (-fallow-argument-mismatch) and potentially generates wrong code – depending what the ME does with the mismatches – while the former, once parsed, causes no potential ME problems and as there is no flag, it always requires code changes. (On the other hand, fixing the BOZ issue is straight forward; argument changes are trickier.) Cheers, Tobias
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
On 10/25/19 2:35 AM, Tobias Burnus wrote: > On 9/26/19 10:45 AM, Mark Eggleston wrote: >> Original thread starts here >> https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01185.html >> OK to commit? > > As Steve, I am not really happy about adding yet another option and > especially not about legacy features. On the other hand, I see that > legacy code is still used. > > Having said this, the patch is OK from my side. > > Tobias > > PS: I was also not that happy about the BOZ changes by Steve, which > broke code here – but, fortunately, adding int( ,kind=) around it was > sufficient and that code was supposed to be F2003 standard conforming. > I could ping the authors and is now fixed. Still, I wonder how much code > broke due to that change; code is not that simple to fix. – But, in > general, I am very much in favour in having valid Fortran 2018 code (can > be fixed form, old and use old features, that's fine). So across Fedora the BOZ stuff tripped 2-3 packages. In comparison the function argument stuff broke 30-40 packages, many of which still don't build without -fallow-argument-mismatch. jeff
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
On 9/26/19 10:45 AM, Mark Eggleston wrote: Original thread starts here https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01185.html OK to commit? As Steve, I am not really happy about adding yet another option and especially not about legacy features. On the other hand, I see that legacy code is still used. Having said this, the patch is OK from my side. Tobias PS: I was also not that happy about the BOZ changes by Steve, which broke code here – but, fortunately, adding int( ,kind=) around it was sufficient and that code was supposed to be F2003 standard conforming. I could ping the authors and is now fixed. Still, I wonder how much code broke due to that change; code is not that simple to fix. – But, in general, I am very much in favour in having valid Fortran 2018 code (can be fixed form, old and use old features, that's fine). gcc/fortran/ChangeLog Mark Eggleston * 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/ChangeLog Mark Eggleston * gfortran.dg/no_overwrite_recursive_1.f90: New test. * gfortran.dg/no_overwrite_recursive_2.f90: New test.
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
PING - OK to commit? On 02/10/2019 09:00, Mark Eggleston wrote: On 28/09/2019 17:50, Steve Kargl wrote: On Thu, Sep 26, 2019 at 09:45:28AM +0100, Mark Eggleston wrote: Original thread starts here https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01185.html OK to commit? I'm not a big fan of option proliferation. If you don't want to see warns just use -w. Of course, this is just MHO. Unfortunately -w switches off ALL warnings. -- https://www.codethink.co.uk/privacy.html
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
On 28/09/2019 17:50, Steve Kargl wrote: On Thu, Sep 26, 2019 at 09:45:28AM +0100, Mark Eggleston wrote: Original thread starts here https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01185.html OK to commit? I'm not a big fan of option proliferation. If you don't want to see warns just use -w. Of course, this is just MHO. Unfortunately -w switches off ALL warnings. -- https://www.codethink.co.uk/privacy.html
Re: [PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
On Thu, Sep 26, 2019 at 09:45:28AM +0100, Mark Eggleston wrote: > Original thread starts here > https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01185.html > > OK to commit? > I'm not a big fan of option proliferation. If you don't want to see warns just use -w. Of course, this is just MHO. -- Steve
[PATCH, Fortran] Optionally suppress no-automatic overwrites recursive warning - for approval
Original thread starts here
https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01185.html
OK to commit?
gcc/fortran/ChangeLog
Mark Eggleston
* 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/ChangeLog
Mark Eggleston
* gfortran.dg/no_overwrite_recursive_1.f90: New test.
* gfortran.dg/no_overwrite_recursive_2.f90: New test.
--
https://www.codethink.co.uk/privacy.html
>From 30d7915ce44629edc85dba210cca9007d1c70d02 Mon Sep 17 00:00:00 2001
From: Mark Eggleston
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| 3 ++-
.../gfortran.dg/no_overwrite_recursive_1.f90 | 11 +++
.../gfortran.dg/no_overwrite_recursive_2.f90 | 10 ++
5 files changed, 42 insertions(+), 6 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 b911ac90018..0177fb400a1 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
}
@@ -995,7 +996,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
@@ -1008,6 +1009,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 environments 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 b8bf7fe7508..1b296c92d19 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..47042a25eab 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -419,7 +419,8 @@ gfc_post_options (const char **pfilename)
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%>");
+gfc_warning_now (OPT_Woverwrite_recursive, "Flag %<-fno-automatic%> "
+ "overwrites %<-frecursive%>");
else if
