On Mon, 16 Feb 2026 15:03:58 GMT, Joel Sikström <[email protected]> wrote:
>> Hello,
>>
>> We should consider moving the enum ArrayProperties from ArrayKlass to its
>> own class, ArrayProperties. In addition to making the code easier to read
>> and understand, this allows us to have explicit setters/getters, replacing
>> the bit-fiddling expressions that are used in many places. The
>> ArrayProperties-specific methods in ArrayKlass have been moved to be methods
>> in the new ArrayProperties class instead.
>>
>> Perhaps the most controversial change in this PR is the removal of
>> `ArrayKlass::ArrayProperties::DEFAULT` in favor of using a default
>> constructor for ArrayProperties. The semantics are still the same, i.e.,
>> asking `.is_null_restricted()` or `.is_non_atomic()` will be false for the
>> default constructed property. With this I've also removed the unused fields
>> from ArrayProperties (DUMMY and comments).
>>
>> I did consider using define macros to generate enum+getters+setters, but I
>> opted for the stamped-out version instead.
>>
>> Testing:
>> * Running through tier1-2
>
> Joel Sikström has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Braces around if-statement
src/hotspot/share/c1/c1_Runtime1.cpp line 1211:
> 1209: k = ek->array_klass(CHECK);
> 1210: if (!k->is_typeArray_klass() && !k->is_refArray_klass() &&
> !k->is_flatArray_klass()) {
> 1211: k =
> ObjArrayKlass::cast(k)->klass_with_properties(ArrayProperties(), THREAD);
Summarizing the discussion we had offline: I like this refactor overall and I
think it makes the code much easier to understand, but `ArrayProperties()`
meaning `DEFAULT` is less maintainable than before in my opinion. It looks like
there are no properties.
I suggest making a private constructor and a static `from_default` or `infer`
or `infer_default`. We had a discussion previously on the name `DEFAULT` name
not being ideal/slightly misleading. Perhaps it is time to address it?
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2114#discussion_r2812870629
