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