RFR: implementation for JEP 334: JVM Constants API

[Brian Goetz]

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

The parameter T in Class<T> is so unreliable (so frequently wildcarded) 
that this seems to have pretty limited leverage.

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

You're misunderstanding the audience for this method.  These aren't 
meant to be called by humans; these are the condy bootstraps that are 
referenced by the ConstantDesc returned from describeConstable, which is 
called reflectively by compilers.  So it's all under-the-hood plumbing, 
so ease of use isn't an issue.

The reason we explicitly flatten these things to String, is that it 
dramatically reduces the size of the BootstrapMethods table. Without 
these, a DynamicConstantDesc takes six BSMT slots to constantize; with 
it, they take one.  (And, if you need all the sub-components for other 
constants, they're still shared in the CP.)  So while for humans, the 
ClassDesc abstraction gives us type safety and readability, for 
machines, its just extra data movement.

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

Reasonable.

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

Right.

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

Good catch.

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

This code is very sensitive to bootstrap cycles, since its loaded 
early.  Best to stay away.

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

Yeah, we didn't used to have isPrimitive(), which does the same test.  
We should go back and use that where we do it.

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

OK

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

This is now isClassOrInterfaceType(), but we can't use this from the ctor.

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

Right, we added Class.arrayType() after this was written.

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

Both are equally offensive :)