[foreign-preview] RFR: 8280527: Move jdk.incubator.foreign to java.lang.foreign

[Jorn Vernee]

On Mon, 24 Jan 2022 16:34:34 GMT, Julia Boes <jboes at openjdk.org> wrote:

> To prepare for preview, this change removes the jdk.incubator.foreign module and moves the public classes to java.base/java.lang.foreign, implementation classes to java.base/jdk.internal.foreign respectively. 
> 
> While here, `SystemLookup` is replaced with the method `ClassLoader::findNative` and the methods in `MemoryHandles` are moved to `MethodHandles`. The `@PreviewFeature(feature=PreviewFeature.Feature.FOREIGN)` annotation and the `--enable-preview` flag are added were needed.

- jdk/internal/foreign/abi/BufferLayout file is empty. It should be removed.
- same for test/jdk/java/foreign/TestUpcall.java

Also, is `java.lang.foreign` the package we want? In the past we were in `java.foreign`.

make/test/BuildMicrobenchmark.gmk line 97:

> 95:     SRC := $(MICROBENCHMARK_SRC), \
> 96:     BIN := $(MICROBENCHMARK_CLASSES), \
> 97:     JAVAC_FLAGS := --add-exports java.base/sun.security.util=ALL-UNNAMED --enable-preview, \

AFAIK adding --enable-preview to JAVAC_FLAGS like this would result in the class file version being poisoned for all the benchmark classes. I.e. it would require running all benchmarks with `--enable-preview` (not just the foreign ones).

This was an issue in the past as well (see https://bugs.openjdk.java.net/browse/JDK-8250669 and related).

I think at the minimum, `--enable-preview` should also be added to the default VM args when running benchmarks if we go with a javac flag. But, even that might not be good enough for mainline. I don't remember the exact details, but it was hard to have benchmarks that used preview features without effecting all benchmarks.

@cl4es Do you remember the outcome of this? I remember it was not as simple a fix as just adding `--enable-preview` to the VM args... (but I'm having trouble finding the email thread now).

src/java.base/share/classes/jdk/internal/access/foreign/MemoryAddressProxy.java line 98:

> 96:     private static IndexOutOfBoundsException overflowException(long min, long max) {
> 97:         return new IndexOutOfBoundsException(String.format("Overflow occurred during offset computation ; offset exceeded range { %d .. %d }", min, max));
> 98:     }

I don't see these methods back after the move, only the ones from MemorySegmentProxy. Were they not needed anymore?

src/java.base/share/classes/jdk/internal/foreign/SharedScope.java line 56:

> 54:         }
> 55:     }
> 56: 

This shouldn't be needed I think. It's already in ResourceScopeImpl

test/hotspot/jtreg/gc/shenandoah/compiler/TestLinkToNativeRBP.java line 52:

> 50:     }
> 51: 
> 52:     final static CLinker abi = CLinker.getInstance();

This looks incorrect. This method was renamed to `systemCLinker`. Note that this test doesn't run as part of run-test-jdk_foreign.

test/jdk/java/foreign/TestMemoryHandleAsUnsigned.java line 1:

> 1: /*

Why was this file deleted?

test/jdk/java/foreign/virtual/TestVirtualCalls.java line 60:

> 58:         funcA = TestVirtualCalls.class.getClassLoader().findNative("funcA").get();
> 59:         funcB = TestVirtualCalls.class.getClassLoader().findNative("funcB").get();
> 60:         funcC = TestVirtualCalls.class.getClassLoader().findNative("funcC").get();

Maybe a helper method could be added to `NativeTestHelper`, look `findNativeOrThrow(...)` to make lookups a little bit simpler in the tests

test/micro/org/openjdk/bench/jdk/incubator/foreign/LoopOverSlice.java line 1:

> 1: /*

File deleted by accident?

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

PR: https://git.openjdk.java.net/panama-foreign/pull/633