JDK 15 RFR of JDK-8241789: Make citations of JLS and JVMS consistent in java.lang.Class

2020-03-27 Thread Joe Darcy
Hello, As noticed during the CSR review of JDK-8238359, not all the citations of JVMS and JLS are done consistently in java.lang.Class. These citations should be done uniformly.     http://cr.openjdk.java.net/~darcy/8241789.0/ Patch below. The referenced JVMS "table 4.1" had been renumbered

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

2020-03-27 Thread Chris Plummer
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

2020-03-27 Thread Dean Long
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

2020-03-27 Thread Paul Sandoz
+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

2020-03-27 Thread David Holmes
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

2020-03-27 Thread Mandy Chung
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 stri

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

2020-03-27 Thread Remi Forax
- Mail original - > De: "David Holmes" > À: "mandy chung" , "Vicente Romero" > , "jan lahoda" > > Cc: "serviceability-dev" , "hotspot-dev" > , > "core-libs-dev" , "valhalla-dev" > > Envoyé: Samedi 28 Mars 2020 00:01:58 > Objet: Re: Review Request: 8238358: Implementation of JEP 371: H

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

2020-03-27 Thread David Holmes
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: RFR: JDK-8237490: [macos] Add support notarizing jpackage app-image and dmg

2020-03-27 Thread Alexey Semenyuk
Agree with Alexander's findings. The rest looks good. - Alexey On 3/27/2020 5:18 PM, Alexander Matveev wrote: Hi Andy, http://cr.openjdk.java.net/~herrick/8237490/webrev.05/src/jdk.incubator.jpackage/macosx/classes/jdk/incubator/jpackage/internal/MacAppImageBuilder.java.frames.html Line 819,

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

2020-03-27 Thread Vicente Romero
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

2020-03-27 Thread Mandy Chung
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

2020-03-27 Thread Mandy Chung
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: RFR: JDK-8237490: [macos] Add support notarizing jpackage app-image and dmg

2020-03-27 Thread Alexander Matveev
Hi Andy, http://cr.openjdk.java.net/~herrick/8237490/webrev.05/src/jdk.incubator.jpackage/macosx/classes/jdk/incubator/jpackage/internal/MacAppImageBuilder.java.frames.html Line 819,857,902 - Looks like temp debug log message. Remove it or align with rest of code. http://cr.openjdk.java.net/~h

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

2020-03-27 Thread Vicente Romero
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: RFR: JDK-8237490: [macos] Add support notarizing jpackage app-image and dmg

2020-03-27 Thread Erik Joelsson
I think the proposed signing mechanism makes sense based on my experience signing and notarizing the JDK itself. /Erik On 2020-03-27 12:35, Andy Herrick wrote: Please review the fix to issue [1] at [2]. This change enables notarization on Mac for dmg images and app-image zip files. /Andy

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

2020-03-27 Thread Mandy Chung
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_CL

RFR: JDK-8237490: [macos] Add support notarizing jpackage app-image and dmg

2020-03-27 Thread Andy Herrick
Please review the fix to issue [1] at [2]. This change enables notarization on Mac for dmg images and app-image zip files. /Andy [1]: https://bugs.openjdk.java.net/browse/JDK-8237490 [2]: http://cr.openjdk.java.net/~herrick/8237490

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

2020-03-27 Thread Paul Sandoz
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

2020-03-27 Thread Mandy Chung
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: ClassLoader used in Module.loadModuleInfoClass should be self-first

2020-03-27 Thread Alan Bateman
On 27/03/2020 15:34, Michael Rasmussen wrote: The ClassLoader used to create the pseudo module-info interface is parent-first, so if there are any modules on the classpath, it will try to load the module-info.class file from there instead of the synthetic that is being generated in the method.

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

2020-03-27 Thread forax
> 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: Implementation of JEP 371: Hidden Classes > On 3/27/20 5:00 AM, Remi Forax wrote: >> Hi Mandy

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

2020-03-27 Thread Mandy Chung
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

ClassLoader used in Module.loadModuleInfoClass should be self-first

2020-03-27 Thread Michael Rasmussen
The ClassLoader used to create the pseudo module-info interface is parent-first, so if there are any modules on the classpath, it will try to load the module-info.class file from there instead of the synthetic that is being generated in the method. This interface is generated if you try to read

REMINDER of RFR CSR JDK-8202555: Double.toString(double) sometimes produces incorrect results

2020-03-27 Thread Raffaello Giulietti
Hello, here's the periodic monthly reminder that the latest patches to overcome [1] have been submitted [2] and are waiting for a review. Full background documentation is available at [3]. Greetings Raffaello [1] https://bugs.openjdk.java.net/browse/JDK-8202555 [2] http://cr.openjdk.ja

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Toshio 5 Nakamura
Hi Roger, Thank you so much for the review and being sponsor this fix. Best regards, Toshio > From: Roger Riggs > > Hi Toshio, > > Looks good, > > I can sponsor it. > > Thanks, Roger > > On 3/27/20 6:46 AM, Toshio 5 Nakamura wrote: > > Thank you for review, Suenaga-san, Thomas. > > > > (My ma

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Roger Riggs
Hi Toshio, Looks good, I can sponsor it. Thanks, Roger On 3/27/20 6:46 AM, Toshio 5 Nakamura wrote: Thank you for review, Suenaga-san, Thomas. (My mail in html format was blocked by the list. Sorry for inconvenience.) The latest webrev is 02: http://cr.openjdk.java.net/~tnakamura/8232846/we

Re: [15] RFR 8241727 : Typos: empty lines in javadoc, inconsistent indents, etc. (core-libs only)

2020-03-27 Thread Pavel Rappo
140 a successful * query to a constant will always remain successful. Made me pause and read a bit of the surrounding context to make sure there's no "*" type of query. Looks good to me. Thanks for doing this. -Pavel > On 27 Mar 2020, at 10:26, Ivan Gerasimov wrote: > > Hello! > > This

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

2020-03-27 Thread Remi Forax
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

2020-03-27 Thread Jan Lahoda
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

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Toshio 5 Nakamura
Thank you for review, Suenaga-san, Thomas. (My mail in html format was blocked by the list. Sorry for inconvenience.) The latest webrev is 02: http://cr.openjdk.java.net/~tnakamura/8232846/webrev.02/ Thanks, Toshio > From: Yasumasa Suenaga > > Ok, your change looks good. > > > Thanks, > > Ya

[15] RFR 8241727 : Typos: empty lines in javadoc, inconsistent indents, etc. (core-libs only)

2020-03-27 Thread Ivan Gerasimov
Hello! This is a javadoc-only fix. There are a few changes that will actually be visible (for example [1], [2]), but the vast majority of changes are to remove redundant empty lines, correct indentation, or otherwise restore harmony. Would you please help review this rather technical fix? B

Re: [15] RFR: 8214694: cleanup rawtypes warnings in open jndi tests

2020-03-27 Thread Chris Yin
Thank you, Vyom Regards, Chris > On 27 Mar 2020, at 6:06 PM, Vyom Tiwari wrote: > > Hi Chris, > > Latest changes look good to me. I can see that there are couple of unused > imports in files(DeadServerTimeoutSSLTest.java) but unused imports are > separate issue. > > Thanks, > Vyom > > On F

Re: [15] RFR: 8214694: cleanup rawtypes warnings in open jndi tests

2020-03-27 Thread Vyom Tiwari
Hi Chris, Latest changes look good to me. I can see that there are couple of unused imports in files(DeadServerTimeoutSSLTest.java) but unused imports are separate issue. Thanks, Vyom On Fri, Mar 27, 2020 at 2:48 PM Chris Yin wrote: > Hi, Vyom > > On 27 Mar 2020, at 12:08 PM, Vyom Tiwari wrot

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Yasumasa Suenaga
Ok, your change looks good. Thanks, Yasumasa On 2020/03/27 18:10, Toshio 5 Nakamura wrote: Hi Suenaga-san, Thank you for the comment. I updated the limit to 32768. webrev.02: http://cr.openjdk.java.net/~tnakamura/8232846/webrev.02 Well, I believe the new logic works as same as the current on

Re: [15] RFR: 8214694: cleanup rawtypes warnings in open jndi tests

2020-03-27 Thread Chris Yin
Hi, Vyom > On 27 Mar 2020, at 12:08 PM, Vyom Tiwari wrote: > > Hi Chris, > > I have one question to you, is there is any specific reason for using > wildcard(?) ?. Thank you for reviewing and comments. I just replaced most of the wildcard(?) with specified type as precise as they could be i

Re: [15] RFR: 8214694: cleanup rawtypes warnings in open jndi tests

2020-03-27 Thread Chris Yin
Hi, Joe Thank you for reviewing and comments. I agree that some part of the change could be as precise as they could be, and yes, after fix them, a round of redundant “cast” warnings just show up as you mentioned, both fixed now For rest of them, I could see most are due to the API return valu

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Yasumasa Suenaga
Nakamura-san, I think location of CHECK_NULL and SetObjectField() are incorrect. Currently they are called when QueryFullProcessImageName() succeed only. In addition, you need to allocate 32768 chars (32767 + 1 ('\0')) via malloc. I understand "32767" is max length, not includes null-terminator.

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Thomas Stüfe
Looks good to me in its current form, thanks. On Fri, Mar 27, 2020 at 8:02 AM Toshio 5 Nakamura wrote: > Hi Thomas, > > Yes, I tested it locally with small buffer size, 50 for example. But, it's > hard to create a jtreg test. > > I confirmed ERROR_INSUFFICIENT_BUFFER returned from GetLastError()