Optimizer doesn't work on JDK 16 (needs ASM 9)

[SethTisue]

This came up at scala/community-build#1352

Typically manifests as

[error] java.lang.IllegalArgumentException: Unsupported class file major version 60
[error] 	at scala.tools.asm.ClassReader.<init>(ClassReader.java:196)
[error] 	at scala.tools.asm.ClassReader.<init>(ClassReader.java:177)
[error] 	at scala.tools.asm.ClassReader.<init>(ClassReader.java:163)
[error] 	at scala.tools.nsc.backend.jvm.opt.ByteCodeRepository.$anonfun$parseClass$1(ByteCodeRepository.scala:281)

We'll need to upgrade to ASM 9.

[mkurz]

@SethTisue
When do you plan to work on this issue?
Also when will Scala 2.12.14 be due?
I am asking because current ASM master already unlocks Java 17 (https://gitlab.ow2.org/asm/asm/-/merge_requests/307), however there was no new release including this merge request yet.

Given that the last ASM release was cut in September I opened an issue asking if they could release new version: https://gitlab.ow2.org/asm/asm/-/issues/317936

I know Java 17 wont be released before September, however given it will be an LTS, it would be great to have support for it in Scala ASAP, so projects already can run tests against it and see what needs to be fixed.

Let's see what they answer and maybe we can just wait for that new ASM release (if it is acceptable for the timeframe of the 2.12.14 milestone)?

[SethTisue]

@mkurz I intend to go ahead with the ASM 9 upgrade promptly (this week, if all goes well), so that people can test 2.12.x and 2.13.x nightlies on JDK 16 ASAP. If another ASM release follows, we can upgrade again -- I don't mind doing it in two rounds.

re: release timing, let's use Discourse for that:

• https://contributors.scala-lang.org/t/scala-2-12-14-planning/4852
• https://contributors.scala-lang.org/t/scala-2-13-5-planning/4825

[smarter]

I welcome the ASM upgrade, but as I've said before, the long term solution here is to get the optimizer to stop trying to read Java classfiles, otherwise the compiler will keep breaking at every Java release for no good reason: #11671 (comment)

[SethTisue]

I followed the steps at https://github.com/scala/scala-asm/blob/readme/README.md and published https://repo1.maven.org/maven2/org/scala-lang/modules/scala-asm/9.0.0-scala-1/

PR: scala/scala#9477

[dwijnand]

I welcome the ASM upgrade, but as I've said before, the long term solution here is to get the optimizer to stop trying to read Java classfiles, otherwise the compiler will keep breaking at every Java release for no good reason: #11671 (comment)

I feel likewise that it's annoying to be pressed like so, especially on a "there might be changes in semantics" potential basis... Looking at the community build runs it looks like utest is opting in to -opt:l:method and curryhoward into -opt:l:inline (to pick 2 out of a handful of projects).

We don't support these non-LTS JDKs but we still try to test them on the basis that users might not be aware that apt-get install or whatever is giving them JDK 16 and so it's beneficial for them and our project that compiling and running scala code works on the latest, even-non-LTS version, JDK. However, I highly doubt these users are compiling code with those optimisations, and even if they are "all" they need to do is switch to a supported JDK.

So I suggest we "just" change the community build overrides to drop those opt flags. Or, actually, tweak them as we can keep l:method but just disable the closure inlining piece of it which uses the call graph; I forget how things are for l:inline. And we only need to do so when running on such JDKs, we can continue to assert correct behaviour of the optimizer on the supported JDKs.

We'll still look to upgrade ASM on patch releases, but the community build on non-LTS JDKs should just compile+test the projects without this part of the optimizer and we needn't coordinate our patch releases based on JDK releases - I'm sure utest/curryhoward/etc maintainers can continue with their existing JDK or temporarily disable the optimizer parts.

It would also be good to get the inliner to fail early and/or clearly on what the cause is, so users know what's going on. Or maybe that needless as Google/StackOverflow/GitHub make it easy for people to get to the same answer.

[SethTisue]

I largely agree with what you guys are saying, but I also want to add that ASM 9 came out more than four months ago, so there's no real reason the upgrade couldn't or shouldn't have made 2.12.13 + 2.13.4. ASM is only bearing on release timing decisions currently because we were so tardy there. (We were even tardier in the case of JDK 15, again for no good reason. Let's do even better next time.)

[dwijnand]

I also want to add that ASM 9 came out more than four months ago, so there's no real reason the upgrade couldn't or shouldn't have made 2.12.13 + 2.13.4.

Ah, that's my mistake. I didn't realise how scala-asm + ASM worked ("The only other dependenncy in versions.properties is scala-asm, which is latest." in scala/scala#9294 🙄...). Amended the release docs: scala/scala-dev@aca8a3b.

[mkurz]

@SethTisue ASM 9.1 was released yesterday: https://asm.ow2.io/versions.html It would be great if you could upgrade ASM once more, thanks!

[SethTisue]

@mkurz will do: #12332