RFR: 8313552: Fix -Wconversion warnings in JFR code

Coleen Phillimore coleenp at openjdk.org
Fri Aug 4 15:15:35 UTC 2023


On Fri, 4 Aug 2023 10:55:04 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

> Greetings,
> 
> this change set removes -Wconversion warnings in the JFR code.
> 
> Testing: jdk_jfr
> 
> Thanks
> Markus

Thank you so much for fixing these warnings. I had one or two suggestions for changes.

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.

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.

src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.hpp line 58:

> 56: 
> 57:   template <typename T>
> 58:   static void meta_clear(uint8_t bits, const T* ptr);

jbyte -> uint8_t is definitely preferable.

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

PR Review: https://git.openjdk.org/jdk/pull/15152#pullrequestreview-1563070398
PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284541315
PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284542564
PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284543641


More information about the hotspot-jfr-dev mailing list