On Fri, 13 Mar 2026 10:08:50 GMT, Stefan Karlsson <[email protected]> wrote:
>> src/hotspot/share/memory/oopFactory.cpp line 144:
>>
>>> 142:
>>> 143: ArrayKlass* ak = klass->array_klass(CHECK_NULL);
>>> 144: ObjArrayKlass* oak =
>>> ObjArrayKlass::cast(ak)->klass_with_properties(props, CHECK_NULL);
>>
>> Potential follow-up.
>>
>> Evaluate changing the `Klass::array_klass` interface to return an
>> `ObjArrayKlass*`, AFAIK we never get `TypeArrayKlass*` this way as we do not
>> have `Klass*` for primitives.
>
> I tested it out and it didn't work all the way. It all falls on this piece of
> code:
>
> ObjArrayKlass* ArrayKlass::array_klass(int n, TRAPS) {
>
> assert(dimension() <= n, "check order of chain");
> int dim = dimension();
> if (dim == n) return this;
>
>
> Where the returned `this` could be a `TypeArrayKlass`.
>
> While hitting this issue, I have a vague collection of having already done
> this experiment and hit the same issue.
Right `klass->array_klass(n, TRAPS)` means `element_klass()->array_klass(n,
TRAPS)` for arrays. The recursion uses a fixed dimension.
I wonder why we ended up with this behaviour for array_klass. For me it seems
like being able to have this feels more natural, but this would crash if oop is
an arrayOop:
```c++
oop obc = ...;
ArrayKlass* ak = obj->klass()->array_klass(1, CHECK);
And we have a different behaviour for `array_klass(TRAPS)` which does not mean
`array_klass(1, TRAPS)` but `array_klass(dim() + 1, TRAPS)`. Given that we
should be able to change `array_klass(TRAPS)` just not `array_klass(int n,
TRAPS)`.
I wonder if there is a way we could unify their behaviour. `array_klass` does
not have a lot of callers.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2207#discussion_r2930335575
