[foreign-memaccess] 8227393: Add Constable support to the layout API

[Jorn Vernee]

Hi,

Some comments:

- The constants added to AbstractLayout should probably be `final`? 
(Also minor naming; these use `XXX_CD` while ConstantDescs uses 
`CD_XXX`, might want to follow the latter)
- In MemoryHandles.java there are 2 imports added that are not used.
- In TestLayoutConstants.java, there seems to be a spurious line: 
`MemoryLayout.ofStruct(MemoryLayout.ofFloatingPoint(32), 
MemoryLayout.ofPadding(8));`.

Also, what do we want to do with covariant return types for various 
`describeConstable` methods? e.g. GroupLayout::describeConstable could 
return Optional<DynamicConstantDesc<GroupLayout>>, do we want to go that 
route?

Cheers,
Jorn

On 2019-07-08 16:17, Maurizio Cimadamore wrote:
> As suggested by Brian, there's no reason as to why MemoryLayout
> classes could not implement the Constable interface. After all, they
> are static description of some memory layout, and a constant pool form
> for them can be defined (using condy). This patch does just that.
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8227393/
> 
> Now, there are some degree of freedom in doing this mapping; from
> other cases, I infer that the idiomatic way to implement constable
> would be to return a custom DynamicConstantDesc (e.g. enums return an
> EnumDesc, VarHandles a VarHandleDesc) - but in this case I'm not sure
> the exercise is worth it for two reasons:
> 
> * I can't see any 'interesting method' that could be added to a
> LayoutDesc - seems simpler to just work on the layouts themselves
> 
> * To canonicalize dynamic constants representing layouts, we'd still
> need some behavioral changes in the public SE API - which is a bit
> tricky given everything is defined in an incubator module
> 
> I'd say that what I have implemented is good enough for now, but of
> course we should reserve the right to change it later if/when the code
> ends up in java.base.
> 
> Similar considerations hold for memory access VarHandles Constable
> support - on the one hand we could just add such support - on the
> other hand, to do that correctly we'd need some surgery in
> VarHandleDesc - which is again a SE API in java.base. I think when the
> time comes, we'd add a new 'kind' to VarHandleDesc called MEMORY, and
> then add a suitable implementation of describeConstable under
> VarHandleMemoryAddressBase (as well as canonicalize support).
> 
> Note: I had to add a 'missing' BSM constant in ConstantDescs - namely
> for ConstantBootstraps#getStaticFinal. I hope this change is
> innocuous/standalone enough.
> 
> Cheers
> Maurizio