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

[Matias Saavedra Silva]

On Wed, 2 Aug 2023 13:40:12 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This patch fixes various -Wconversion warnings in classfile code.  I broke the change into commits so the changes are easier to see.
>> Tested with tier1-4.
>
> 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.

Thanks for this! I have a few questions and potential improvements overall this looks good!

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?

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?

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.

src/hotspot/share/classfile/symbolTable.cpp line 926:

> 924:   if (_symbols_counted > 0) {
> 925:     tty->print_cr("  Percent removed          %3.2f",
> 926:           ((float)_symbols_removed / (float)_symbols_counted) * 100);

I think you can also just cast once here.

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

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

PR Review: https://git.openjdk.org/jdk/pull/15111#pullrequestreview-1559621098
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282284397
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282289794
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282302648
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282303865
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282304084