On Mar 18, 2020, at 8:54 AM, Brian Goetz <brian.goetz@oracle.com> wrote:

Overall, the approach seems sound, and I like that you are introducing only one new bootstrap that is usable for a range of things. A few comments and questions.

1. Not sure the explicit source type carries its weight, but whether it does or not is probably informed by whatever we do for (3) below.

It seems true to me that Object is an acceptable universal source type. You can convert anything in the CP to Object without loss of information. IIRC this is true even for the asType rules; they are designed to take most of their information (when deciding which conversions to apply) from the target type, not the source type.

2. Naming: “convert” seems kind of general; the name should suggest some sort of asType conversion. asType is pretty rich, and will get richer in the future, so aligning with that is probably a good move.

+1 Suggest “convertAsType”.

3. The spec on convert() needs to be fleshed out, as to exactly which set of conversions are supported. “Exactly the same set as asType” is OK; “similar to asType” is kind of vague. Similarly, the spec on parameters and exceptions could be fleshed out a bit too.

+1 I suggest a formal semantics along these lines: var id = identity(sourceType); // could be destType also var mt = methodType(destType, sourceType); var conv = explicitCastArguments(id, mt); return conv.invoke(x); The explicitCastArguments is linked to asType but adjusts the handling of interfaces and of subword types (char and boolean, byte and short) to more closely match the conventions used by JVM bytecodes, when that differs from the conventions used in the Java language. I think it covers the conversions of JVM-level values you are likely to encounter in the constant pool. — John

W.r.t. the source type being needed, I see the following 4 major cases: 1. src=Prim & dst=Prim -> cast convert. For boolean the least-significant-bit is used to convert it to/from an integer type. 2. src=Prim & dst=Ref   -> box the source value and potentially cast 3. src=Ref   & dst=Prim -> apply an unboxing conversion if possible and then a casting conversion (with same trick for boolean as (1)) 4. src=Ref   & dst=Ref   -> reference cast Without the source type we can't disambiguate between cases (2) and (4), or (1) and (3) because the bootstrap takes Object as an input. For (2) and (4) the bootstrap invocation mechanism takes care of the boxing for us, and the cast is performed by (4). For (1) and (3) the conversion code for the latter handles conversion of wrapper types to Number and then to the target primitive per (1). In the end things seems to work out to the same result (though maybe I'm missing some subtle difference in a failure case). What I'm mostly worried about is that the source type already affects _how_ the conversion is done, and the fact that this difference is not observable seems somewhat incidental.... Coupled with the fact that asType and explicitCastArguments also have access to both the source and destination type, I think maybe the new bootstrap method should as well. After all, what if the set of cases is extended (valhalla?) and/or not being able to disambiguate starts to matter? --- I've written a more in-depth specification for the bootstrap, and re-implemented it using explicitCastArguments, since that helps to catch discrepancies between the input value and the source type. Here is the next iteration: http://cr.openjdk.java.net/~jvernee/8241100/webrev.01 I've kept the source type for now, if it should be removed the specification can be trimmed down (since there would be less cases to specify). As for the name; I think "asType" might be confusing since the applied conversion is not quite the same as MethodHandle::asType. Since the bootstrap is implemented in terms of explicitCastArguments I went with "explicitCast", how is that? Thanks, Jorn

Ok, thanks for explaining this. I hadn't realized the conversions were designed to work without a source type. I've removed the source type in the next revision: http://cr.openjdk.java.net/~jvernee/8241100/webrev.02/ Jorn On 21/03/2020 06:08, John Rose wrote:

On Mar 20, 2020, at 10:49 AM, Jorn Vernee <JORN.VERNEE@ORACLE.COM> wrote:

...

W.r.t. the source type being needed, I see the following 4 major cases:

1. src=Prim & dst=Prim -> cast convert. For boolean the least-significant-bit is used to convert it to/from an integer type. 2. src=Prim & dst=Ref -> box the source value and potentially cast 3. src=Ref & dst=Prim -> apply an unboxing conversion if possible and then a casting conversion (with same trick for boolean as (1)) 4. src=Ref & dst=Ref -> reference cast

Without the source type we can't disambiguate between cases (2) and (4), or (1) and (3) because the bootstrap takes Object as an input. I think that if you box the src=Prim from 1 or 2 and then hand the resulting src=Ref to 3 or 4 you will get the same thing. The basic design here is that dst=Prim is the key signal that unlocks the primitive conversions. The type of the src (Ref or Prim) doesn’t change the access to the primitive conversions.

...

For (2) and (4) the bootstrap invocation mechanism takes care of the boxing for us, and the cast is performed by (4). For (1) and (3) the conversion code for the latter handles conversion of wrapper types to Number and then to the target primitive per (1). In the end things seems to work out to the same result (though maybe I'm missing some subtle difference in a failure case). Indeed they do. That’s how I designed it. The upshot is that Object is a reliable carrier even of primitive values, for these APIs.

...

What I'm mostly worried about is that the source type already affects _how_ the conversion is done, and the fact that this difference is not observable seems somewhat incidental.... Coupled with the fact that asType and explicitCastArguments also have access to both the source and destination type, I think maybe the new bootstrap method should as well. After all, what if the set of cases is extended (valhalla?) and/or not being able to disambiguate starts to matter? You shouldn’t be appealing to an internal function here for designing the semantics. The dual arguments to the internal function are excess information; the shape of that function predates the design decision described above.

Instead, appeal to some pseudocode, such as:

var id = identity(Object.class); var mt = methodType(dst, Object.class); var conv = explicitCastArguments(id, mt); return conv.invoke(x);

I think that will give us all conversions we will need.

...

---

I've written a more in-depth specification for the bootstrap, and re-implemented it using explicitCastArguments, since that helps to catch discrepancies between the input value and the source type. Here is the next iteration: http://cr.openjdk.java.net/~jvernee/8241100/webrev.01

I've kept the source type for now, if it should be removed the specification can be trimmed down (since there would be less cases to specify).

As for the name; I think "asType" might be confusing since the applied conversion is not quite the same as MethodHandle::asType. Since the bootstrap is implemented in terms of explicitCastArguments I went with "explicitCast", how is that? Yes, I like that name.

— John

Hi David, Thanks for the heads up! A CSR for this patch was created here: https://bugs.openjdk.java.net/browse/JDK-8241667 It was moved to 'provisional' today, but still requires one or more engineer reviews. Could someone here review the CSR? Thanks, Jorn On 18/03/2020 22:59, David Holmes wrote:

Hi Jorn,

This needs a CSR request before it can be pushed.

Thanks, David

On 19/03/2020 12:08 am, Jorn Vernee wrote:

...

Hi,

Can someone please sponsor this patch that makes Boolean, Character, Byte, and Short implement Constable?

Bug: https://bugs.openjdk.java.net/browse/JDK-8241100 Webrev: http://cr.openjdk.java.net/~jvernee/8241100/webrev.00/

Having the other box types implement Constable makes them easier to use with APIs that accept any Constable. Though I'm mostly interesting in boolean, for which I'm currently falling back to "true" & "false" strings, but the downside is that this also requires parsing the string again and having to deal with random other strings.

This patch also adds the ConstantBootstraps::convert method that is used to facilitate the conversion from int to (short|char|byte). This currently takes a source type explicitly. In practice, it seems that Object can always be used as a source type for the same behavior, but explicitly specifying source and destination types seems more robust to me in case this behavior ever changes, or we want to expand on the supported kinds of conversion. (for instance it is currently not possible to convert from an int to a Long directly, or from Short to Integer, but maybe those cases could be supported in the future as well).

Testing: tier1-3 & downstream testing for my particular use case

Thanks, Jorn