RFR: JDK-8211073 Remove -Wno-extra from Hotspot
Kim Barrett
kim.barrett at oracle.com
Thu Sep 27 14:55:20 UTC 2018
> On Sep 24, 2018, at 4:31 PM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
>
> The -Wextra option to gcc enables a bunch of useful warnings.[1] Some of them, but not all, can be individually enabled or disabled. All other libraries in OpenJDK are compiled with -Wextra, but not Hotspot. Enabling -Wextra on Hotspot triggers a couple of warnings for zero that can be individually disabled.
>
> However, -Wextra also includes some check that cannot be disabled individually, so to be able to add this, we must at the same time fix those warnings.
>
> The warnings that cannot be disabled and which have been triggered in Hotspot is "enumeral and non-enumeral type in conditional expression" and "base class should be explicitly initialized in the copy constructor". The former complains about mixing enums and integers in the tertiary operator (x ? enum_val : int_val). If you think that gcc is a bit too picky here, I agree. It's not obvious per se that the added casts improve the code. However, this is the price we need to pay to be able to enable -Wextra, and *that* is something that is likely to improve the code.
>
> The second warning about the copy constructor is, for what I can tell, a highly valid warning and the code it warned on was indeed broken. As far as I can tell, in a derived copy constructor you should always explicitly initialize the base class.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8211073
> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8211073-remove-Wno-extra-from-hotspot/webrev.01
>
> /Magnus
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
The last time I looked at -Wextra, there were several warnings that
seemed questionable to me. As the set may change between versions
(which is itself a potential issue), I took a quick look and
-Wempty-body seems quite problematic, -Wshift-negative-value arguably
so, and I haven't had time to think about others.
I'm somewhat dubious about -Wextra, -Wall, &etc for a project of this
size and usage that also uses -Werror. Those umbrella sets change
from version to version of the compiler, and the overall project
doesn't have tight control over which compiler versions might be used
(even though individual organizations, like Oracle, might have
specific blessed versions).
More information about the build-dev
mailing list