[lworld] RFR: 8378000: [lworld] Move ArrayProperties to its own class [v2]
Paul Hübner
phubner at openjdk.org
Mon Feb 16 15:08:33 UTC 2026
On Mon, 16 Feb 2026 15:03:58 GMT, Joel Sikström <jsikstro at openjdk.org> 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
More information about the valhalla-dev
mailing list