RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator)

[Maurizio Cimadamore]

On Tue, 27 Apr 2021 18:46:02 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more detailed description of such changes, to avoid repetitions during the review process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> src/java.base/share/classes/java/lang/Module.java line 253:
> 
>> 251:     }
>> 252: 
>> 253:     boolean isEnableNativeAccess() {
> 
> Would it be possible change isEnableNativeAccess and addNEnableNativeAccess to keep them consistent with the existing methods in this file.

Change as in "adding javadoc" ? Or is the naming of the method something that's bothering?

> src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java line 384:
> 
>> 382:     Module addEnableNativeAccess(Module m);
>> 383: 
>> 384:     boolean isEnableNativeAccess(Module m);
> 
> Probably better to co-locate these with the other module methods, and add a comment so that it's consistent with the existing methods.

sure I'll fix this

> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 272:
> 
>> 270:         BootLoader.loadModule(base);
>> 271:         SharedSecrets.getJavaLangAccess()
>> 272:                 .addEnableNativeAccess(Modules.defineModule(null, base.descriptor(), baseUri));
> 
> This would be cleaner if you replace it with:
> 
> Module baseModule = Modules.defineModule(null, base.descriptor(), baseUri);
> JLA.addEnableNativeAccess(baseModule);
> 
> You can use JLA in addEnableNativeAccess too, no need for "jla".

ok

> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 876:
> 
>> 874:     }
>> 875: 
>> 876:     private static void addEnableNativeAccess(ModuleLayer layer) {
> 
> It would be useful to add a method description in the same style as the existing methods, just to keep these methods consistent.

I'll fix

-------------

PR: https://git.openjdk.java.net/jdk/pull/3699