Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Tue, 27 Oct 2020 10:40:40 GMT, Jorn Vernee wrote:
>>> …`dropReturn` seemed like a good choice since we already have
>>> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`...
>>
>> That's a very reasonable point. People might look for `dropRT` after they
>> find `dropAs`. And `MHs` is designed as a large-ish set of utility methods.
>>
>> I agree that adding `MH::changeRT` is a slippery slope; it tends to lift
>> more of the MT API onto MH, and it starts to put new stuff into the smaller
>> MH API; that was my bad.
>>
>> But (in the spirit of brainstorming, and agreeing with your present
>> proposal), I'll suggest a separate idea to think about. Use a HOFP API to
>> give easy access to the entire `MT` API all in one go, by reifying the
>> `MH.type` property as a temporary (lambda argument):
>>
>> MethodHandle mapType(MethodHandle mh, UnaryOperator fn) {
>>return mh.asType(fn.apply(mh.type()));
>> }
>>
>> This also suggests a possible argument transform utility, a sort of
>> mini-version of Charlie Nutter's MH builder API:
>>
>> MethodHandle mapArguments(MethodHandle mh,
>> UnaryOperator>> fn) {
>> var args = mh.type().argumentList().stream().map(t -> makeArg(t)).asList();
>> args = fn.apply(args);
>> … do stuff to re-weave mh with the resulting argument transforms …
>> }
>> public sealed interface ArgumentValue {
>> Class type();
>>ArgumentValue asType(Class newType);
>> … other stuff for pre-applying MHs …
>> }
>
> It's an interesting idea. It turns something like:
>
> target = target.asType(target.type().changeReturnType(void.class));
>
> Into
>
> target = target.asType(mt -> mt.changeReturnType(void.class));
>
> I.e. it removes the need to reference the target handle/type twice, which
> prevents them from potentially going out of sync (which seems more likely
> when you have multiple MethodTypes and MethodHandles flying around). Also, it
> feels unnecessary that, I have to re-specify `target`'s type as a base, when
> I'm asking `target` to change it's type. This also solves that.
CSR is now provisional. Please review:
https://bugs.openjdk.java.net/browse/JDK-8255398
Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 18:33:11 GMT, Rémi Forax
wrote:
>>> …`dropReturn` seemed like a good choice since we already have
>>> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`...
>>
>> That's a very reasonable point. People might look for `dropRT` after they
>> find `dropAs`. And `MHs` is designed as a large-ish set of utility methods.
>>
>> I agree that adding `MH::changeRT` is a slippery slope; it tends to lift
>> more of the MT API onto MH, and it starts to put new stuff into the smaller
>> MH API; that was my bad.
>>
>> But (in the spirit of brainstorming, and agreeing with your present
>> proposal), I'll suggest a separate idea to think about. Use a HOFP API to
>> give easy access to the entire `MT` API all in one go, by reifying the
>> `MH.type` property as a temporary (lambda argument):
>>
>> MethodHandle mapType(MethodHandle mh, UnaryOperator fn) {
>>return mh.asType(fn.apply(mh.type()));
>> }
>>
>> This also suggests a possible argument transform utility, a sort of
>> mini-version of Charlie Nutter's MH builder API:
>>
>> MethodHandle mapArguments(MethodHandle mh,
>> UnaryOperator>> fn) {
>> var args = mh.type().argumentList().stream().map(t -> makeArg(t)).asList();
>> args = fn.apply(args);
>> … do stuff to re-weave mh with the resulting argument transforms …
>> }
>> public sealed interface ArgumentValue {
>> Class type();
>>ArgumentValue asType(Class newType);
>> … other stuff for pre-applying MHs …
>> }
>
> instead of mapArguments(), i believe it's better to have a method map on a
> MethodType that takes two mapping functions, one for the return type and one
> for the prameter types.
>
> so one can write:
> MHs.mapType(mh, mt -> mt.map(Function.identity(), t-> t.isPrimitive? t:
> Object.class))
> to not change the return type and erase the parameter type to Object if they
> are not primitives
>
> With that, changing the return type to void can be done that way too
> MHs.mapType(mh, mt -> mt.map(__ -> void.class, Function.identity())
It's an interesting idea. It turns something like:
target = target.asType(target.type().changeReturnType(void.class));
Into
target = target.asType(mt -> mt.changeReturnType(void.class));
I.e. it removes the need to reference the target handle/type twice, which
prevents them from potentially going out of sync (which seems more likely when
you have multiple MethodTypes and MethodHandles flying around). Also, it feels
unnecessary that, I have to re-specify `target`'s type as a base, when I'm
asking `target` to change it's type. This also solves that.
-
PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 18:01:04 GMT, John R Rose wrote:
>> @rose00 My bad. You're right, I realized we could also use `asType` later on
>> (I did call it out in the CSR), but It's still a bit more wordy than what
>> I'm proposing.
>>
>> `dropReturn` seemed like a good choice since we already have
>> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`, I think
>> doing that also begs for adding a `MethodHandle::changeParameterType` (with
>> a single index and varargs overload). But, in that case it seems fair to
>> just point users at `asType` instead.
>>
>> In other words; `dropReturn` seems fine to get parity with `dropArguments`,
>> but `changeReturnType` seems a step too close to `asType` (also since it
>> implies adding `changeParameterType` as well). So, maybe this RFE is a bust,
>> and users should use `asType` instead? Or do you think adding both
>> `changeReturnType` and `changeParameterType`(s) seems worth it? (I'm not so
>> sure).
>
>> …`dropReturn` seemed like a good choice since we already have
>> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`...
>
> That's a very reasonable point. People might look for `dropRT` after they
> find `dropAs`. And `MHs` is designed as a large-ish set of utility methods.
>
> I agree that adding `MH::changeRT` is a slippery slope; it tends to lift more
> of the MT API onto MH, and it starts to put new stuff into the smaller MH
> API; that was my bad.
>
> But (in the spirit of brainstorming, and agreeing with your present
> proposal), I'll suggest a separate idea to think about. Use a HOFP API to
> give easy access to the entire `MT` API all in one go, by reifying the
> `MH.type` property as a temporary (lambda argument):
>
> MethodHandle mapType(MethodHandle mh, UnaryOperator fn) {
>return mh.asType(fn.apply(mh.type()));
> }
>
> This also suggests a possible argument transform utility, a sort of
> mini-version of Charlie Nutter's MH builder API:
>
> MethodHandle mapArguments(MethodHandle mh,
> UnaryOperator>> fn) {
> var args = mh.type().argumentList().stream().map(t -> makeArg(t)).asList();
> args = fn.apply(args);
> … do stuff to re-weave mh with the resulting argument transforms …
> }
> public sealed interface ArgumentValue {
> Class type();
>ArgumentValue asType(Class newType);
> … other stuff for pre-applying MHs …
> }
instead of mapArguments(), i believe it's better to have a method map on a
MethodType that takes two mapping functions, one for the return type and one
for the prameter types.
so one can write:
MHs.mapType(mh, mt -> mt.map(Function.identity(), t-> t.isPrimitive? t:
Object.class))
to not change the return type and erase the parameter type to Object if they
are not primitives
With that, changing the return type to void can be done that way too
MHs.mapType(mh, mt -> mt.map(__ -> void.class, Function.identity())
-
PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 17:29:11 GMT, Jorn Vernee wrote:
> …`dropReturn` seemed like a good choice since we already have
> `dropArguments`. WRT changing to `MethodHandle::changeReturnType`...
That's a very reasonable point. People might look for `dropRT` after they find
`dropAs`. And `MHs` is designed as a large-ish set of utility methods.
I agree that adding `MH::changeRT` is a slippery slope; it tends to lift more
of the MT API onto MH, and it starts to put new stuff into the smaller MH API;
that was my bad.
But (in the spirit of brainstorming, and agreeing with your present proposal),
I'll suggest a separate idea to think about. Use a HOFP API to give easy
access to the entire `MT` API all in one go, by reifying the `MH.type` property
as a temporary (lambda argument):
MethodHandle mapType(MethodHandle mh, UnaryOperator fn) {
return mh.asType(fn.apply(mh.type()));
}
This also suggests a possible argument transform utility, a sort of
mini-version of Charlie Nutter's MH builder API:
MethodHandle mapArguments(MethodHandle mh,
UnaryOperator>> fn) {
var args = mh.type().argumentList().stream().map(t -> makeArg(t)).asList();
args = fn.apply(args);
… do stuff to re-weave mh with the resulting argument transforms …
}
public sealed interface ArgumentValue {
Class type();
ArgumentValue asType(Class newType);
… other stuff for pre-applying MHs …
}
-
PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 17:16:17 GMT, John R Rose wrote: >> LGTM, pending CSR. >> >> A minor simplification suggested inline. > > I don't mind shorthands, but the existing idiom is shorter than advertised, > a one-liner assuming the MH is already bound to a var: > > target = target.asType(target.type().changeReturnType(void.class)); > > The API has good short circuits already; no new objects are > created if the MH is already void-returning. > > Perhaps this RFE could be adjusted to `MethodHandle::changeReturnType`? > Then it could be used for dropping returns *or* casting them to other types. @rose00 My bad. You're right, I realized we could also use `asType` later on (I did call it out in the CSR), but It's still a bit more wordy than what I'm proposing. `dropReturn` seemed like a good choice since we already have `dropArguments`. WRT changing to `MethodHandle::changeReturnType`, I think doing that also begs for adding a `MethodHandle::changeParameterType` (with a single index and varargs overload). But, in that case it seems fair to just point users at `asType` instead. In other words; `dropReturn` seems fine to get parity with `dropArguments`, but `changeReturnType` seems a step too close to `asType` (also since it implies adding `changeParameterType` as well). So, maybe this RFE is a bust, and users should use `asType` instead? Or do you think adding both `changeReturnType` and `changeParameterType`(s) seems worth it? (I'm not so sure). - PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
On Mon, 26 Oct 2020 15:45:00 GMT, Claes Redestad wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Collapse lform get and update into single line >> >> Co-authored-by: Claes Redestad > > LGTM, pending CSR. > > A minor simplification suggested inline. I don't mind shorthands, but the existing idiom is shorter than advertised, a one-liner assuming the MH is already bound to a var: target = target.asType(target.type().changeReturnType(void.class)); The API has good short circuits already; no new objects are created if the MH is already void-returning. Perhaps this RFE could be adjusted to `MethodHandle::changeReturnType`? Then it could be used for dropping returns *or* casting them to other types. - PR: https://git.openjdk.java.net/jdk/pull/866
Re: RFR: 8255398: Add a dropReturn MethodHandle combinator [v3]
> Hi, > > This patch adds a `dropReturn` combinator to `MethodHandles` that can be used > to create a new method handle that drops the return value, from a given > method handle. Similar to the following code: > > MethodHandle target = ...; > Class targetReturnType = target.type().returnType(); > if (targetReturnType != void.class) > target = filterReturnValue(target, empty(methodType(void.class, > targetReturnType))); > // use target > > But as a short-hand. > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Collapse lform get and update into single line Co-authored-by: Claes Redestad - Changes: - all: https://git.openjdk.java.net/jdk/pull/866/files - new: https://git.openjdk.java.net/jdk/pull/866/files/2793ac1b..e392a0f8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=866=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=866=01-02 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/866.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/866/head:pull/866 PR: https://git.openjdk.java.net/jdk/pull/866
