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

[Mandy Chung]

On Mon, 10 May 2021 18:15:01 GMT, Maurizio Cimadamore <mcimadamore 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
>
> Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 23 additional commits since the last revision:
> 
>  - Merge branch 'master' into JEP-412
>  - Remove redundant checks for --enable-native-access
>  - Fix issue in snippet in package-info
>  - Replace uses of -Djdk.foreign.restricted (useless now) with --enable-native-access
>  - Fix message string in Reflection::ensureNativeAccess
>  - Tweak comment in Module::enableNativeAccess
>  - Tweak code some more
>  - Use uniform naming convention for implementation metods in Module
>  - Remove IllegalNativeAccessChecker
>  - Remove redundant initializer in Module
>  - ... and 13 more: https://git.openjdk.java.net/jdk/compare/5a35ba03...8de9da36

The caller-sensitive cleanup change looks good.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 148:

> 146:      * @throws IllegalArgumentException in the case of a method type and function descriptor mismatch.
> 147:      * {@code --enable-native-access} is either absent, or does not mention the module name {@code M}, or
> 148:      * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.

line 147-148 are leftover from ICE and they should be removed.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 209:

> 207:      * than the thread owning {@code scope}.
> 208:      * {@code --enable-native-access} is either absent, or does not mention the module name {@code M}, or
> 209:      * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.

line 208-209 are also left-over from the last patch.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractCLinker.java line 44:

> 42:     @CallerSensitive
> 43:     public final MethodHandle downcallHandle(Addressable symbol, MethodType type, FunctionDescriptor function) {
> 44:         Reflection.ensureNativeAccess(Reflection.getCallerClass());

`downcallHandle` method is no longer caller-sensitive.  `@CallerSensitive` and `Reflection.ensureNativeAccess` are no longer needed.    Same for the 4-arg `downcallHandle` method.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/LibrariesHelper.java line 38:

> 36: import jdk.internal.loader.NativeLibrary;
> 37: import jdk.internal.reflect.CallerSensitive;
> 38: import jdk.internal.reflect.Reflection;

I think these 2 imports are no longer used.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/AArch64Linker.java line 37:

> 35: import jdk.internal.foreign.abi.UpcallStubs;
> 36: import jdk.internal.reflect.CallerSensitive;
> 37: import jdk.internal.reflect.Reflection;

These 2 imports are no longer needed.  Same for other CLinker implementation classes.

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

Marked as reviewed by mchung (Reviewer).

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