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

[Jorn Vernee]

Hi,

Thanks for the comments.

Update webrev: 
http://cr.openjdk.java.net/~jvernee/panama/webrevs/8237545/webrev.01

- Moved layout constants into a nested Layout interface
- Moved asVarArg(ValueLayout) to SystemABI (as suggested offline)
- Cleaned up some of the import noise

Thanks,
Jorn

On 21/01/2020 14:26, Maurizio Cimadamore wrote:
> 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
>>