[foreign-abi] RFR: 8253796: Consider making CValueLayout public.

[Maurizio Cimadamore]

On Tue, 29 Sep 2020 15:23:03 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Hi,
> 
> This PR makes CValueLayout a public nested class of CLinker, meant to be extended. To quote the motivation from the JBS
> issue:
>> Jextract uses an attribute to attach temporary information to layouts regarding the canonical layout field in CLinker
>> they correspond to.
>> But, creating a dynamic constant out of these layouts is a complex process, since we need to recursively filter out
>> this internal jextract attribute, which requires us to recreate GroupLayouts, SequenceLayouts, FunctionDescriptors, and
>> ValueLayouts.  It would be easier if we could extend CValueLayout with a custom jextract layout class, which has an
>> extract field to indicate which CLinker constant they correspond to. In that case e.g. SourceConstantHelper can cast to
>> this custom class and retrieve this extra field, removing the need to use attributes, and the difficulties associated
>> with them.
> 
> Thanks,
> Jorn

Looks good - except for missing javadoc

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 709:

> 707:     }
> 708:
> 709:     class CValueLayout extends ValueLayout {

Some javadoc is missing here

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 710:

> 708:
> 709:     class CValueLayout extends ValueLayout {
> 710:         public enum Kind {

not sure we want the Kind exposed

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 794:

> 792:         }
> 793:
> 794:         public final Kind kind() {

Same here - should this exposed?

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

PR: https://git.openjdk.java.net/panama-foreign/pull/359