java.lang.constant.ClassDesc and TypeDescriptor for hidden class??

[forax at univ-mlv.fr]

> De: "mandy chung" <mandy.chung at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>, "Peter Levart" <peter.levart at gmail.com>
> Cc: "John Rose" <john.r.rose at oracle.com>, "Brian Goetz"
> <brian.goetz at oracle.com>, "valhalla-dev" <valhalla-dev at openjdk.java.net>
> Envoyé: Mercredi 15 Avril 2020 01:45:04
> Objet: Re: java.lang.constant.ClassDesc and TypeDescriptor for hidden class??

> On 4/14/20 2:59 AM, [ mailto:forax at univ-mlv.fr | forax at univ-mlv.fr ] wrote:

>>> De: "Peter Levart" [ mailto:peter.levart at gmail.com | <peter.levart at gmail.com> ]
>>> À: "Remi Forax" [ mailto:forax at univ-mlv.fr | <forax at univ-mlv.fr> ] , "mandy
>>> chung" [ mailto:mandy.chung at oracle.com | <mandy.chung at oracle.com> ]
>>> Cc: "John Rose" [ mailto:john.r.rose at oracle.com | <john.r.rose at oracle.com> ] ,
>>> "Brian Goetz" [ mailto:brian.goetz at oracle.com | <brian.goetz at oracle.com> ] ,
>>> "valhalla-dev" [ mailto:valhalla-dev at openjdk.java.net |
>>> <valhalla-dev at openjdk.java.net> ]
>>> Envoyé: Mardi 14 Avril 2020 11:28:52
>>> Objet: Re: java.lang.constant.ClassDesc and TypeDescriptor for hidden class??

>>> On 4/13/20 10:05 PM, [ mailto:forax at univ-mlv.fr | forax at univ-mlv.fr ] wrote:

>>>>> That's my summary why I went with option c'. The preference is "slightly".

>>>>> Any other thought?
>>>> I think c' is good for GetClassSignature where throwing an exception is not
>>>> something you can do, but for the Java side, Class.descriptorString(), i prefer
>>>> to follow the adage "the blow early, blow often" because we can throw an
>>>> exception as fast as we detect an error.

> A hidden class is like a normal class from JVM's perspective. JVM TI class
> loading events are sent for hidden classes. It's not surprised that existing
> agents may call GetClassSignature on hidden classes. JDI also has APIs to
> return class signature.

> If JVM TI GetClassSignature returns an error for hidden classes, it probably has
> higher compatibility impact.

> With the descriptor string for a hidden class defined, `Class:descriptorString`,
> JVM TI `GetClassSignature` and JDI signature related APIs should return the
> descriptor string consistently (see [
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff-inc/overview-summary.html
> |
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff-inc/overview-summary.html
> ] )

yes, that the consistency argument, all these methods should behave the same, i get that. 

> The most important property is that the result string is not resolvable. It will
> "blow early, blow often" when it attempts to get a ClassDesc from the result
> string.
Given that it's a field descriptor, it that can be passed to a library like ASM and the bug will be "hidden" until you get a VerifyError far from where the problem has occurred and with an error message people will not understand because it's not described in the JVMS. 
The class descriptor of an hidden class doesn't exist from the JVM spec POV. 

Moreover, i don't like the consistency argument because it hides the people part of the issue, JVM TI and JDI methods will never be used as often as Class::descriptorString and not by the same kind of people, 
Providing consistency to two mostly separate population (JVM engineer doesn't count, there are too few of them) is not very useful. 

I think it's important for Java people that are using/will use Class::descriptorString to have a Java experience (blow early) more than a C experience (the doc says you should not do that). 

>>> But what if there was no error? I find conservative API helpful only as long as
>>> it doesn't prevent me from doing valid things or make me jump through hoops to
>>> achieve them.

>>>> Consistency between C and Java is a great ideal but not at the price of delaying
>>>> the error reporting on the Java side.

>>> You punish the programmer before (s)he even made an error on the assumption that
>>> (s)he probably would have made it.

>>> This feels like "Minority Report".
>> :)

>> I think part of the issue here is that we don't agree about what
>> descriptorString() is,
>> The javadoc of TypeDescriptor.descriptorString() said that
>> "[it] ... must be either a field type descriptor (JVMS 4.3.2) or method type
>> descriptor (JVMS 4.3.3)"

>> So it should be a *valid* descriptor, the problem with option c' is not valid
>> because it changes the semantics of descriptorString().

>> Moreover, the javadoc of Class.descriptorString() explicitly reference
>> Class.forName() and said that descriptorString() that is an incomplete dual of
>> Class.forName(),
>> so again, returning a non valid descriptor to have it to fail to resolve later
>> is in my opinion non conform to the described semantics.

> For option c and c', `TypeDescriptor::descriptorString` and
> `Class:descriptorString` needs spec change to allow the returned string be an
> invalid type descriptor if it cannot be described nominally (see the proposed
> spec change [1]).
yes, but it's not a compatible spec change apart if the JVMS is changed too. 

> The key is that no spec change on `java.lang.constant` APIs.
I think you implicitly believe that usage of Class:descriptorString and ClassDesc are colocated. You can use TypeDescriptor to represent field descriptor at runtime to dynamically generate classes. In this kind of scenario, the `java.lang.constant` API is not in the picture. 

Rémi 

> Mandy
> [1] [
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/8242013/specdiff/overview-summary.html
> |
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/8242013/specdiff/overview-summary.html
> ]