On Tue, 2 Sep 2025 01:31:59 GMT, Ioi Lam <[email protected]> wrote:
> By adding an `InstanceKlass* InstanceKlass::super()` method to shadow `Klass*
> Klass:super()`, this PR makes it possible to simplify the following code
>
>
> InstanceKlass* ik;
> InstanceKlass* s;
> s = InstanceKlass::cast(ik->super()); // before JDK-8366024
> s = ik->java_super(); // after JDK-8366024
>
>
> to
>
>
> s = k->super();
>
>
> So you no longer need to do casting or need to understand what `java_super()`
> is.
Looks good! Great to see all those `java_super()` calls disappear. A couple of
nits/comments.
Thanks
src/hotspot/share/memory/heapInspection.cpp line 405:
> 403: bool
> print_subclasses) {
> 404: // Set do_print for all superclasses of this class.
> 405: InstanceKlass* super = InstanceKlass::cast(cie->klass())->super();
Pre-existing, but if this cast is safe then `KlassInfoEntry::_klass` should be
declared `InstanceKlass`. Otherwise this cast is not safe! Separate RFE for
that.
src/hotspot/share/oops/instanceKlass.hpp line 921:
> 919: }
> 920:
> 921: // This shadows Klass::super(). The _super of an InstanceKlass is
Suggestion:
// This overrides Klass::super(). The _super of an InstanceKlass is
src/hotspot/share/oops/instanceKlass.hpp line 924:
> 922: // always an InstanceKlass (or nullptr)
> 923: InstanceKlass* super() const {
> 924: return (Klass::super() == nullptr) ? nullptr :
> InstanceKlass::cast(Klass::super());
Is it better/simpler/cleaner to just do:
return static_cast<InstanceKlass*>(Klass::super());
?
src/hotspot/share/oops/klassVtable.cpp line 1574:
> 1572: Klass* super = _klass->super();
> 1573: if (super != nullptr) {
> 1574: InstanceKlass* sk = InstanceKlass::cast(super); // why are we sure
> this is InstanceKlass??
I don't think it is guaranteed to be an IK. But AFAICS we don't actually need
an IK here anyway, we should be able to use `super` directly.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27037#pullrequestreview-3174870461
PR Review Comment: https://git.openjdk.org/jdk/pull/27037#discussion_r2314910364
PR Review Comment: https://git.openjdk.org/jdk/pull/27037#discussion_r2314913536
PR Review Comment: https://git.openjdk.org/jdk/pull/27037#discussion_r2314912896
PR Review Comment: https://git.openjdk.org/jdk/pull/27037#discussion_r2314951109
