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>
> Cc: "Brian Goetz" <brian.goetz at oracle.com>, "John Rose"
> <john.r.rose at oracle.com>, "valhalla-dev" <valhalla-dev at openjdk.java.net>
> Envoyé: Jeudi 2 Avril 2020 22:35:12
> Objet: Re: java.lang.constant.ClassDesc and TypeDescriptor for hidden class??

> On 4/2/20 1:18 PM, [ mailto:forax at univ-mlv.fr | forax at univ-mlv.fr ] wrote:

>>> De: "mandy chung" [ mailto:mandy.chung at oracle.com | <mandy.chung at oracle.com> ]
>>> À: "Remi Forax" [ mailto:forax at univ-mlv.fr | <forax at univ-mlv.fr> ] , "Brian
>>> Goetz" [ mailto:brian.goetz at oracle.com | <brian.goetz at oracle.com> ]
>>> Cc: "John Rose" [ mailto:john.r.rose at oracle.com | <john.r.rose at oracle.com> ] ,
>>> "valhalla-dev" [ mailto:valhalla-dev at openjdk.java.net |
>>> <valhalla-dev at openjdk.java.net> ]
>>> Envoyé: Jeudi 2 Avril 2020 21:58:29
>>> Objet: Re: java.lang.constant.ClassDesc and TypeDescriptor for hidden class??

>>> On 4/2/20 12:03 PM, Remi Forax wrote:

>>>> ----- Mail original -----

>>>>> De: "Brian Goetz" [ mailto:brian.goetz at oracle.com | <brian.goetz at oracle.com> ]
>>>>> À: "mandy chung" [ mailto:mandy.chung at oracle.com | <mandy.chung at oracle.com> ] ,
>>>>> "John Rose" [ mailto:john.r.rose at oracle.com | <john.r.rose at oracle.com> ] Cc:
>>>>> "valhalla-dev" [ mailto:valhalla-dev at openjdk.java.net |
>>>>> <valhalla-dev at openjdk.java.net> ] Envoyé: Jeudi 2 Avril 2020 20:34:31
>>>>> Objet: Re: java.lang.constant.ClassDesc and TypeDescriptor for hidden class??

>>>>> Mandy reminded me that descriptorString() is specified to return a JLS
>>>>> 4.2.3 field descriptor, and so (c) would violate that spec.  I think
>>>>> this tilts the table away from (c), even though this string is arguably
>>>>> useful (though not clear to whom.)

>>>>> While the spec for TypeDescriptor doesn't explicitly say "for use in a
>>>>> classfile" or "for reflective instantiation", its hard to imagine use
>>>>> cases that don't terminate in one of these situations, at which point,
>>>>> we're going to fail.  So it is not clear whether we do anyone favors by
>>>>> having Class::descriptorString() return something "helpful" that is not
>>>>> really helpful.

>>>>> Given the choice between (a) and (b), well, both are going to result in
>>>>> user surprises (though, the spec for TD::descriptorString doesn't
>>>>> prohibit returning null.)  Taken together, I think (b) is less awful
>>>>> than (a), so I revise my answer to (b).

>>>> (a) and (b) are incompatible changes.
>>>> returning null is equivalent of throwing a NPE because at least the existing
>>>> callers will not expect null, so i think (a) is better because you can have a
>>>> proper error message.
>>>> And the javadoc can explain that testing myClass.isHidden() upfront is the right
>>>> way to avoid the exception.

>>> What exception would you propose to throw for this "not a valid type descriptor
>>> runtime entity"? If it throws an exception, it may be better to define a new
>>> exception type in `java.lang.constant` - would it seem a little overkill? The
>>> clients would need to be updated to handle hidden classes if they expect
>>> Class::descriptorType invoked on any class. I consider a helpful error message
>>> is nice while returning null indicates this class does not have a valid field
>>> descriptor.
>> Hidden class is not something we want every developers to know, only the ones
>> that write frameworks. So it's a secondary feature, that's why you ask if
>> creating a new runtime exception worth the cost or not ?

> Exactly and explain the reason I propose (b) to return null than throwing an
> exception.
The method already exist, so for an existing program if you are lucky returning null will raise a NPE immediately on the next instruction, if your not, it will wreck havoc several hundreds of instructions later. 
As a user, i'm for an ISE with a nice error message. 

> Mandy
Rémi 

>> I have no strong feeling, i'm fine with IllegalStateException but having an
>> exception in java.lang.constant is fine too.