scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

2.13.12 regression? Third-party macro gives `NoSuchMethodError` in void `Trees$TypeTreeWithDeferredRefCheck` #12862

Closed bursauxa closed 1 year ago

bursauxa commented 1 year ago

Reproduction steps

Problem

The project no longer compiles. Relevant stack trace:

[error] java.lang.NoSuchMethodError: 'void scala.tools.nsc.ast.Trees$TypeTreeWithDeferredRefCheck.(scala.tools.nsc.Global, scala.Function0)' [error] ai.x.play.json.Macros.verifyKnownDirectSubclassesPostTyper(play-json.scala:353) [error] ai.x.play.json.Macros.formatSealedInternal(play-json.scala:444) [error] ai.x.play.json.Macros.formatSealed(play-json.scala:396) [error] java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) [error] java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) [error] java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) [error] java.base/java.lang.reflect.Method.invoke(Method.java:566) [error] scala.reflect.macros.runtime.JavaReflectionRuntimes$JavaReflectionResolvers.$anonfun$resolveJavaReflectionRuntime$5(JavaReflectionRuntimes.scala:45) [error] scala.tools.nsc.typechecker.Macros.macroExpandWithRuntime(Macros.scala:796)

This looks like a breaking change with a method that was removed (or renamed, replaced...).

SethTisue commented 1 year ago

Thank you for the report.

Note that it's normal for internal compiler APIs such as scala.tools.nsc.ast.Trees to change in Scala 2 minor releases. (Scala 2 version numbers are epic.major.minor.)

Regardless, it's certainly possible we'll conclude we broke compatibility in a way we shouldn't have and that might be reversible. But deciding that would involve some back-and-forth about specifics with the authors of the macro in question (or with someone who understands the macro well, if the authors aren't available).

SethTisue commented 1 year ago

@lrytz @som-snytt does the exception spark any guesses at what the responsible Scala PR might have been? scala/scala#10494 , perhaps?

som-snytt commented 1 year ago

That's the one. IIRC I needed the field for development and there was a comment to add it. I don't see that it's exploited in that ticket, however.

lrytz commented 1 year ago

c.universe.asInstanceOf[scala.tools.nsc.Global] and the library is not fully cross-built 😕

The library doesn't seem to be active, the last commit was 3.5 years ago - @caente is that project still relevant to you?

SethTisue commented 1 year ago

c.universe.asInstanceOf[scala.tools.nsc.Global] and the library is not fully cross-built 😕

Ah, okay, it's an outlaw twice over, then. So I suspect we will end up simply closing this ticket.

bursauxa commented 1 year ago

Ah, okay, it's an outlaw twice over, then. So I suspect we will end up simply closing this ticket.

Although it is problematic for us down the line, I can understand the rationale for that. Just let us know when you have reached a decision.

SethTisue commented 1 year ago

Sorry! If you fork the repo and have questions about how to make it work with 2.13.12, let us know.

bursauxa commented 1 year ago

It is unlikely we will do that. Probably we stick with 2.13.11, and wait for Scala 3 migration to be possible for us, as the 22 field case class limit is resolved with it (and we can drop the ai.x library).

Thanks for the heads-up @SethTisue .

rtyley commented 10 months ago

We've also encountered this issue - we're currently trying to figure out what to do, as we're not just using the play-json-extensions library for its 22 field case class limit fix, we're also using it for other functionality like the Jsonx.formatSealed[] method - quite a big code change to get from Scala 2.13.11 to 2.13.12.

som-snytt commented 10 months ago

Today is the 1-year anniversary of the PR to update versions on that project. :tada:

https://github.com/bizzabo/play-json-extensions/pull/92

It looks like the sneaky internal API usage is due to retronym, but it's in service of verifyKnownDirectSubclassesPostTyper which precedes Miles Sabin's work. Even if the project were still supporting 2.11, the hack would be unnecessary, perhaps.

Since the project is all of two source files, I'd suggest ingesting them, deleting the unnecessary method, updating versions as desired.