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.ch...@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
>>
>
