Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Paul Sandoz]

Looks good to me (not familiar with all the code areas.

Minor suggestion:

MethodHandles.java

1811          * ASCII periods. For the instance of {@link java.lang.Class} representing {@code C}:
1812          * <ul>
1813          * <li> {@link Class#getName()} returns the string {@code GN + "/" + <suffix>},
1814          *      even though this is not a valid binary class or interface name.</li>
1815          * <li> {@link Class#descriptorString()} returns the string
1816          *      {@code "L" + N + ";" + "/" + <suffix> },
1817          *      even though this is not a valid type descriptor name.</li>
1818          * </ul>

Add another bullet:

“
even though this is not a valid type descriptor name; and

- therefore {@link Class#describeConstable} returns an empty {@code Optional}.
“

?

Paul.

> On Apr 11, 2020, at 8:13 PM, Mandy Chung <mandy.chung at oracle.com> wrote:
> 
> Please review the delta patch:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-delta/
> 
> Incremental specdiff:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff-inc/overview-summary.html
> This is to follow a discussion [1] on Class::descriptorString and MethodType::descriptorString for hidden classes.   The proposal is to define the descriptor string for a hidden class of this form:
>     "L" + N + ";" + "/" + <suffix>
> 
> The spec of `Lookup::defineHiddenClass`, `Class::descriptorString` and `MethodType::descriptorString` are updated to return the descriptor of this form for a hidden class.   To support hidden class, `java.lang.invoke.TypeDescriptor` spec is revised such that a `TypeDescriptor` object can represent an entity that may not be described in nominal form.     This change affects JVM TI, JDWP and JDI.    The spec change includes a couple other JVM TI and java.instrument spec clarification w.r.t. hidden classes that Serguei has been working on.
> 
> 
> 
> Mandy
> [1] https://mail.openjdk.java.net/pipermail/valhalla-dev/2020-April/007093.html
> 
> ----------------
> As a record, the above patch is applied on the top:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06/
> 
> webrev.06 includes the following changes that have been reviewed:
> 1. rename "weak hidden" and Klass::is_hidden_weak to is_non_strong_hidden
> 2. remove unused `setImplMethod` method from lambda proxy class
> 3. include Serguei's patch to fix JDK-8242166: regression in JDI ClassUnload events
> 4. drop the uniqueness guarantee of the suffix of the hidden class's name
> 
> On 3/31/20 8:01 PM, Mandy Chung wrote:
>> Thanks to the review feedbacks.
>> 
>> Updated webrev:
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04/ 
>> Delta between webrev.03 and webrev.04:
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta/ 
>> 
>> Summary of changes is:
>> 1. Update javac to retain the previous behavior when compiling to target release <= 14 where lambda proxy class is not a nestmate
>> 2. Rename Class::isHiddenClass to Class::isHidden
>> 3. Address Joe's feedback on the CSR to document the behavior of the relevant methods in java.lang.Class for hidden classes
>> 4. Add test case for unloadable class with nest host error
>> 5. Add test cases for hidden classes with different kinds of class or interface
>> 6. Update dcmd to drop "weak hidden class" and refer it as "hidden class"
>> 7. Small changes in response to Remi, Coleen, Paul's review comments
>> 8. Fix copyright headers
>> 9. Fix @modules in tests
>> 
>> Most of the changes above have also been reviewed as separate patches.
>> 
>> Thanks
>> Mandy
>> 
>> On 3/26/20 4:57 PM, Mandy Chung wrote:
>>> Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area.  Small changes are made in javac, VM compiler (intrinsification of Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been reviewed and is in the finalized state (see specdiff and javadoc below for reference).
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 
>>> 
>>> javadoc/specdiff
>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/
>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/ 
>>> 
>>> JVMS 5.4.4 change:
>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf 
>>> 
>>> CSR:
>>> https://bugs.openjdk.java.net/browse/JDK-8238359
>>> 
>>> Thanks
>>> Mandy
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8238359
>>> [2] https://bugs.openjdk.java.net/browse/JDK-8240338
>>> [3] https://bugs.openjdk.java.net/browse/JDK-8230502
>> 
>