[foreign-memaccess+abi] RFR: Change classes into records and fix finality [v2]

[Jorn Vernee]

On Mon, 19 Sep 2022 14:24:28 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/foreign/abi/VMStorage.java line 38:
>> 
>>> 36:  * @param debugName         the debug name to use
>>> 37:  */
>>> 38: public record VMStorage(byte type,
>> 
>> I think this should stay a `class`. VMStorage should not be constructed directly, but one of the factory methods should be used. But, the change to `record` forces the constructor to be public.
>
> Doesn't the same reasoning hold for Binding? E.g. there's a builder there, no?

For Binding there's a builder, but it just calls the constructor of the respective Binding class. i.e. there's no additional instantiation logic encapsulated that can lead to a malformed object. Although, it is possible to lead to malformed recipes, that is not a consequence of the public constructors of the Binding classes.

VMStorage does encapsulate some instantiation logic, namely the debug name for stack storage. Though, to be fair, there used to be more of that in an earlier version where I also checked that the wrong `type` was not being used. So, maybe it's time to revisit 🤔 

Thinking about it now, I think the real addition of extra instantiation logic happens in the architecture classes (e.g. https://github.com/openjdk/panama-foreign/blob/cb272592d5043aaa2d9f013eac37620de50418ef/src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/AArch64Architecture.java#L139) where the type of stack storage is actually known.

Maybe it would be better to make `VMStorage` just a dumb record, and drop the factory methods from it altogether.

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

PR: https://git.openjdk.org/panama-foreign/pull/725