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. 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. 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. I suspect you’ll want to iterate on (3) a few times before we have a spec that is solid enough that it says exactly what it does and doesn’t do. > On Mar 18, 2020, at 10:08 AM, Jorn Vernee <jorn.ver...@oracle.com> 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 >
