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