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é: Lundi 13 Avril 2020 21:09:25
> Objet: Re: java.lang.constant.ClassDesc and TypeDescriptor for hidden class??

> On 4/12/20 5:14 AM, Remi Forax wrote:

>>> The problem is not that 'c' is easier to parse, but that 'c`' is not
>>> parsable at all. Do we really want unparsable method descriptors?

>>> If the problem is preventing resolving of hidden class names or
>>> descriptors, then it seems that making the method descriptors unparsable
>>> is not the right place to do that.

>> I agree with Peter,
>> throwing an exception is better, there is no way to encode a hidden class in a
>> descriptor because a hidden class has no name you can lookup,
>> if the API return an unparsable method descriptor, the user code will throw an
>> exception anyway.

> Several points that are noteworthy:

> 1. A resolved method never has a hidden class in its signature as a hidden class
> cannot be discovered by any class loader.

> 2. When VM fails to resolve a symbolic reference to a hidden class, it might
> print its name or descriptor string in the error message. Lois and Harold can
> confirm if this should or should not cause any issue (I can't see how it would
> cause any issue yet).

> 3. The only way to get a method descriptor with a hidden class in it is by
> constructing `MethodType` with a `Class` object representing a hidden class.

> 4. `Class::descriptorString` on a hidden class is human-readable but not a valid
> descriptor (both option c and c')

> 5. The special character chosen by option c and c' is an illegal character for
> an unqualified name ("." ";" "[" "/" see JVMS 4.2.2). This way loading a class
> of the name of a hidden class will always get CNFE via bytecode linkage or
> Class::forName etc (either from Class::getName or mapped from
> Class::descriptorString).

> For existing tools that map a descriptor string by trimming "L;" envelope and/or
> replacing "/" with ".", "Lfoo/Foo;/123Z" (option c') may be mapped to "foo.Foo"
> and ".123Z" (if used ";" as a separator) or "foo.Foo/123Z" which are invalid
> name whereas "Lfoo/Foo.123Z;" (option c) may have higher chance be mapped to
> "foo.Foo.123Z" which is a valid binary name.

> ";" and "[" are already used for descriptor. The remaining ones are "." and "/".

> JDWP and JDI are examples of existing tools that obtain the type descriptor by
> calling JVM TI `GetClassSignature` and then trims the "L;" envelope and replace
> "/" with ".". Option c produces "foo.Foo.123Z" as the resulting string which
> might make it harder

> 6. Throwing an exception (option a) may make existing libraries to catch issues
> very early on. I see the consistency that John made about dual-use APIs that
> prints a human-readable but not resolvable descriptor. I got convinced that
> option c and c' have the benefit over option a.

> 7. Existing tools or scripts that parse the descriptor string have to be updated
> in both option c and c' to properly handle hidden classes. Option c may just
> hide the problem which is bad if it's left unnoticed but happens in customer
> environments.

> My only concern is the compatibility risk on existing agents that assume JVM TI
> `GetClassSignature` returns a valid type descriptor and use it to resolution.
> Both option c and c' return an invalid descriptor string and so I consider the
> impact is about the same. JDI and JDWP have to be updated to work with either
> new form. As John noted, option c' has the fail-fast properties that may help
> existing code to diagnose issues during migration.

> 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. Consistency between C and Java is a great ideal but not at the price of delaying the error reporting on the Java side. 

> Mandy
> [1] [
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-delta/
> |
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-delta/
> ]

Rémi