RFR: 8313552: Fix -Wconversion warnings in JFR code [v2]
Markus Grönlund
mgronlun at openjdk.org
Fri Aug 4 18:02:56 UTC 2023
On Fri, 4 Aug 2023 15:19:28 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:
>> src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.cpp line 914:
>>
>>> 912: u4 stack_map_attrib_len = writer.current_offset() - stack_map_attrib_len_offset;
>>> 913: // the stack_map_table_attributes_length value is exclusive
>>> 914: stack_map_attrib_len -= sizeof(u4);
>>
>> I'm thinking we should have globalDefinitions.hpp:
>>
>>
>> const int u4_size = (int)sizeof(u4);
>> const int u2_size = (int)sizeof(u2);
>> const int u1_size = (int)sizeof(u1);
>>
>>
>> I don't think you have to static_cast<u4>(4) here. It looks bad here.
>
> Since 4 is an int by default, the cast is needed.
A constant value is fine.
>> src/hotspot/share/jfr/periodic/jfrPeriodic.cpp line 437:
>>
>>> 435: EventGCSurvivorConfiguration event;
>>> 436: event.set_maxTenuringThreshold(static_cast<u1>(conf.max_tenuring_threshold()));
>>> 437: event.set_initialTenuringThreshold(static_cast<u1>(conf.initial_tenuring_threshold()));
>>
>> The Wconversion team has been preferring checked_cast<> for all narrowing casts.
>
> Are you saying that static_cast should not be used, but checked_cast should?
Ok, I have updated to use checked_cast.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284703300
PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284704458
More information about the hotspot-jfr-dev
mailing list