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

[Mandy Chung]

On 4/2/20 10:03 PM, John Rose wrote:
> On Apr 2, 2020, at 11:34 AM, Brian Goetz <brian.goetz at oracle.com> wrote:
>> 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.)
>>
> Here’s a compromise that almost makes sense on its own:
>
> (d) return the field descriptor string for the hidden class,
> minus the “.X” suffix, appropriately wrapped.
>
> First of all, it’s a valid descriptor.  (Though probably not
> resolvable to a class, still its valid.)  Second, it’s arguably
> how the HC’s descriptor *would* be spelled as a field
> descriptor, if it could.  The CONSTANT_Class entry of
> the original ClassInfo.this_class of the HC has that
> class name, sans added “.X”.
>
> But, I think the best compromise is to admit that, just
> as Class::getName returns an intentionally invalid
> though suggestive class name (rather than null or an
> exception), so Class::descriptorString should return
> the intentionally invalid though suggestive field
> descriptor which is regularly derived from said
> Class::getName.  This provides consistency in
> string-valued outputs; in the absence of an argument
> why their treatment should be inconsistent,
> consistency is less confusing and should win.

This give consistency in the type descriptor returned by 
Class::descriptorString for all classes.

However, inconsistency among the factory methods that produce ClassDesc:

    ClassDesc.ofDescriptor(HC.descriptorString()) - returns a ClassDesc
    ClassDesc.ofDescriptor(HC.getName()) - IAE thrown
    ClassDesc.of(HC.getPackageName(), HC.getSimpleName()) - IAE thrown
    Class::describeConstable - returns an empty optional.

(I am not close to java.lang.constant and the above calls may not be 
expected.)

We should also consider MethodType.  One can create a MethodType with a 
hidden class as a parameter type or return type via the factory 
methods.  In practice no one would need to create such a MethodType as a 
hidden class can't be resolved by name.   If MT(HC) is used to lookup a 
method handle, NoSuchMethodException will be thrown.

If MT(HC).toMethodDescriptorString() and MT(HC)::descriptorString drop 
`.X` suffix, the return descriptor is a valid method descriptor
     MT(HC).descriptorString() - returns "(La/b/C;)V"
     MT(HC).toMethodDescriptorString() - returns "(La/b/C;)V"
     MethodType.fromMethodDescriptorString(MT(HC).descriptorString(), 
loader) - if the loader happens to find class "a.b.C", it will produce a 
MethodType with a.b.C as its return type.
     MT(HC).describeConstable() - returns an empty optional

I'm starting to think throwing a TypeDescriptorNotPresentException would 
be consistent all these relevant APIs.

> The validity checking in the constructor of
> ReferenceClassDescImpl already ensures that such
> a HC descriptor string cannot be mistaken for a
> well-formed field descriptor, since it excludes dots “.”
> in the input.  I think *that* is the right place to
> put the error check, and it’s no accident (this time)
> that the prototype code behaves this way, because
> complicating the spec. with extra validity checks
> would also require more validity checks for the
> affected method (Class::descriptorString).
>
> Under this theory, the code needs one more validity
> check:
>
>      @Override
>      public Optional<ClassDesc> describeConstable() {
>     +     if (isHidden()) return Optional.empty();
>          return Optional.of(ClassDesc.ofDescriptor(descriptorString()));
>      }
>
> If that validity check is missing, then ClassDesc.of would
> throw (in the constructor referred to above), violating
> the convention 0f using Optional.empty().

Yes, that validity check is in needed and in my local patch.

Mandy