Runtime uses jvm methods that are deprecated and may be soon removed

szymon-rd commented 2 years ago

Compiler version

At current main - 1ea177df6d, and also previous versions.

The problem

The problem is caused by usage of Unsafe.objectFieldOffset in runtime. When compiling the compiler on JVM 18, the following errors appear:

sbt:scala3> testCompilation
[error] -- Error: /Users/srodziewicz/Documents/Projects/VirtusLab/dotty/library/src/scala/runtime/LazyVals.scala:103:19 
[error] 103 |    val r = unsafe.objectFieldOffset(clz.getDeclaredField(name))
[error]     |            ^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |method objectFieldOffset in class Unsafe is deprecated since : see corresponding Javadoc for more information.
[error] -- Error: /Users/srodziewicz/Documents/Projects/VirtusLab/dotty/library/src/scala/runtime/LazyVals.scala:110:19 
[error] 110 |    val r = unsafe.objectFieldOffset(field)
[error]     |            ^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |method objectFieldOffset in class Unsafe is deprecated since : see corresponding Javadoc for more information.

It's not a problem per se, as we don't use this version of JDK on compiling. But, per the documentation:

The guarantee that a field will always have the same offset and base may not be true in a future release. The ability to provide an offset and object reference to a heap memory accessor will be removed in a future release. Use {@link java.lang.invoke.VarHandle} instead.

And we heavily rely on this feature in the current and the new lazy vals implementation. It's not a problem now, but soon lazy vals can stop working on new jvms as we don't have any guarantee on their stability.

Proposal

Use MethodHandles to execute the Unsafe methods if the currently running jvm version == 1.8. In other case, use the VarHandles. It's a pretty complex approach and there may be another simpler solution.

smarter commented 2 years ago

This is tracked by https://github.com/lampepfl/dotty/issues/9013, as noted in that issue it doesn't seem possible to decide whether we can use VarHandle at runtime without ruining the performance of VarHandle

smarter commented 2 years ago

What we could do is 1) under -release >= 9, use VarHandle properly 2) under -release 8, keep using unsafe but have a (potentially slower) runtime fallback if the method is not available

smarter commented 2 years ago

Also I'm not sure if we already changed the default of -release to match the currently used jvm, I think we wanted to do that for 3.2

prolativ commented 2 years ago

It looks like currently the default value is java 8

Kordyjan commented 2 years ago

@smarter You want to have both (or even all three possible) implementations of lazy vals in the stdlib. Then, depending on the -release flag set in the user project, choose the proper implementation during desugaring.

Am I getting it right?

michelou commented 2 years ago

Two thoughts here :

if 3.2 and newer require JVM 9+ as default (which I would welcome) the Dotty team must emphasize the change and must be ready to publish one or more 3.1 versions (after 3.1.3 final) to keep support for JVM 8 for a while (transition period to be announced).
the above change must discussed with the Scala team at Lightbend to keep both Scala 2 and Scala 3 in synch in respect to the default (JVM 8 resp. JVM 9+).

smarter commented 2 years ago

@Kordyjan yes @michelou There's no plan to drop java 8 support

michelou commented 2 years ago

@smarter That's fine (and also my expectation). I was focused primarily on the "default" JVM.

smarter commented 2 years ago

The "default" is whatever is installed on your system, this is already the case now but it's not reflected in the class file version number we emit.

som-snytt commented 2 years ago

If whining helps, it is annoying to go to try something out and it errors because my current jdk is 18.

Perhaps if I read up on the linked ticket and the "new lazy val" PR, I'll understand the end game. -release is awesome.

smarter commented 2 years ago

I think it's important for the compiler to compile on every jdk people might use, so a PR that would silence the deprecation would be welcome.

som-snytt commented 2 years ago

Just for silence https://github.com/lampepfl/dotty/pull/15520

scala / scala3