On Tue, 17 Feb 2026 15:05:31 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 two additional 
> commits since the last revision:
> 
>  - Add assert to check for invalid flags/bits
>  - Change type to be consistent with compiler type

This is a very nice encapsulation of ArrayProperties. However, I'm wondering if 
the transformation could go one step further and make ArrayProperties instances 
immutable, because after their creation, there's no reason to change the set of 
properties of the array. This would prevent accidental modifications of the 
property set. It would also allow the creation of a static instance for the 
"default" set of properties independent from the constructors.

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

PR Comment: https://git.openjdk.org/valhalla/pull/2114#issuecomment-3922535080

Reply via email to