-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Support Java 9 bytecode by adding "-target:jvm-1.9" #6146
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
Conversation
Just signed the CLA right now. |
Maybe we should also use |
Let's move taking advantage of Java 9 to 2.13.x, where new feature development is now focussed. The scope of 2.12 is limited to making sure the compiler and other tools run properly on Java 9. To do this, please use the edit button to select 2.13.x as your PR's base branch. |
a possible counterargument is that emitting the higher bytecode version, without actually otherwise altering the bytecode, is a quick way for a library that requires Java 9 (e.g. because it uses Java 9 only APIs) to fail fast if inadvertently used on a lower version. @mkurz we appreciate you looking at this and submitting the patch. are there any other arguments in favor of this (on 2.12.x) you want to make? adding this isn't a no-brainer, I'm afraid. adding it might raise expectations that we aren't prepared to meet in the 2.12.x series. such a flag wouldn't be backed by our normal testing regimen (we wouldn't be validating PRs with the new bytecode version, or running the community build with it), and it's entirely possible that the new bytecode version might be problematic in ways that we don't even know about at the moment. on the balance I think I agree with Adriaan, but having given it some thought, I thought I'd write those thoughts down. |
Good point Seth — I should have phrased my comment more as a question. I’m open to discussing, but am indeed concerned about setting to high expectations for 2.12, which has to yield to 2.13 development. |
No. I also can only think of the fail fast argument you made. I don't know if that is worth the trouble that may arise. So I changed the base branch to However do we want to keep |
I like |
Alright, updated! |
We need to tweak the encoding of final vals in traits before we allow the new classfile format. In short, the backing field in subclassed should no longer have the We also need to add JDK 9 to our CI matrix, and work through test cases that fail. That doesn't strictly need to happen before this change, but we must realise that we're flying blind. |
Also, we should probably get rid of the |
There is a new a new ( |
FYI: ASM 6.2 released with better JDK 10 support and even with JDK 11 support already! |
Thanks for the heads up, @mkurz. I've rebased our ASM fork atop 6.2. Our test suite seems happy with the result. We'll need to do a bit of due diligence on the 6.0..6.2 diffs in ASM before we incorporate it. We'll definitely include this in Scala 2.13. I'd also like to include it in the next maintenance release of Scala 2.12. |
@retronym Thanks! Great to hear that you will also backport it to the 2.12 branch. BTW: I don't think I will have time to work on this open pull request here anymore, so everyone is welcome to take over. Thanks! |
Thanks to upgrading
scala-asm
to5.2-scala-2
in #6116 we now have support for Java 9 bytecode because asm now providesOpcodes.V1_9
.Note: asm 6.0 renames the constant to
Opcodes.V9
, see scala/scala-asm@f7f44a8This should also be cherry-picked to the
2.13.x
branch I assume.