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

Coleen Phillimore coleenp at openjdk.org
Thu Aug 3 00:16:11 UTC 2023


On Wed, 2 Aug 2023 23:36:42 GMT, David Holmes <dholmes 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/loaderConstraints.cpp line 498:
> 
>> 496:     for (int i = 0; i < len; i++) {
>> 497:       LoaderConstraint* probe = set.constraint_at(i);
>> 498:       sum += (int)sizeof(*probe) + (probe->num_loaders() * (int)sizeof(ClassLoaderData*));
> 
> Can't you just cast the final result to int?

I suppose I could.

> src/hotspot/share/classfile/stringTable.cpp line 891:
> 
>> 889: 
>> 890:   _shared_table.reset();
>> 891:   CompactHashtableWriter writer((int)_items_count, ArchiveBuilder::string_stats());
> 
> This makes me question why `Items_count` Is a size_t, when we treat it as an int.

Good question. I see no reason why it needs to be.  Also _items_count in StringTable is the same.

> src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.cpp line 59:
> 
>> 57:   if ((value % BytesPerLong) != 0) {
>> 58:     JVMFlag::printError(verbose,
>> 59:                         "ContendedPaddingWidth (%u) must be "
> 
> Why %u when it is an int?

I had a temporary version where it was uint that didn't do the range check message correctly.  Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282515731
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282517308
PR Review Comment: https://git.openjdk.org/jdk/pull/15111#discussion_r1282517830


More information about the graal-dev mailing list