Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-31 Thread Hannes Wallnoefer
+1 for the incremental changes in the last iteration. Hannes Am 2016-05-31 um 09:17 schrieb Sundararajan Athijegannathan: Sorry, I have updated nashorn webrev again: http://cr.openjdk.java.net/~sundar/8158131/nashorn/webrev.02/ Incremental changes: * Missed "nashornModule" name in JavaAdapte

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-31 Thread Hannes Wallnoefer
Sorry for the slow review, I needed some explanation from Sundar about the difficulties of dynamic module creation, cross-layer read edges etc. With that knowledge +1 for this change. Hannes Am 2016-05-30 um 19:55 schrieb Alan Bateman: On 30/05/2016 11:43, Sundararajan Athijegannathan wrote:

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-31 Thread Sundararajan Athijegannathan
Sorry, I have updated nashorn webrev again: http://cr.openjdk.java.net/~sundar/8158131/nashorn/webrev.02/ Incremental changes: * Missed "nashornModule" name in JavaAdapterClassLoader.java. Fixed it to use all caps name * AccessController.doPrivileged call in Context.createModuleTrusted. Earlier

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Alan Bateman
On 30/05/2016 11:43, Sundararajan Athijegannathan wrote: Updated nashorn webrev: http://cr.openjdk.java.net/~sundar/8158131/nashorn/webrev.01/ * Added @link in ModuleGraphManipulator.java's javadoc * Using Context.class.getModule() in all places where nashorn module is needed * Using all caps

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Sundararajan Athijegannathan
Updated nashorn webrev: http://cr.openjdk.java.net/~sundar/8158131/nashorn/webrev.01/ * Added @link in ModuleGraphManipulator.java's javadoc * Using Context.class.getModule() in all places where nashorn module is needed * Using all caps names for static final variables - nashornModule, javaBas

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Sundararajan Athijegannathan
Hi, Inline replies below. On 5/30/2016 2:16 PM, Michael Haupt wrote: > Hi Sundar, > > lower-case thumbs up for both the jdk and nashorn changes, with remarks: > > == ModuleGraphManipulator.java == > > * please add a @see or @link to the place where the bytes are read and > code is dynamically ge

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Michael Haupt
Hi Sundar, lower-case thumbs up for both the jdk and nashorn changes, with remarks: == ModuleGraphManipulator.java == * please add a @see or @link to the place where the bytes are read and code is dynamically generated, or used, and vice versa. == JavaAdapterBytecodeGenerator.java == +pr

RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Sundararajan Athijegannathan
Please review http://cr.openjdk.java.net/~sundar/8158131/ for https://bugs.openjdk.java.net/browse/JDK-8158131 This code cleanup is to avoid Nashorn's use of JDK internal class jdk.internal.module.Modules. With this change, nashorn uses java.lang.reflect.Layer public API to create single Module l