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

[Stefan Karlsson]

On Thu, 12 Feb 2026 18:44:42 GMT, Leo Korinth <lkorinth 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.

I did a pass over this.

src/hotspot/share/gc/g1/g1RegionMarkStatsCache.hpp line 101:

> 99: 
> 100:   uint hash(uint idx) {
> 101:     return idx & static_cast<uint>(_num_cache_entries_mask);

It looks like `_num_cache_entries_mask` could be changed to an `uint` instead.

src/hotspot/share/gc/z/zBitField.hpp line 77:

> 75:   static ContainerType encode(ValueType value) {
> 76:     assert(((ContainerType)value & (FieldMask << ValueShift)) == (ContainerType)value, "Invalid value");
> 77:     return (ContainerType)(((ContainerType)value >> ValueShift) << FieldShift);

This change should not be done in a PR that sounds like it is only fixing G1. It needs its own PR with an explanation why it is 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.

src/hotspot/share/oops/fieldInfo.hpp line 305:

> 303:   // boilerplate:
> 304:   u1 _flags;
> 305:   static constexpr u1 flag_mask(FieldStatusBitPosition pos) { return static_cast<u1>(1 << pos); }

Shouldn't this use `checked_cast` or Kim's proposed `integer_cast` to make sure that a future change to `FieldStatusBitPosition` doesn't make this overflow an `u1`?

src/hotspot/share/utilities/globalDefinitions.hpp line 173:

> 171: // Use this instead of sizeof() if you want your expression not to be of type size_t
> 172: template <typename T>
> 173: constexpr auto sizeof_auto() {

I would consider extracting this into a separate PR.

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

PR Review: https://git.openjdk.org/jdk/pull/29701#pullrequestreview-3793426644
PR Review Comment: https://git.openjdk.org/jdk/pull/29701#discussion_r2800875871
PR Review Comment: https://git.openjdk.org/jdk/pull/29701#discussion_r2800881629
PR Review Comment: https://git.openjdk.org/jdk/pull/29701#discussion_r2800920998
PR Review Comment: https://git.openjdk.org/jdk/pull/29701#discussion_r2800926320
PR Review Comment: https://git.openjdk.org/jdk/pull/29701#discussion_r2800930026