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

[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 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.athijegannathan at oracle.com
>> <mailto:sundararajan.athijegannathan at 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
>
>