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,
javaBaseModule, myModule in generated
-Sundar
On 5/30/2016 2:50 PM, Sundararajan Athijegannathan wrote:
> 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 generated, or used, and vice versa.
>>
> Will do.
>
>> == JavaAdapterBytecodeGenerator.java ==
>>
>> + private static final Module nashornModule =
>> JavaAdapterBytecodeGenerator.class.getModule();
>>
>> This borders on the emerging discipline of Jigsaw idioms and patterns
>> ... assuming this class moves, at some point in the future, to another
>> (internal?) module. This will itch. How about having one central
>> authority saying "I represent the Nashorn module", rather than
>> implicitly assuming the class at hand will be in that module forever?
>> One line below, Object.class.getModule() looks like a waterproof
>> representative for the java.base module.
> hmm.. if this class moves out of nashorn , many things will break
> anyway! i.e., this has to be in nashorn (because of security assumptions
> for eg). I could use Context class perhaps..
>
>> + private static byte[] MODULES_READ_ADDER_BYRES;
>>
>> If it cannot be final, how about @Stable? It would help, if not in
>> terms of performance, so at least to document that this is an array
>> that will be populated *once* and never change thereafter.
>>
> hmm.. @Stable would bring another internal API dependency to nashorn.
> Would like to avoid another internal API dependency to nashorn.
>
>> + // private static final Module myModule;
>> + {
>> + FieldVisitor fv = cw.visitField(ACC_PRIVATE | ACC_FINAL |
>> ACC_STATIC,
>> + "myModule", "Ljava/lang/reflect/Module;", null, null);
>> + fv.visitEnd();
>> + }
>>
>> Suggestion: adhere to Java coding style even in generated code and
>> spell this as MY_MODULE.
> Will fix that.
>
> Thanks,
> -Sundar
>
>> Best,
>>
>> Michael
>>
>>> Am 30.05.2016 um 10:24 schrieb Sundararajan Athijegannathan
>>> <sundararajan.athijegannat...@oracle.com
>>> <mailto:sundararajan.athijegannat...@oracle.com>>:
>>>
>>> Please review http://cr.openjdk.java.net/~sundar/8158131/
>>> <http://cr.openjdk.java.net/%7Esundar/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 layers as needed. And nashorn generates/loads code
>>> into appropriate modules (which calls java.lang.reflect.Module.addReads,
>>> .addExports public APIs) to add module export/read edges as needed.
>>>
>>> -Sundar
>>>
>> --
>>
>> Oracle <http://www.oracle.com/>
>> Dr. Michael Haupt | Principal Member of Technical Staff
>> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
>> Oracle Java Platform Group | LangTools Team | Nashorn
>> Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467
>> Potsdam, Germany
>>
>> ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25,
>> D-80992 München
>> Registergericht: Amtsgericht München, HRA 95603
>>
>> Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering
>> 163/167, 3543 AS Utrecht, Niederlande
>> Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
>> Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
>> Green Oracle <http://www.oracle.com/commitment> Oracle is committed
>> to developing practices and products that help protect the environment
>>
>>
