RFR: JEP 359-Records: reflection code

[forax at univ-mlv.fr]

Maurizio, 
This seems a good idea to separate the API in (1) and (2). 
I think the BSM of (1) should return a MethodHandle instead of a CallSite and don't have a be real BSM but an API method that can be called by a BSM (see below). 

There is still an issue with (2). As i understand you will have to extract the ingredients each time you call the BSM, so once for each toString/equals/hashCode instead of once for all of them. 
To be able to share the extraction of the ingredients, the simplest way is to have a contstant dynamic that create an instance of the class that will contains the ingredient (i wonder if there is a bonus point to use a record class for that :) ) so the ingredients are extracted once and stored in an opaque object in the constant pool then each invokedynamic call inside toString/equals/hashCode can take that object as their argument. 

And for the trick to return a constant method handle because in the future maybe we will be able to use a MethodHandle instead of a ConstantCallSite + a MethodHandle, 
the BSM (2) should have a relaxed signature (return an Object) but the implementation doesn't have to support that now and when we will need it, it believe the best way to do that will be to document that in the future this BSM may return a MethodHandle instead of a CallSite so user code should be ready to deal with that. 

Rémi 

> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
> À: "Chris Hegarty" <chris.hegarty at oracle.com>
> Cc: "Remi Forax" <forax at univ-mlv.fr>, "amber-dev" <amber-dev at openjdk.java.net>,
> "compiler-dev" <compiler-dev at openjdk.java.net>
> Envoyé: Mardi 22 Octobre 2019 12:01:47
> Objet: Re: RFR: JEP 359-Records: reflection code

> Thinking more,
> I think the fundamental reason as to why the bootstrap method leaves me not 100%
> convinced is that, on the one hand, ObjectMethods tries hard to be a _general_
> helper class, offering a bootstrap method to compute equals/hashCode/toString
> on _all_ classes, not just records. And, I think, it is because of that
> generality that the bootstrap method receives a bunch of getter method handles
> - so that e.g. other language implementation can still use these methods on
> regular POJOs.

> But, if this is meant to be a general building block, then I don't understand
> e.g. why we are unifying all the signatures. If a client wants hashCode, I
> think it is kind of a design flaw that (i) there's no such BSM with that name
> (the BSM is just called "bootstrap") and (ii) that there still a requirement to
> pass a 'name list', which is ignored by the BSM.

> So, the _general_ building block case seems to push for one BSM per Object
> method, whereas the record translation use case seems to push towards a single
> unified BSM.

> I think we can actually have our cake and eat it too:

> 1) ObjectMethods could expose several BSMs - one for each Object method, with
> the _right_ static argument list and names, to make it easy for clients to find
> them. Of course, for generality, these methods should be expressed in terms of
> MethodHandle[], since we can't rely on the class being a record
> 2) We coud add a j.l.Record.bootstrap, which, using core reflection could
> extract the required ingredients, before delegating to the _right_ BSM in
> ObjectMethods (e.g. the MH list and the names list)

> This way we get the best of both worlds: sharp BSMs for clients that just want
> to implement object methods (which also works on things that are not records);
> unified BSM which act as a record translation target.

> Maurizio
> On 22/10/2019 10:41, Chris Hegarty wrote:

>>> On 22 Oct 2019, at 09:51, Maurizio Cimadamore < [
>>> mailto:maurizio.cimadamore at oracle.com | maurizio.cimadamore at oracle.com ] >
>>> wrote:

>>> ..

>>> Let me rephrase. We have Class::getRecordComponents; and we have
>>> RecordComponent::accessor.

>>> What is the need to pass _anything_ to the BSM, other than the record class
>>> (e.g. Point.class) AND the method name (e.g. toString) ?

>>> The BSM can take the .class, and get the components; from there:

>>> - you can easily derive names
>>> - you can also easily derive accessor MHs (just a lookup away)

>>> I was _not_ advocating for extracting a name from a direct method handle - that
>>> seems brittle, given that the compiler implementation might change, eventually.
>>> But doubling down on the reflection API seems the right thing to do?

>> A quick change shows that this simplifies the code a lot ( since there is less
>> arg checking )

>> [ http://cr.openjdk.java.net/~chegar/records/ObjectMethods.00/ |
>> http://cr.openjdk.java.net/~chegar/records/ObjectMethods.00/ ]

>> Q: the TypeDescriptor arg can be used for linking a dynamic constant. Is this
>> still needed?

>> -Chris.