RFR: 8377658: G1: Cleanup g1ConcurrentMark.cpp for stricter warning flags

Leo Korinth lkorinth at openjdk.org
Fri Feb 13 14:07:51 UTC 2026


On Thu, 12 Feb 2026 20:29:02 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Cleanup `g1ConcurrentMark.cpp` and included header files to remove implicit narrowing conversions.
>> 
>> For gcc: `-Wconversion -Wno-float-conversion`
>> For clang: `-Wimplicit-int-conversion`
>> 
>> `sizeof_auto` is created so that we can reduce the amount of casting and thus make the code more type safe. The normal `sizeof` will return a `size_t` although the size of most stuff can be represented in a `uint8_t`. `sizeof_auto`  will return the size in an as small unsigned type as is possible. The result is that expressions that uses `sizeof_auto` can convert to most integral types, and no explicit narrowing cast will be needed.
>
> src/hotspot/share/oops/constantPool.hpp line 558:
> 
>> 556:     BSMAttributeEntry* bsme = bsm_attribute_entry(bsmai);
>> 557:     assert(j < bsme->argument_count(), "oob");
>> 558:     return bsm_attribute_entry(bsmai)->argument(static_cast<u2>(j));
> 
> It is unclear if this is the right change or if the `j` should be changed to `u2` and the call sites fixed.

In some way, I guess the call sites should be fixed. In other ways I feel that this is a perfect place to stop. By stopping here I can reason using the above assert that `j` is indeed a `u2` (as the type of `bsme->argument_count()` is a u2). If I change the type of `j` I do not know where to stop. I think it would be better with a checked_cast though.

If you want me to continue following the u2 type, you kind of also need to tell me where to stop. After looking at this again, I still come to the conclusion that this is probably a good place to stop.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29701#discussion_r2804362597


More information about the hotspot-dev mailing list