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

[Maurizio Cimadamore]

On Tue, 29 Sep 2020 16:47:51 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
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move CValueLayout factories to PlatformLayouts

Good moves - I missed the fact that exposing factories was not super desirable. Added some minor comments around the
javadoc

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

> 708:
> 709:     /**
> 710:      * Subclass of ValueLayout that contains information needed when linking

Use `@link` ?

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

> 713:     class CValueLayout extends ValueLayout {
> 714:         /**
> 715:          * The kind of CValueLayout. Each kind corresponds to a particular

Use `@link` ?

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

Marked as reviewed by mcimadamore (Committer).

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