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

On 4/20/20 1:57 PM, Mandy Chung wrote: On 4/20/20 1:06 PM, Chris Plummer wrote: On 4/18/20 3:28 PM, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an ov

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

On 4/20/20 1:06 PM, Chris Plummer wrote: On 4/18/20 3:28 PM, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an overview of all these topics, but mostly

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

On 4/18/20 3:28 PM, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an overview of all these topics, but mostly clarifies what an initiating loader is, and

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

Hi Mandy, Thank you for the update! I like it as it is a good compromise. I do not see any wording issues. Thanks, Serguei On 4/18/20 15:28, Mandy Chung wrote: Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec

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

Hi Chris, Serguei, On 4/18/20 12:18 AM, Chris Plummer wrote: Yes. I'd like to see all this as part of the Class/Classloading spec in some sort of section that gives an overview of all these topics, but mostly clarifies what an initiating loader is, and the (non) relationship to hidden classe

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

On 4/18/20 12:47 AM, Chris Plummer wrote: It's a link to https://download.java.net/java/early_access/jdk15/docs/specs/jni/types.html#type-signatures. This is how the current JVM TI spec defines. Since it looks like this reference was present before your changes, I guess it's ok to leave it,

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

Hi Mandy, Comments inline. On 4/17/20 4:52 PM, Mandy Chung wrote: On 4/17/20 3:51 PM, Chris Plummer wrote: Hi Mandy, Thanks for updating the svc specs. Some comments below: In the JDWP spec update, you changed "JNI signature" to "type signature" in one place, but left it as "JNI signatur

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

On 4/17/20 9:37 PM, serguei.spit...@oracle.com wrote: On 4/17/20 16:52, Mandy Chung wrote: On 4/17/20 3:51 PM, Chris Plummer wrote: Hi Mandy, Thanks for updating the svc specs. Some comments below: In the JDWP spec update, you changed "JNI signature" to "type signature" in one place, but

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

On 4/17/20 16:52, Mandy Chung wrote: On 4/17/20 3:51 PM, Chris Plummer wrote: Hi Mandy, Thanks for updating the svc specs. Some comments below: In the JDWP spec update, you changed "JNI signature" to "type signature" in one place, but left it as "JNI signature" everywhere else. Should the

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

On 4/17/20 3:51 PM, Chris Plummer wrote: Hi Mandy, Thanks for updating the svc specs. Some comments below: In the JDWP spec update, you changed "JNI signature" to "type signature" in one place, but left it as "JNI signature" everywhere else. Should they all be changed? JDWP signature i

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

On 4/16/20 9:45 AM, Mandy Chung wrote: On 4/14/20 11:51 AM, Paul Sandoz wrote: 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  * 181

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

Okay, thanks. Non-serviceability changes look good too. Thanks, Serguei On 4/16/20 11:48, Mandy Chung wrote: On 4/16/20 11:42 AM, serguei.spit...@oracle.com wrote: Hi Mandy, I have a c

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

On 4/16/20 11:42 AM, serguei.spit...@oracle.com wrote: Hi Mandy, I have a couple of minor comments on the Serviceability spec update. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-svc-spec-changes/src/jdk.jdi/share/classes/com/sun/jdi/ReferenceType.java.udiff.ht

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

Hi Mandy, I have a couple of minor comments on the Serviceability spec update. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-svc-spec-changes/src/jdk.jdi/share/classes/com/sun/jdi/ReferenceType.java.udiff.html  R

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

On 4/14/20 11:51 AM, Paul Sandoz wrote: 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 * 1813 * {@link Class#getName()} ret

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

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 * 1813 * {@link Class#getName()} returns the string {@code GN + "/" + }, 1814

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

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::descriptorStr

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

On 4/6/20 11:54, Mandy Chung wrote: On 4/6/20 9:56 AM, serguei.spit...@oracle.com wrote: The suggested fix is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/valhalla-jdi-regression-8242166.1/ This patch looks okay. I'll include in my local patch. On 4/6/20 11:00 AM, Chris Plummer wrote:

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

On 4/6/20 9:56 AM, serguei.spit...@oracle.com wrote: The suggested fix is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/valhalla-jdi-regression-8242166.1/ This patch looks okay. I'll include in my local patch. On 4/6/20 11:00 AM, Chris Plummer wrote: I think that's fine but I don't th

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

Hi Chris, Okay, I'll file an RFE. Thanks, Serguei On 4/6/20 11:00, Chris Plummer wrote: Hi Serguei, I think that's fine but I don't think it should be done in the context of this Vahalla webrev s

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

Hi Serguei, I think that's fine but I don't think it should be done in the context of this Vahalla webrev since it has nothing to do with Vahalla. I'd suggest filing an RFE and pushing it to jdk/jdk. Easier to track that way if there are issues down the roa

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

Thanks, Chris! What do you think about this suggestion from Mandy:   Is ClassUnloadEvent only sent for non-array reference type?  If so, is it worth adding an assert   like assert classSignature.startsWith('L') && classSignature.en

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

Hi Serguei, This fix looks good. thanks, Chris On 4/6/20 10:05 AM, serguei.spit...@oracle.com wrote: Sorry for sending mach5 links to the open mailing lists, I've removed them below. Thank

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

Sorry for sending mach5 links to the open mailing lists, I've removed them below. Thanks, serguei On 4/6/20 09:56, serguei.spit...@oracle.com wrote: Hi Mandy, Two JDI tests started failing with the update of

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

Hi Mandy, Two JDI tests started failing with the update of src/jdk.jdi/share/classes/com/sun/tools/jdi/EventSetImpl.java:   vmTestbase/nsk/jdi/ClassUnloadRequest/addClassFilter/filter001/TestDescription.java      vmTestbase/nsk/jdi/ClassUnloadEvent/

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

Hi Peter, On 4/4/20 3:58 AM, Peter Levart wrote: Here I think, you are not quite right. First I need to clarify that we are talking about the case where the method reference in above example is not converted to lambda by javac, so the proxy class needs to invoke the superclass method directl

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

On 4/4/20 9:16 AM, Peter Levart wrote: Hi Mandy, Just another observation of the code in InnerClassLambdaMetafactory... For the useImplMethodHandle case you generate the protectedImplMethod static field to hold the MH and a static setter method:     mv = cw.visitMethod(ACC_PRIVATE

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

Hi Mandy, Just another observation of the code in InnerClassLambdaMetafactory... For the useImplMethodHandle case you generate the protectedImplMethod static field to hold the MH and a static setter method:     mv = cw.visitMethod(ACC_PRIVATE + ACC_STATIC,

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

Hi Mandy, On 4/3/20 11:32 PM, Mandy Chung wrote: Hi Peter, I added a JBS comment [1] to describe this special case trying to put the story together (let me know if it needs more explanation).   More comment inline below. Thanks, this helps in establishing the historical context. On 4/3/2

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

Hi Peter, I added a JBS comment [1] to describe this special case trying to put the story together (let me know if it needs more explanation). More comment inline below. On 4/3/20 4:40 AM, Peter Levart wrote: Ok, I think I found one such use-case. In the following example: package test; pub

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

Ok, I think I found one such use-case. In the following example: package test; public class LambdaTest {     protected void m() {     } } package test.sub; public class LambdaTestSub extends test.LambdaTest {     public void test() {     Runnable r = this::m;     r.run();     } } ...whe

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

Hi Mandy, Good work. I'm trying to find out which language use-case is covered by the InnerClassLambdaMetafactory needing to inject method handle into the generated proxy class to be invoked instead of proxy class directly invoking the method:     useImplMethodHandle = !implClass.getPa

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

Hi Mandy, Update seems fine. Thanks, David On 3/04/2020 4:56 am, Mandy Chung wrote: Hi David, Thank you for checking thoroughly.   I now grep on src/hotspot and clean all of them. Updated delta webrev: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04-delta/ On

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

On 4/2/2020 2:56 PM, Mandy Chung wrote: Hi David, Thank you for checking thoroughly.   I now grep on src/hotspot and clean all of them. Updated delta webrev: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04-delta/ Patch looks good Mandy. Lois On 4/1/20 11:21

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

Hi David, Thank you for checking thoroughly.   I now grep on src/hotspot and clean all of them. Updated delta webrev: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04-delta/ On 4/1/20 11:21 PM, David Holmes wrote: Hi Mandy, On 2/04/2020 3:17 pm, Mandy Chung wrote

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

Hi Mandy, On 2/04/2020 3:17 pm, Mandy Chung wrote: Hi David, Thanks for the edits to the comments.   "weak hidden" were missed to be changed to "non-strong hidden".  Here is the patch fixing the comments. There are 33 cases of "weak hidden" in the patch I reviewed and also some "hidden weak

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

Hi David, Thanks for the edits to the comments.   "weak hidden" were missed to be changed to "non-strong hidden".  Here is the patch fixing the comments. diff --git a/src/hotspot/share/ci/ciField.cpp b/src/hotspot/share/ci/ciField.cpp --- a/src/hotspot/share/ci/ciField.cpp +++ b/src/hotspot/

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

Hi Mandy, On 1/04/2020 1:01 pm, Mandy Chung wrote: Thanks to the review feedbacks. Updated webrev: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04/ I've had a good look through all the hotspot related changes. All looks good. A few minor comments: src/hotspot/

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

On 4/1/20 7:26 AM, Alan Bateman wrote: Maybe I missed it going by, but why is the isHidden check in the field offset methods on sun.misc.Unsafe rather than the internal Unsafe? The reflection and VarHandle implementation uses jdk.internal.misc.Unsafe to do field access (both read and write)

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

On 31/03/2020 20:25, Mandy Chung wrote: Alex's feedback:  rename isHiddenClass to isHidden as it can be a hidden class or interface. `isLocalClass` and `sAnonymousClass` are correct because the Java language only has local classes and anon classes, not local interfaces or anon. interfaces.

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

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 retai

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

Alex's feedback:  rename isHiddenClass to isHidden as it can be a hidden class or interface. `isLocalClass` and `sAnonymousClass` are correct because the Java language only has local classes and anon classes, not local interfaces or anon. interfaces.  `isHidden` is like `isSynthetic`, it could

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

This patch addresses Joe's feedback on the CSR [1]: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-jdarcy/ Specifically, it adds to the class specification of java.lang.Class to describe how the relevant methods behave for hidden classes.  In addition, use t

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

This is the patch to keep the JDK 14 behavior if target release to 14 (thanks to Jan for helping making change in javac to get the tests working) http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/ Mandy On 3/27/20 9:29 AM, Mandy Chung wrote: Hi Jan, Go

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

On 3/30/20 02:30, serguei.spit...@oracle.com wrote: Hi Mandy, I have just one comment so far. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.frames.html  356   void add_classes(LoadedClassInfo* first_clas

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

On 3/30/20 7:16 AM, coleen.phillim...@oracle.com wrote: I agree with you that this comment needs update.   Perhaps it should say "primitive, array types and hidden classes are non-modifiable. A modifiable class must be an InstanceKlass." I may have written the last part of that comment (or r

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

Adding back hotspot-dev. On 3/30/20 11:02 AM, coleen.phillim...@oracle.com wrote: Hi,  This is great work!  I did a prereview and all of my comments were addressed.  These are a few minor things I noticed. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotsp

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

Hi,  This is great work!  I did a prereview and all of my comments were addressed.  These are a few minor things I noticed. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/ci/ciInstanceKlass.hpp.udiff.html Nit. Can you add 'const' to the is_hidde

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

Adding back serviceability-dev.  Sometimes reply (and myself) remembers it and sometimes it strips it off Coleen On 3/30/20 10:16 AM, coleen.phillim...@oracle.com wrote: On 3/29/20 10:17 PM, Mandy Chung wrote: On 3/27/20 8:51 PM, Chris Plummer wrote: Hi Mandy, A couple of very minor

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

On 3/30/20 5:54 AM, David Holmes wrote: Sorry to jump in on this but it caught my eye though I may be missing a larger context ... On 30/03/2020 7:30 pm, serguei.spit...@oracle.com wrote: Hi Mandy, I have just one comment so far. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-

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

Sorry to jump in on this but it caught my eye though I may be missing a larger context ... On 30/03/2020 7:30 pm, serguei.spit...@oracle.com wrote: Hi Mandy, I have just one comment so far. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/classfil

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

Hi Mandy, I have just one comment so far. http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.frames.html  356   void add_classes(LoadedClassInfo* first_class, int num_classes, bool has_class_mirror_holder) {  3

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

Hi Mandy and Chris, On 3/29/20 19:17, Mandy Chung wrote: On 3/27/20 8:51 PM, Chris Plummer wrote: Hi Mandy, A couple of very minor nits in the jvmtiRedefineClasses.cpp comments:  153 // classes for primitives, arrays, hidden and vm unsafe anonymous classes  154 // cannot be redefi

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

On 3/27/20 8:51 PM, Chris Plummer wrote: Hi Mandy, A couple of very minor nits in the jvmtiRedefineClasses.cpp comments:  153 // classes for primitives, arrays, hidden and vm unsafe anonymous classes  154 // cannot be redefined.  Check here so following code can assume these classes

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

Hi Mandy, A couple of very minor nits in the jvmtiRedefineClasses.cpp comments:  153 // classes for primitives, arrays, hidden and vm unsafe anonymous classes  154 // cannot be redefined.  Check here so following code can assume these classes  155 // are InstanceKlass.  156 if

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

I looked at the AOT, C2, and JVMCI changes and I didn't find any issues. dl 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 (intrinsifica

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

+1 Paul. > On Mar 27, 2020, at 3:22 PM, Mandy Chung wrote: > > Hi Paul, > > This is the delta incorporating your comment: > > http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-psandoz/ > >

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

Hi Mandy, On 28/03/2020 9:46 am, Mandy Chung wrote: On 3/27/20 4:01 PM, David Holmes wrote: Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release  >= 11. However this is about javac --release 14 and the com

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

On 3/27/20 4:01 PM, David Holmes wrote: Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release  >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lambda and strin

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

- Mail original - > De: "David Holmes" > À: "mandy chung" , "Vicente Romero" > , "jan lahoda" > > Cc: "serviceability-dev" , "hotspot-dev" > , > "core-libs-dev" , "valhalla-dev" >

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

Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lambda and string concat spin classes that are not nestmat

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

Hi Mandy, On 3/27/20 6:29 PM, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release >= 11. I was not suggesting the use of `hasNestmateAccess` but to follow the same approach which is adding a new method at class `Target` to check if the new

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

Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lambda and string concat spin classes that are not nestmates. I have a patch with Jan's help: http://cr.openjdk

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

Hi Paul, This is the delta incorporating your comment: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-psandoz/ This patch also took Alex's comment to make it clear that the hidden class is the lookup class of the returned Lookup object and drops the sentence

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

Hi Mandy, The patch for nestmates [1] could be used as a reference. There a new method was added to class `com.sun.tools.javac.jvm.Target`, named: `hasNestmateAccess` which checks if a target is ready for nestmates or not. I think that you can follow a similar approach here. Thanks, Vicente

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

On 3/27/20 11:59 AM, Paul Sandoz wrote: Hi Mandy, Very thorough, bravo! Thanks. Minor suggestions below. Paul. MethodHandleNatives.java — 142 143 /** 144 * Flags for Lookup.ClassOptions 145 */ 146 static final int 147 NESTMATE_CLA

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

Hi Mandy, Very thorough, bravo! Minor suggestions below. Paul. MethodHandleNatives.java — 142 143 /** 144 * Flags for Lookup.ClassOptions 145 */ 146 static final int 147 NESTMATE_CLASS= 0x0001, 148 HIDDEN_CLASS

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

Hi Jan, Good point.  The javac change only applies to JDK 15 and later and the lambda proxy class is not a nestmate when running on JDK 14 or earlier. I probably need the help from langtools team to fix this.  I'll give it a try. Mandy On 3/27/20 4:31 AM, Jan Lahoda wrote: Hi Mandy, Rega

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

> De: "mandy chung" > À: "Remi Forax" > Cc: "valhalla-dev" , "core-libs-dev" > , "serviceability-dev" > , "hotspot-dev" > > Envoyé: Vendredi 27 Mars 2020 16:50:55 > Objet: Re: Review Request: 8238358: Imple

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

On 3/27/20 5:00 AM, Remi Forax wrote: Hi Mandy, in ReflectionFactory, why in the case of a constructor the check to the anonymous class is removed ? Good catch.  Fixed in BytecodeGenerator, the comment "// bootstrapping issue if using condy" can be promoted on top of clinit, because i ask my

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

Hi Mandy, in ReflectionFactory, why in the case of a constructor the check to the anonymous class is removed ? in BytecodeGenerator, the comment "// bootstrapping issue if using condy" can be promoted on top of clinit, because i ask myself the same question seeing a static block was generated i

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

Hi Mandy, Regarding the javac changes - should those be switched on/off depending the Target? Or, if one compiles with e.g. --release 14, will the newly generated output still work on JDK 14? Jan On 27. 03. 20 0:57, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Class