The updated api looks pretty good for handling both L and Q descriptors.
There's one case that isn't handled here though - L* descriptors.
WIth the bucket 2 & 3 design, we really have 3 kinds of descriptors:
L, Q, and L*. Over the years, we've spent a lot of time as an EG
talking about stars on descriptors and working through the issues
related to "stars washing off". I think this API is one of the cases
where stars may wash off and therefore needs some way to indicate that
a given L descriptor is actually an L* descriptor.
Why are stars important? If we don't have stars - which are the L
form of Q's "go and look" preload contract - we lose out on calling
convention and layout optimizations for the bucket 2 classes with
their L descriptors. Without the "*" on the descriptor, we may miss
the preload signal for a given ClassDesc and lose our chance to
optimize.
Tracking this extra boolean state in the ClassDesc seems like a
reasonable thing to do to let the stars flow through the system and
ensure they are available at classfile generation time when we want to
write the preload attribute.
> So, summarizing the new methods (modulo naming changes to reflect changes in
> Valhalla language syntax):
>
> ClassDesc ofInternal(String internalBinaryName)
> boolean isValue()
> ClassDesc valueType()
> ClassDesc refType()
Having 3 forms of descriptors unfortunately causes some issues with
the "isValue()" and "valueType()" apis. Do both bucket 2 and 3 return
"true" for isValue()? There are also two possible results for
valueType() when called on an L - add a star or convert to a Q
descriptor.
Figuring out the right methods to add - and naming them - overlaps
somewhat with the other thread about the meaning of "primitive". So
borrowing John's terminology, I think we can get by with a single new
potential API after redefining the contract for some of the other
proposed apis:
ClassDesc ofInternal(String internalBinaryName)
boolean isValue() // true for both L* & Q
ClassDesc valueType() // creates L* descriptor
ClassDesc extendedPrimitive() // creates Q descriptor
ClassDesc refType()
Alternatively, a single 'ClassDesc valueType(boolean isQ)' could be
added but I think the multiple method approach is better as aligning
the names makes the intention clearer.
--Dan
