On Thu, 26 Feb 2026 07:43:13 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:
>> * Oracle's tier1-4
>
> Joel Sikström has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update fail check in ciReplay/TestValueClassArrays.java
LGTM.
src/hotspot/share/oops/arrayProperties.hpp line 77:
> 75: return "INVALID";
> 76: } else {
> 77: ss.print("%s", (is_null_restricted() != 0) ? "NULL_RESTRICTED " :
> "NULLABLE ");
Nit: boolean comparison with `!=`
-------------
Marked as reviewed by phubner (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/2114#pullrequestreview-3860533250
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2114#discussion_r2858679527
