RFR: implementation for JEP 334: JVM Constants API

[Brian Goetz]

Thanks for the review and the great comments!

> This version is far better than the previous one,

... or the previous seven :)

> MethodHandleDesc.Kind: The JVMS defines names for these constants, why using different names here ?

Mostly for readability of client code.  Happy to consider alternatives.

> ClassDesc.isClassOrInterface() seems very specific, does it includes annotation or enums, how will it works when value types will be included ?

Hah, this was added based on a previous review, where someone said "you 
have isArray() and isPrimitive(), but there are methods that only work 
on non-array, non-primitive (e.g., packageName()), so you should have a 
corresponding method for this, and then the isXxx methods partition the 
space."  After some bikeshed painting, So isClassOrInterface(), which 
means "not array, not primitive", seemed the best choice.   (The VM sees 
enums as classes and annotations as interfaces, and this is essentially 
a VM interop API.)  For value types, value types will be classes ("codes 
like a class!") so they will be included by this method.

We could have call this isClassWhoseDescriptorStartsWithL() but that 
seemed a bit less friendly, though that's basically the intent.

> it seems to be defined as !isPrimitive() and !isArray(), in that case, i think it can be removed.

Tried that, see above :)

> EnumDesc and VarHandleDesc, those should be static inner classes of respectively Enum and VarHandle,
> they are not part of the main API but nice addons.

Good thought!  Will consider this (need to work through some of the 
other specializations we intend to add, to make sure this scales properly.)

> DynamicCallSiteDesc is not a Desc (a ConstantDesc), so the name is very confusing.

It's not a ConstantDesc, but it _is_ a nominal descriptor.  Just not a 
nominal descriptor for a (loadable) constant.  The confusing bit is that 
its the only one of its kind, for now; later, I think this will become 
more common.  (If we had descriptors for the non-loadable constants in 
the CP (e.g., NameAndType), they'd be in this boat too.)


> DynamicConstantDesc, it's not clear to me why this is not an interface like ClassDesc or MethodHandleDesc
Fair question; we went back and forth on this a few times.  It could be 
shredded into a pair like ClassDesc/ConstantClassDesc.  However, in 
reality, we expect that all implementations of DCD would want to extend 
the base implementation anyway, since it provides useful functionality 
for which there's no point in reinventing.  Which means really 
DCD/ConstantDCD/AbstractDCD.  Which is starting to seem a little 
silly?   But I agree that things are a bit all over the map here.  (This 
is in part due to trying to cover the seam between primitive and 
reference class mirrors with a single abstraction, because the 
alternative is far worse.)

> For ConstantClassDesc, ConstantMethodTypeDesc, ConstantMethodHandleDesc, do we really need to make these classes public, in theory they should not have a proper name because they have the same API as there interface counter parts and it will be easier to retrofit them as value type if there are not part of the public APIs. By doing that, i believe that FieldDescriptor and TypeDescriptor will not need to be generics anymore.

The reason these were made public is that we expect bytecode APIs to 
case over them.  If you're writing out an LDC based on a ConstantDesc, 
you can case over: String, Integer (and friends), 
Constant{Class,MT,MH}Desc, DynamicConstantDesc -- which are the concrete 
types that correspond exactly to the loadable CP types defined in JVMS 
-- and you're done.  If we made ConstantMHDesc private, the API gets 
more complicated, because now there are a bunch of methods that only 
work on direct method handles.

> It seems that java.lang.invoke.Intrinsics has disappear (at least from the javadoc).

We split JEP 334 off from 303; Intrinsics remains in 303, and will come 
later.  This is the foundation.

> Constable.describeConstable() is weirdly named, does asConstantDesc better ?

Yeah, we had quite a loooooong discussion on this...suffice it to say we 
considered every possible option, and then some.  The basic challenge is 
that ConstantDesc implements Constable, and code like

     constantDesc.asConstantDesc()

would totally look like a no-op -- but it's not!  It's asking the 
constantDesc to _describe itself_ as a constantDesc.  So calling it

     constantDesc.describeConstable()

is more clearly asking a Constable to describe itself.

> Some implementation of Constable do not have the return type of describeConstable being tidy up (i.e. the return type uses the Optional<? ...> instead of a more specific one).

I'll take a look at these.  Some are trickier than they look.

> I really dislike the fact that there is a method describeConstable in java.lang.Enum. Given the number of people that are interested by describeConstable(), i think this interface should be removed and static methods should be used instead. And fundamentally, i do not like the fact that you can go from a live object to its ConstantDesc representation, people will abuse of that like currently we use a java.lang.Class instead of a ClassDesc. This API, for me, goes in the wrong direction and pollute too many classes.

Yeah, this is a sharp edge.  But there's also a reason for it; it 
enables the compiler to much more aggressively constant-fold things into 
condy.

The problem is not the method, or the mechanism -- the mechanism is 
awesome.  The problem is that this is a very low-level method bleeding 
into the Javadoc of some high-level things, which is sure to confuse 
users.  (We have the same problem with the two new methods on String -- 
describeConstable and resolveConstantDesc. Both are kind of silly on 
String, since they are basically identities.  In fact, the reason these 
methods are named so oddly is because they are going to show up in the 
API of some very general classes, like String.)