On Wed, 17 Dec 2025 11:13:48 GMT, Paul Hübner <[email protected]> wrote:

>> Sub-classes of `InstanceKlass` can't have C++ fields because they would end 
>> up overlayed on top of the vtable and other dynamically sized sections of 
>> the `InstanceKlass` object.
>> 
>> To handle that the `InlineKlass` has a companion class named 
>> `InlineKlassFixedBlock`, which lists all the member fields that belongs to 
>> the InlineKlass, and an instance of that gets stamped into the `InlineKlass` 
>> object after the parts that are provided by the `InstanceKlass`.
>> 
>> I propose a few changes:
>> 
>> 1) Move `InlineKlassFixedBlock` away from instanceKlass.hpp and place it 
>> inside inlineKlass.hpp instead.
>> 
>> 2) Nest `InlineKlassFixedBlock` it inside `InlineKlass`. It's only 
>> `InlineKlass `(and the compilers) that touch these fields, so it doesn't 
>> have to be a public, top-level class.
>> 
>> 3) Rename it from `InlineKlassFixedBlock` to `InlineKlass::Members`. I think 
>> that "fixed block" term is unclear and doesn't help the reader understand 
>> its role in the `InlineKlass`. Hopefully, the name `Members` is a clearer.
>> 
>> WDYT?
>
> src/hotspot/share/ci/ciMethod.hpp line 44:
> 
>> 42: class BCEscapeAnalyzer;
>> 43: class InlineTree;
>> 44: class SigEntry;
> 
> Why do we need this declaration? AFAICT `ciMethod`'s method should be 
> unaffected by this change?

This file uses it here:

  const GrowableArray<SigEntry>* get_sig_cc() const;


and I removed the forward declaration from instanceKlass.hpp, which caused this 
file to lose its last forward declaration of this class, leading to compilation 
errors.

This is a pre-existing "issue" that we can upstream together with various other 
nit fixes if we want to.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1812#discussion_r2626973828

Reply via email to