RFR: JDK-8211073 Remove -Wno-extra from Hotspot

[David Holmes]

Hi Magnus,

On 27/09/2018 8:47 AM, Magnus Ihse Bursie wrote:
> Anyone from Hotspot who can review the code changes?

I had looked at this but it is unclear whether the casts are the right 
fix or whether we have a poor type selection in the first place.  For 
example,

./share/opto/node.hpp:  enum { NO_HASH = 0 };

defines NO_HASH which is implicitly treated as int but then it's used 
everywhere as uint - which just seems odd.

So I'm unclear whether this needs more indepth examination by the 
various code owners.

Thanks,
David

> /Magnus
> 
> On 2018-09-24 22:31, Magnus Ihse Bursie 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
>>
>