On Mon, 16 Feb 2026 14:53:31 GMT, Joel Sikström <[email protected]> wrote:
>> Joel Sikström has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains seven commits:
>>
>> - Cleanups of earlier commits
>> - Merge branch 'lworld' into JDK-8378000_array_properties
>> - Semi-builder pattern for ArrayProperties
>> - Add assert to check for invalid flags/bits
>> - Change type to be consistent with compiler type
>> - Braces around if-statement
>> - 8378000: [lworld] Move ArrayProperties to its own class
>
> src/hotspot/share/oops/objArrayKlass.cpp line 425:
>
>> 423: if (next_refined_array_klass() == nullptr) {
>> 424: ObjArrayKlass* first = this;
>> 425: if (!is_refArray_klass() && !is_flatArray_klass() &&
>> (props.is_null_restricted() || props.is_non_atomic())) {
>
> The check for this was `props != ArrayKlass::ArrayProperties::DEFAULT` before
> this, which included the INVALID state. I ran this through tier1-2 with the
> assert `assert(!props.is_invalid(), "surely");`, which did not hit.
>
> I'd appreciate feedback on if we still want to check for the invalid property
> here!
This method should never be called with an INVALID property set.
Only the `ObjArrayklass` instance representing the Java type is allowed to have
an INVALID property set (in fact it must because it is an unrefined array type).
The method `ObjArrayKlass::klass_from_description()` is used to find or create
a refined array type, and refined array types must have a valid array property
set.
I'd suggest to add the assert you mentioned to ensure no INVALID props argument
is passed.
I'd also suggest to keep the test like this:
` if (!is_refArray_klass() && !is_flatArray_klass() && (props !=
ArrayProperties::Default())) {`
because it would still be correct after the introduction of new properties for
final arrays and volatile arrays.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2114#discussion_r2853986690
