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