RFR: implementation for JEP 334: JVM Constants API

[jbvernee]

Hello,

(Thanks, Brian, for your earlier response. I've been interested in the 
constants API/condy/constant propagation since first hearing about it 
last year, so it's great to be able to get involved :)

I have some more code review comments. This time I looked at the 
implementations as well. I hope it's okay for a non-member to give 
comments about that, but after writing them down I didn't want to just 
throw them away.

They're organized by file:

Class.java

- 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)

Enum.java

- For `EnumDesc.constantBootstrap` I think you can require a `ClassDesc` 
directly as a parameter instead of a descriptor string that you then 
have to convert to a ClassDesc? (When you use it in `describeConstable` 
you have to convert your ClassDesc to a descriptor string. It'd be 
simpler if you could just pass the ClassDesc)

VarHandle.java

- For `VarHandleDesc.Kind.toBSMArgs` you could change it to return a 
`ConstantDesc<?>[]`, since the only use case requires an array. (also, 
see the next comment)

- In `VarHandleDesc.describeConstable` you can use `Constable<?>[] args 
= kind.toBSMArgs(declaringClass, constantName(), varType)` (assuming the 
changed return type), instead of having the ternary.

X-VarHandle.java.template

- In the different `describeConstable` maybe use `orElseThrow()` instead 
of `get()` on the optionals. (I think get() will be deprecated soon?)

- For the last one (line 662), instead of doing an explicit `isPresent` 
check, you can also use `Optional::map`: `return 
arrayTypeRef.map(VarHandleDesc::ofArray);`. (If it's ok to use indy 
there?)

ClassDesc.java

- Some of the string manipulation going on with descriptor strings looks 
kinda cryptic (for instance, several places use `descriptor.length() == 
1`, instead of a more descriptive function call, e.g. 
`isPrimitveDescriptor(descriptor)`). It might be a good idea to factor 
out the nitty gritty string manipulation into a DescriptorStrings class 
as static helpers, and use static imports (I don't know if that exists 
in the JDK, but there seems to be a wide demand for it).

- In `arrayType(int)`, add an exception message here. e.g.: `"rank must 
be positive, but was: " + rank`.

ConstantClassDesc.java

- In the constructor, I suggest factoring out the if-condition to an 
`isReferenceDescriptor(String descriptor)` helper. (more descriptive)

- In `resolveConstantDesc` (line 77): You can use `if(c.isPrimitive())` 
instead of `c.descriptorString().length() == 1`

- In the same method (line 82): You can use `clazz = clazz.arrayType()` 
instead of `clazz = Array.newInstance(clazz, 0).getClass();`

DynamicConstantDesc.java

- 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.


Best regards,
Jorn Vernee