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

Binary Break Affecting Macro Annotations Compiled on 3.3 #22352

Closed
j-mie6 opened this issue Jan 12, 2025 · 7 comments
Closed

Binary Break Affecting Macro Annotations Compiled on 3.3 #22352

j-mie6 opened this issue Jan 12, 2025 · 7 comments

Comments

@j-mie6
Copy link

j-mie6 commented Jan 12, 2025

Compiler version

3.6.2 (but also applies as far back as 3.5.0, works on 3.4)

Minimized code

Using scala --dependency com.github.j-mie6::parsley-debug::5.0.0-M9

Then, use the annotation @parsley.debuggable, which is a macro annotation compiled on 3.3.1.

@parsley.debuggable object foo

Output (click arrow to expand)

  unhandled exception while running inlining on rs$line$1

  An unhandled exception was thrown in the compiler.
  Please file a crash report here:
  https://github.com/scala/scala3/issues/new/choose
  For non-enriched exceptions, compile with -Xno-enrich-error-messages.

     while compiling: rs$line$1
        during phase: inlining
                mode: Mode(ImplicitsEnabled,ReadPositions,Interactive)
     library version: version 2.13.15
    compiler version: version 3.6.2
            settings: -Xcook-docs true -Xread-docs true -classpath /home/jamie/.cache/coursier/arc/https/github.com/scala/scala3/releases/download/3.6.2/scala3-3.6.2-x86_64-pc-linux.tar.gz/scala3-3.6.2-x86_64-pc-linux/maven2/org/scala-lang/scala3-library_3/3.6.2/scala3-library_3-3.6.2.jar:/home/jamie/.cache/coursier/v1/https/repo1.maven.org/maven2/com/github/j-mie6/parsley-debug_3/5.0.0-M9/parsley-debug_3-5.0.0-M9.jar:/home/jamie/.cache/coursier/arc/https/github.com/scala/scala3/releases/download/3.6.2/scala3-3.6.2-x86_64-pc-linux.tar.gz/scala3-3.6.2-x86_64-pc-linux/maven2/org/scala-lang/scala-library/2.13.15/scala-library-2.13.15.jar:/home/jamie/.cache/coursier/v1/https/repo1.maven.org/maven2/com/github/j-mie6/parsley_3/5.0.0-M9/parsley_3-5.0.0-M9.jar -d <REPL compilation output>

Exception in thread "main" java.lang.reflect.InvocationTargetException
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:118)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at scala.reflect.Selectable.applyDynamic(Selectable.scala:40)
        at scala.reflect.Selectable.applyDynamic$(Selectable.scala:11)
        at scala.reflect.Selectable$DefaultSelectable.applyDynamic(Selectable.scala:51)
        at dotty.tools.dotc.transform.MacroAnnotations$.callMacro(MacroAnnotations.scala:113)
        at dotty.tools.dotc.transform.MacroAnnotations$.$anonfun$1(MacroAnnotations.scala:68)
        at scala.collection.LinearSeqOps.foldLeft(LinearSeq.scala:183)
        at scala.collection.LinearSeqOps.foldLeft$(LinearSeq.scala:179)
        at scala.collection.immutable.List.foldLeft(List.scala:79)
        at dotty.tools.dotc.transform.MacroAnnotations$.expandAnnotations(MacroAnnotations.scala:63)
        at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transformMemberDef(Inlining.scala:115)
        at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:71)
        at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.loop$2(tpd.scala:1265)
        at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1265)
        at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1267)
        at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:58)
        at dotty.tools.dotc.ast.TreeMapWithTrackedStats.transform(TreeMapWithTrackedStats.scala:63)
        at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:95)
        at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1605)
        at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:67)
        at dotty.tools.dotc.ast.TreeMapWithTrackedStats.transform(TreeMapWithTrackedStats.scala:60)
        at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transformMemberDef(Inlining.scala:135)
        at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:71)
        at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.loop$2(tpd.scala:1265)
        at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1265)
        at dotty.tools.dotc.ast.tpd$TreeMapWithPreciseStatContexts.transformStats(tpd.scala:1267)
        at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1616)
        at dotty.tools.dotc.ast.TreeMapWithImplicits.transform(TreeMapWithImplicits.scala:67)
        at dotty.tools.dotc.ast.TreeMapWithTrackedStats.transform(TreeMapWithTrackedStats.scala:48)
        at dotty.tools.dotc.transform.Inlining$InliningTreeMap.transform(Inlining.scala:75)
        at dotty.tools.dotc.transform.Inlining$$anon$2.transform(Inlining.scala:57)
        at dotty.tools.dotc.transform.MacroTransform.run(MacroTransform.scala:20)
        at dotty.tools.dotc.transform.Inlining.run(Inlining.scala:39)
        at dotty.tools.dotc.core.Phases$Phase.runOn$$anonfun$1(Phases.scala:380)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.collection.immutable.List.foreach(List.scala:334)
        at dotty.tools.dotc.core.Phases$Phase.runOn(Phases.scala:373)
        at dotty.tools.dotc.Run.runPhases$1$$anonfun$1(Run.scala:343)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.collection.ArrayOps$.foreach$extension(ArrayOps.scala:1323)
        at dotty.tools.dotc.Run.runPhases$1(Run.scala:336)
        at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:384)
        at dotty.tools.dotc.Run.compileUnits$$anonfun$adapted$1(Run.scala:396)
        at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:69)
        at dotty.tools.dotc.Run.compileUnits(Run.scala:396)
        at dotty.tools.dotc.Run.compileUnits(Run.scala:288)
        at dotty.tools.repl.ReplCompiler.compile(ReplCompiler.scala:88)
        at dotty.tools.repl.ReplDriver.compile(ReplDriver.scala:334)
        at dotty.tools.repl.ReplDriver.interpret(ReplDriver.scala:296)
        at dotty.tools.repl.ReplDriver.loop$1(ReplDriver.scala:209)
        at dotty.tools.repl.ReplDriver.runUntilQuit$$anonfun$1(ReplDriver.scala:212)
        at dotty.tools.repl.ReplDriver.withRedirectedOutput(ReplDriver.scala:251)
        at dotty.tools.repl.ReplDriver.runBody$$anonfun$1(ReplDriver.scala:225)
        at dotty.tools.runner.ScalaClassLoader$.asContext(ScalaClassLoader.scala:80)
        at dotty.tools.repl.ReplDriver.runBody(ReplDriver.scala:225)
        at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:212)
        at dotty.tools.repl.ReplDriver.tryRunning(ReplDriver.scala:149)
        at dotty.tools.repl.Main$.main(Main.scala:7)
        at dotty.tools.repl.Main.main(Main.scala)
Caused by: java.lang.AbstractMethodError
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        ... 61 more

This also happens outside the REPL. From my understanding of AbstractMethodError, this means the binary API of macro-annotations was broken from 3.3/3.4 at 3.5.

@j-mie6 j-mie6 added itype:bug itype:crash stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 12, 2025
@j-mie6
Copy link
Author

j-mie6 commented Jan 12, 2025

A quick check within the parsley codebase suggests:

@experimental class debuggable extends MacroAnnotation {
[error]    |                    ^
[error]    |class debuggable needs to be abstract, since def transform
[error]    |  (using x$1: scala.quoted.Quotes)
[error]    |    (definition: x$1.reflect.Definition, companion:
[error]    |      Option[x$1.reflect.Definition]): List[x$1.reflect.Definition] in trait MacroAnnotation in package scala.annotation is not defined 
[error]    |(The class implements a member with a different type: def transform
[error]    |  (using x$1: scala.quoted.Quotes)
[error]    |    (tree: x$1.reflect.Definition): List[x$1.reflect.Definition] in class debuggable in package parsley)

Which is indeed a binary break. Is it even possible to resolve this? If the API for macro-annotations is broken from the LTS 3.3 series, then basically no cross-compiled libraries would be able to use them!

I would guess this is possible to resolve by making this method non-abstract and have a forwarder to the new method with a default argument provided.

@j-mie6 j-mie6 changed the title Possible Binary Break Affecting Macro Annotations Binary Break Affecting Macro Annotations Compiled on 3.3 Jan 12, 2025
@hamzaremmal
Copy link
Member

@j-mie6 MacroAnnotation is an experimental API. It has no guaranteed backward binary or source compatibility. See the documentation about @experimental here.

@j-mie6
Copy link
Author

j-mie6 commented Jan 12, 2025

I understand that, however, in this case this is fixable with a simple forwarder method, I believe.

@hamzaremmal
Copy link
Member

Sure, but that's not the API we want to have (for now), see SIP-63.

@j-mie6
Copy link
Author

j-mie6 commented Jan 13, 2025

As a mitigation for Scala 3.3 targeting library authors, just add

def transform(using Quotes)(tree: quotes.reflect.Definition, companion: Option[quotes.reflect.Definition]): List[quotes.reflect.Definition] = transform(tree)

To your annotation class, it will work again

@Gedochao Gedochao added area:inline stat:needs minimization Needs a self contained minimization regression This worked in a previous version but doesn't anymore stat:needs bisection Need to use nightly builds and git bisect to find out the commit where this issue was introduced area:metaprogramming:macro-annotations area:experimental stat:wontfix and removed stat:needs minimization Needs a self contained minimization regression This worked in a previous version but doesn't anymore stat:needs triage Every issue needs to have an "area" and "itype" label stat:needs bisection Need to use nightly builds and git bisect to find out the commit where this issue was introduced labels Jan 13, 2025
@hamzaremmal
Copy link
Member

Just to add more context, here is the PR that changed this: #19677

@Gedochao
Copy link
Contributor

The change behind this was intentional, it is not an error.
Closing as wontfix.

@Gedochao Gedochao closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants