This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
Re: Explosion of noise from -Wall
On 08/22/2012 10:04 AM, N.M. Maclaren wrote:
On Aug 21 2012, Steve Kargl wrote:
While I can appreciate the value of the new warnings
recently added to gfortran, it seems the automatic
inclusion of these options when -Wall is used causes
an explosion of noise.
One could also consider to move some to -Wextra. Still, the advantage of
-Wall is that users usually do not bother finding other -W* options.
I think we should do the same as the middle end does: Print the flag in
square brackets to make it easier to disable a single warning. For
instance, -Wall -Wno-conversion should work. (That's independent which
flag gets set when.)
-Wall at one time was usable to find questionable code. Now one needs
to do '-Wall -Wno-conversion -Wno-compare-real'
to silence a large amount of false positives. I don't
have CP2K or Damian's code available, it may be interesting
to see what happens with a fairly modern codebase.
I disagree in the case of the conversions. I have just run a
check on which ones it warns about, and they are all common
causes of serious errors. Better languages, like Algol 68,
made implicit narrowing coercions an error - so you would HAVE
to fix them. The only reason that any reasonable Fortran
programmer ever uses them is because of the utterly ghastly
specification of the INT, REAL and COMPLEX intrinsics.
And using -Wconversion-extra one gets a huge amount of warnings. I
really like that option; it is useful to run it once in a while as it
helps finding bugs. (But it is good that it is not enabled by default.)
On our not-that-clean Fortran 77 converted to Fortran 2003 code
(www.flapw.de), I get 104 warnings for complex(8)->real(8); 83 for
real(8)->integer(4), 25 for real(16)->real(8) [due to -fdefault-real-8
and using "1.0d0"] and 3 for real(16)->integer(4).
That's with 117479 lines in 491 files.
Real and complex comparison is more moot, and there is a good
case for putting it in -Wextra. While 90% of such cases are
questionable code, there are a fair number of reasonable uses.
Here, I get 93 warnings on our code. Most comparisons are of the form:
IF (x.NE.0.0) THEN
which should be kind of okay: 0.0 is well representable as
floating-point number and also algorithmicly it looks reasonable.
Other uses are:
IF (INT(number) == number) THEN
IF (t.EQ.-3) type = 'I '
(magic number in the input file) or an overflow test of the form
IF ( 2.0 * test == test ) test = cmplx(0.0,0.0)
In another code (which is written in a bit more object-oriented way;
consisting of 360 files and 160458 lines; www.tddft.org), I get 130
comparison warnings of which 108 are comparisons against 0.0.
Regarding the conversion, I get 215 warnings:
147 Warning: Possible change of value in conversion from COMPLEX(8)
to REAL(8) at (1)
35 Warning: Possible change of value in conversion from REAL(8) to
INTEGER(4) at (1)
12 Warning: Possible change of value in conversion from INTEGER(8)
to REAL(4) at (1)
12 Warning: Possible change of value in conversion from INTEGER(8)
to INTEGER(4) at (1)
5 Warning: Possible change of value in conversion from REAL(8) to
REAL(4) at (1)
2 Warning: Possible change of value in conversion from COMPLEX(8)
to INTEGER(4) at (1)
1 Warning: Possible change of value in conversion from REAL(4) to
INTEGER(4) at (1)
1 Warning: Possible change of value in conversion from COMPLEX(8)
to COMPLEX(4) at (1)
The same applies to underflow, for very similar reasons, and the
trapping of that lost out in the 1970s.
Well, underflow with subnormal numbers might be still something one
should try to avoid as on some systems, those are significantly slower
than other numbers - I think that was especially a problem on Alpha, but
I might misremember. (That's the reason for the code above which tries
to set denormalized numbers to 0.0.)
For both -Wconversion and -Wcompare-real, I am fine with a patch which
moves them from -Wall to -Wextra. The problem is that both of them are
good tools to find bugs, but both of them show many false positives -
either for nicely written code or for properly working but lazily
written code. On the other hand, flags which are not in -Wall are kind
of invisible.
Regarding -Wcompare-real, I wonder whether it makes sense to either
ignore comparisions against zero or to put them into a different flag
(-Wcompare-real-zero); those comparisons seem most of the time perfectly
valid and replacing them with "abs(a-b) < eps" is usually worse. On the
other hand, if -Wcompare-real is in -Wextra, one could also leave in the
zero-comparison warnings.
Tobias