RFR: implementation for JEP 334: JVM Constants API
brian.goetz at oracle.com
Fri Jun 1 15:25:33 UTC 2018
Followups on these.
> - For `arrayType()` you can change it to return `Class<T>` (useful
> for reflection), which is a valid sub-signature. (I mentioned this
> before, but it seems to have fallen through the cracks)
As I suspected, this would go deeper. We'd probably want Class<T> to
extend FieldDescriptor<T>. Which leads to unchecked casts in all the FD
methods. I think the return on trying to get more generic information
out of Class is a loser here.
> - In `VarHandleDesc.describeConstable` you can use `Constable<?>
> args = kind.toBSMArgs(declaringClass, constantName(), varType)`
> (assuming the changed return type), instead of having the ternary.
This doesn't work because ConstantDesc <! Constable, and kind.toBSMArgs
rnow returns ConstantDesc<?>. The first substitution is fine (no
point in wrapping in a list just to rewrap in an array), but I think
this bus stops here.
> - In the different `describeConstable` maybe use `orElseThrow()`
> instead of `get()` on the optionals. (I think get() will be deprecated
get() is called in the shadow of a isPresent(), so on further reflection
I'm inclined to leave this alone.
> - The `resolveArgs` method uses an odd dance to propagate the
> exception thrown from the lambda, imho it's better to just use a
> traditional for-loop here instead of the stream.
Yeah, this is even worse than that. Originally, the logic was a lot
more complicated, and this was carved out of an existing calculation
which was used in multiple places. But now, the calculation got
simplified, and the other places that used it got combined, so this
method can go away entirely, since the only caller just copies into a
bigger array anyway.
Thanks for the careful reading. Stay tuned for a patch.
More information about the amber-dev