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