[foreign-abi] RFR 8237545: Merge C layout constant classes with SystemABI implementations

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Tue Jan 21 13:26:38 UTC 2020


Hi Jorn,
generally looks good - I have one question: since constants are defined 
in both SystemABI and also in the subclasses, one potential problem is 
that, since static constants in Java are inherited, if the set of 
constants should eer diverge - you will start to see e.g. in WindowsABI 
both SystemABI dynamic constants *and* WindowsABI-specific constants; 
while probably this is not an issue right now, I'm concerned about the 
inherent fragility of such a system.

A possible way to avoid this would be to add an indirection - e.g. have 
constants inside an interface SystemABI.Layouts - and then 
WindowsABI.Layouts - and so forth. This way the set of constants would 
be completely disjoint.

Maurizio


On 21/01/2020 12:05, Jorn Vernee wrote:
> Hi,
>
> Please review the following patch that merges the internal SystemABI 
> implementations (which were very minimal) with the layout constant 
> classes we already had in the public API, and puts the resulting 
> classes under SystemABI. I've also added (back) some dynamic constants 
> that get initialized based on the platform we are running on (which 
> saw some immediate use in the tests).
>
> It is a good place to house any platform-specific code that is still 
> needed in the public API. For instance, should we decide to make 
> layout annotations a part of the public API, we can move the different 
> ArgumentClassImpl enums here as well.
>
> Moving the SystemABI implementations into the public space also allows 
> users to check which platform they are running on by checking the 
> value returned by SystemABI::getInstance against one of the 
> implementation types (which we've seen some need for in cross-platform 
> code).
>
> As the implementation classes grow, we might want to put them into 
> their own source files as well, and create a public `abi` package for 
> them, but I've held off on that for now.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8237545
> Webrev: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8237545/webrev.00/
>
> Thanks,
> Jorn
>


More information about the panama-dev mailing list