Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java 9 support #3138

Merged
merged 8 commits into from
Aug 25, 2018
Merged

Java 9 support #3138

merged 8 commits into from
Aug 25, 2018

Conversation

smarter
Copy link
Member

@smarter smarter commented Sep 17, 2017

No description provided.

@smarter smarter changed the title [WIP] Java9 support [WIP] Java 9 support Sep 17, 2017
@smarter smarter changed the title [WIP] Java 9 support Java 9 support Sep 17, 2017
@smarter smarter changed the title Java 9 support [WIP] Java 9 support Sep 17, 2017
@smarter
Copy link
Member Author

smarter commented Sep 17, 2017

sbt 0.13 cannot be used with Java 9 because it tries to compile the project build with Scala 2.10 which really does not like Java 9, so we'll probably have to wait until we switch to sbt 1 to test this properly. But it seems to work:

$ JAVA_HOME=~/opt/openjdk-9-181 ./bin/dotr
scala> java.util.List.of(1,2) 
val res0: java.util.List[Int] = [1, 2]

@retronym
Copy link
Member

retronym commented Sep 18, 2017 via email

@smarter
Copy link
Member Author

smarter commented Sep 18, 2017

@retronym I gave it a try but got an assert in scala.reflect: https://gist.github.com/smarter/67ee91680bc3cc346fb7500ee023112f am I doing something wrong?

@smarter
Copy link
Member Author

smarter commented Sep 18, 2017

The ExposedValues mentioned in the stacktrace is this thing stolen from Scala.js to avoid having to put everything into build.sbt

@retronym
Copy link
Member

retronym commented Sep 18, 2017

Yep, that's a bug in reflection.

Workaround: move your .autoplugin, and maybe your whole ./project/*.scala out of the empty package: scala/scala-dev#304

We did this in retronym/scala@96e8e97

@smarter
Copy link
Member Author

smarter commented Sep 18, 2017

Nice! Thanks :). I now have a bootstrapped dotty on Java 9. My sbt ExtractDependencies phase warned me I wasn't handling PlainNioFile which is also something that is missing from zinc as far as I can tell: https://github.com/sbt/zinc/blob/1.x/internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala#L113-L123

@retronym
Copy link
Member

Good catch, I've added a todo item to scala/scala-dev#139

@smarter
Copy link
Member Author

smarter commented Sep 19, 2017

I now have a bootstrapped dotty on Java 9.

I spoke too soon. the EA 181 build from http://jdk.java.net/9/ works, but a nightly from https://builds.shipilev.net/openjdk-jdk9-dev-release/ doesn't. It crashes with:

Exception in thread "main" java.lang.BootstrapMethodError: call site initialization exception
        at java.base/java.lang.invoke.CallSite.makeSite(CallSite.java:385)
        at java.base/java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:250)
        at java.base/java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:240)
        at dotty.runtime.LazyVals$.<init>(LazyVals.scala:87)
        at dotty.runtime.LazyVals$.<clinit>(LazyVals.scala)
        at dotty.tools.dotc.core.Contexts$ContextBase.<clinit>(Contexts.scala:542)
        at dotty.tools.dotc.Driver.initCtx(Driver.scala:36)
        at dotty.tools.dotc.Driver.process(Driver.scala:88)
        at dotty.tools.dotc.Driver.process(Driver.scala:105)
        at dotty.tools.dotc.Driver.main(Driver.scala:132)
        at dotty.tools.dotc.Main.main(Main.scala)
Caused by: java.lang.invoke.LambdaConversionException: Type mismatch for instantiated parameter 0: int is not a subtype of class java.lang.Object
        at java.base/java.lang.invoke.AbstractValidatingLambdaMetafactory.checkDescriptor(AbstractValidatingLambdaMetafactory.java:308)
        at java.base/java.lang.invoke.AbstractValidatingLambdaMetafactory.validateMetafactoryArgs(AbstractValidatingLambdaMetafactory.java:294)
        at java.base/java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:311)
        at java.base/java.lang.invoke.CallSite.makeSite(CallSite.java:332)
        ... 10 more

The corresponding LMF bootstrap method is:

  3: #68 invokestatic java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/Met
    Method arguments:
      #70 (Ljava/lang/Object;)Ljava/lang/Object;
      #153 invokespecial dotty/runtime/LazyVals$.$init$$$anonfun$4:(I)Ljava/lang/Object;
      #154 (I)Ljava/lang/Object;

which indeed looks fishy. The check was added in http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/4be3ef759ead but the referenced bug report appears to be private.

@smarter
Copy link
Member Author

smarter commented Sep 19, 2017

I spoke too soon. the EA 181 build from http://jdk.java.net/9/ works, but a nightly from https://builds.shipilev.net/openjdk-jdk9-dev-release/ doesn't.

My bad, EA 181 doesn't work either, I messed up and was accidentally running an old jdk9 build.

@Blaisorblade
Copy link
Contributor

Blaisorblade commented May 8, 2018

FWIW we’re now on sbt 1 which might help. Also FWIW now java 9 is EOL: http://www.oracle.com/technetwork/java/javase/downloads/jdk9-downloads-3848520.html

@Glavo
Copy link
Contributor

Glavo commented May 23, 2018

Hmmm, what is the status of this PR? Is it frozen? Now Dotty is my only obstruction to using Java 10 by default.

@smarter
Copy link
Member Author

smarter commented May 23, 2018

Work in progress, will come back to it at some point.

"-Ddotty.tests.classes.scalaAsm=" + findLib(attList, "scala-asm"),
"-Ddotty.tests.classes.scalaXml=" + findLib(attList, "scala-xml"),
"-Ddotty.tests.classes.jlineTerminal=" + findLib(attList, "jline-terminal"),
"-Ddotty.tests.classes.jlineReader=" + findLib(attList, "jline-reader")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also use BuildInfoPlugin to achieve the same thing. Should we pick one over the other?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No preference, but let's not change this now.

@smarter smarter force-pushed the java9 branch 4 times, most recently from 2ddd739 to e28af77 Compare August 24, 2018 13:27
@smarter smarter changed the title [WIP] Java 9 support Java 9 support Aug 24, 2018
@smarter smarter assigned allanrenucci and unassigned smarter Aug 24, 2018
@smarter
Copy link
Member Author

smarter commented Aug 24, 2018

This is finally ready for review ;)

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

How do we test this? Should we use Java 9 on the CI instead of Java 8?

file.getAbsolutePath
} mkString(":")
}
}).mkString(":")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use File.pathSeparator

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Jars.dottyInterfaces + ":" + Jars.jlineTerminal + ":" + Jars.jlineReader,
// and the other compiler dependenies:
Properties.compilerInterface + ":" + Properties.scalaLibrary + ":" + Properties.scalaAsm + ":" +
Properties.dottyInterfaces + ":" + Properties.jlineTerminal + ":" + Properties.jlineReader,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use TestConfiguration.mkClasspath?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mkClasspath checks that each directory exists so won't work on libGroup and dotty1Group here. I agree this code would be cleaned up more but let's keep that for another pr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Then we should use File.pathSeparator here as well

Copy link
Member Author

@smarter smarter Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of place where we use ":" right now which need to be fixed so I've opened an issue for it: #5000

@smarter
Copy link
Member Author

smarter commented Aug 24, 2018

How do we test this? Should we use Java 9 on the CI instead of Java 8?

No because we still want to support Java 8, instead ideally we should run the tests both with Java 8 and with Java 10 (and replace 10 by 11 when it's released).

@allanrenucci
Copy link
Contributor

Ok. I guess I'll set this up as a nightly job then. We don't want to double CI time.

The dotty implementation of Properties.isJavaAtLeast is broken for Java
9 (Fixed in Scalac at scala/scala#5276, but
maybe we just shouldn't have a copy of Properties.scala in Dotty). The
test is useless now anyway since we only compile with 2.12 which
requires Java 8, so this code is never going to run on an older version
of the JVM.
The standard Java library as well as any other module used will not be
available in the regular Java classpath but only in the jrt:/ filesystem.
It's no longer introspectable under Java 9. The scalac 2.12 shell scripts work
around this by passing the bootclasspath jars as an additional system
property, but we can avoid this complication by just using -classpath instead.
This may have a slight negative impact on startup performance since the
JVM bytecode verifier is not run for classes from the bootclasspath, but
we can worry about that later.
- scala-library and scala-xml used to be on the bootclasspath and need
  to be on the regular classpath now.

- Jars.findJarFromRuntime relied on ClassLoader.getSystemClassLoader
  being an URLClassLoader, this isn't true anymore in Java 9. This is
  replaced by passing properties from the sbt build instead, like we were
  already doing for the dotty jars. While I was at it I completely killed
  dotty.Jars and cleaned things a bit.
This broke tests/pos/t2484.scala with Java 9 and doesn't seem to
actually be useful for anything.
@Blaisorblade
Copy link
Contributor

Bootstrap started failing since the merge with FileAlreadyExistsException http://dotty-ci.epfl.ch/lampepfl/dotty/7060/5 @smarter @allanrenucci

liufengyun added a commit to dotty-staging/dotty that referenced this pull request Aug 26, 2018
The classpath is intended for type checking, not for compiler run-time.
Thus, they should be passed as compiler options, not JVM options.

It works before because the compiler makes bootstrap classpath
available for type checking. However, we can no longer use bootstrap
classpath after Java 9 support (scala#3138).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants