| Message ID | CAKwh3qim6+Twqx9tKuOh_w8o7MTEhYB0UKruKvh2=9UxwhkFkQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Fortran] PR 82018: -Wextra should imply -Wconversion-extra | expand |
Hi Janus, > here is a small patch that enables -Wconversion-extra with -Wextra and > updates the documentation. I grepped for warn_conversion_extra and found 14 occurrences in the gfortran source tree. Are we sure we want to enable each of these warnings with -Wextra? Regards Thomas
Hi Thomas, >> here is a small patch that enables -Wconversion-extra with -Wextra and >> updates the documentation. > > I grepped for warn_conversion_extra and found 14 occurrences in the > gfortran source tree. > > Are we sure we want to enable each of these warnings with -Wextra? I'd say: Yes, why not. They're all similar in style, it seems. Which one are you worried about, in particular? AFAICS, these warnings will essentially be triggered on compile-time constants, for which the compiler knows that they can be converted e.g. from real to integer without change in value. For such constants, I guess it is easy (and advisable) to change them. If you're writing '2.0' and assign it to an integer, then you probably just mean '2'. I'd say the warning is justified for such cases. Cheers, Janus
> As a sidenote, I made an observation that is not directly related to > the patch: The second warning in the test case, on "i4 = i8" shows > [-Wconversion] when compiled with -Wall, but [-Wconversion-extra] when > compiled with -Wextra. Does anyone understand how that inconsistency > comes about? Btw, this is probably due to constructs like: if ((warn_conversion || warn_conversion_extra) ... which occur in a few places in arith.c. In this sense, the documentation is apparently not fully correct, claiming that -Wconversion-extra does not imply -Wconversion? It probably does not do so for all warnings, but for some of them it does apparently? Maybe it would be clearer to just make -Wconversion-extra imply -Wconversion (and document that), in order to avoid confusion? Cheers, Janus
Index: gcc/fortran/invoke.texi =================================================================== --- gcc/fortran/invoke.texi (revision 252872) +++ gcc/fortran/invoke.texi (working copy) @@ -144,10 +144,10 @@ by type. Explanations are in the following sectio @item Error and Warning Options @xref{Error and Warning Options,,Options to request or suppress errors and warnings}. -@gccoptlist{-Waliasing -Wall -Wampersand -Wargument-mismatch -Warray-bounds --Wc-binding-type -Wcharacter-truncation @gol --Wconversion -Wfunction-elimination -Wimplicit-interface @gol --Wimplicit-procedure -Wintrinsic-shadow -Wuse-without-only -Wintrinsics-std @gol +@gccoptlist{-Waliasing -Wall -Wampersand -Wargument-mismatch -Warray-bounds @gol +-Wc-binding-type -Wcharacter-truncation -Wconversion -Wconversion-extra @gol +-Wextra -Wfunction-elimination -Wimplicit-interface -Wimplicit-procedure @gol +-Wintrinsic-shadow -Wuse-without-only -Wintrinsics-std @gol -Wline-truncation -Wno-align-commons -Wno-tabs -Wreal-q-constant @gol -Wsurprising -Wunderflow -Wunused-parameter -Wrealloc-lhs -Wrealloc-lhs-all @gol -Wtarget-lifetime -fmax-errors=@var{n} -fsyntax-only -pedantic -pedantic-errors @@ -884,7 +884,7 @@ the expression after conversion. Implied by @optio @cindex warnings, conversion @cindex conversion Warn about implicit conversions between different types and kinds. This -option does @emph{not} imply @option{-Wconversion}. +option does @emph{not} imply @option{-Wconversion}. Implied by @option{-Wextra}. @item -Wextra @opindex @code{Wextra} @@ -891,8 +891,8 @@ Warn about implicit conversions between different @cindex extra warnings @cindex warnings, extra Enables some warning options for usages of language features which -may be problematic. This currently includes @option{-Wcompare-reals} -and @option{-Wunused-parameter}. +may be problematic. This currently includes @option{-Wcompare-reals}, +@option{-Wunused-parameter} and @option{-Wconversion-extra}. @item -Wimplicit-interface @opindex @code{Wimplicit-interface} Index: gcc/fortran/lang.opt =================================================================== --- gcc/fortran/lang.opt (revision 252872) +++ gcc/fortran/lang.opt (working copy) @@ -234,7 +234,7 @@ Fortran Var(warn_conversion) Warning LangEnabledBy ; Documented in C Wconversion-extra -Fortran Var(warn_conversion_extra) Warning +Fortran Var(warn_conversion_extra) Warning LangEnabledBy(Fortran,Wextra) Warn about most implicit conversions. Wextra
Hi all, here is a small patch that enables -Wconversion-extra with -Wextra and updates the documentation. Up to gfortran 5, the warning in the test case was enabled by -Wconversion, and thus with -Wall. That was changed by Thomas in r224190, which makes complete sense to me. However, I think -Wconversion-extra should be implied by -Wextra (I'm sure not everyone wants to see such messages, but I think they're still useful). The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk? As a sidenote, I made an observation that is not directly related to the patch: The second warning in the test case, on "i4 = i8" shows [-Wconversion] when compiled with -Wall, but [-Wconversion-extra] when compiled with -Wextra. Does anyone understand how that inconsistency comes about? Cheers, Janus 2017-09-16 Janus Weil <janus@gcc.gnu.org> PR fortran/82018 * lang.opt: Turn on -Wconversion-extra with -Wextra. * invoke.texi: Update documentation of -Wconversion-extra and -Wextra and mention both in the list of Error and Warning Options. 2017-09-16 Janus Weil <janus@gcc.gnu.org> PR fortran/82018 * gfortran.dg/wextra_2.f90: New test case.