RFR: 8313564: Fix -Wconversion warnings in classfile code [v2]

[Coleen Phillimore]

On Wed, 2 Aug 2023 18:35:14 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Missed a case that gets -Wconversion warning for ContendedPaddingWidth.  prefetch_data_size() returns uint, so added a checked_cast<> to catch the case where it would return greater than signed int max.
>
> src/hotspot/share/classfile/altHashing.cpp line 111:
> 
>> 109: }
>> 110: 
>> 111: static void halfsiphash_init32(uint32_t v[4], uint64_t seed) {
> 
> I believe all the bitwise operators here will promote the result to int. Will this cause further conversion issues with uint32_t?

Since uint32_t is wider, I don't think so (at least I don't get a warning for it with -Wconversion).

> src/hotspot/share/classfile/classFileParser.cpp line 5244:
> 
>> 5242:   // that changes, then InstanceKlass::idnum_can_increment()
>> 5243:   // has to be changed accordingly.
>> 5244:   ik->set_initial_method_idnum(checked_cast<u2>(ik->methods()->length()));
> 
> Here you are setting a field inside an instance klass to a field acquired from within the same instance klass. Maybe it would be better for this to be a method internal to InstanceKlass?

There are a few calls like this here, that should be in instanceKlass.  I'd rather not move it with this change.

> src/hotspot/share/classfile/symbolTable.cpp line 240:
> 
>> 238: 
>> 239: double SymbolTable::get_load_factor() {
>> 240:   return (double)_items_count/(double)_current_size;
> 
> Is this redundant? Both operands are `size_t` so you can cast the result as opposed to both operands.

If I cast the result, I'll lose the remainder.  I had to cast current_size because otherwise the compiler complained about it.

> src/hotspot/share/classfile/symbolTable.cpp line 933:
> 
>> 931:   tty->print_cr("  Total symbol length      " SIZE_FORMAT_W(7), hi.total_length);
>> 932:   tty->print_cr("  Maximum symbol length    " SIZE_FORMAT_W(7), hi.max_length);
>> 933:   tty->print_cr("  Average symbol length    %7.2f", ((float)hi.total_length / (float)hi.total_count));
> 
> Same here

I think each value needs to be cast to get the correct result.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282319686
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282322968
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282321857
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282322221