bug
bug copied to clipboard
scala
Compilation order triggers appearance of unexpected annotations on class members
Having the following project structure
.
├── README.md
├── build.sbt
├── core
│ └── src
│ └── main
│ └── scala
│ ├── Main.scala
│ ├── bar
│ │ └── Bar.scala
│ ├── baz
│ │ └── description.scala
│ └── foo
│ └── Foo.scala
├── generic
│ └── src
│ └── main
│ └── scala
│ └── Gen.scala
└── project
├── build.properties
└── project
13 directories, 8 files
Main.scala
import bar.Bar
import foo.Foo
object Main {
Bar.bar(null)
Gen.derive[Foo]
}
Bar.scala
package bar
import foo.Foo
object Bar {
def bar(foo: Foo) = foo.foo
}
description.scala
package baz
import scala.annotation.StaticAnnotation
class description(val text: String) extends StaticAnnotation
Foo.scala
package foo
import baz.description
case class Foo(
@description("foo")
foo: Int
)
Gen.scala
import scala.language.experimental.macros
import scala.reflect.macros._
object Gen {
def derive[T]: Unit = macro gen[T]
def gen[T: c.WeakTypeTag](c: whitebox.Context): c.Tree = {
import c.universe._
val classMembers = weakTypeOf[T].typeSymbol.asType.toType.members
val annotationsOnConstructorParams =
classMembers
.filter(_.isConstructor)
.flatMap(_.asMethod.paramLists.flatten)
.flatMap(_.annotations)
annotationsOnConstructorParams.foreach(_.tree) // touch annotations to evaluate lazy info
println(
s"Annotations on constructor: $annotationsOnConstructorParams"
)
classMembers
.filter(_.toString.contains("foo")) // Take only "foo" TermSymbol and MethodSymbol
.foreach { s =>
val annotations = s.annotations
annotations.foreach(_.tree) // touch annotations to evaluate lazy info
println(s"Annotations: $annotations, is method? - ${s.isMethod}")
/*
The output of the program shows the following:
Annotations on constructor: List(baz.description("foo"))
Annotations: List(<notype>), is method? - false
Annotations: List(<notype>), is method? - true
Moreover, these annotations are the instances of
scala.reflect.internal.AnnotationInfos.UnmappableAnnotation
*/
/*
On the other hand, commenting out line 6 in Main.scala and running ;clean;compile shows
Annotations on constructor: List(baz.description("foo"))
Annotations: List(), is method? - false
Annotations: List(), is method? - true
*/
}
q"()"
}
}
build.sbt
ThisBuild / version := "0.1.0-SNAPSHOT"
ThisBuild / scalaVersion := "2.13.8"
lazy val root = (project in file("."))
.settings(
name := "annotations-bug"
)
.aggregate(core, generic)
lazy val core = (project in file("core"))
.settings(
name := "core"
)
.dependsOn(generic)
lazy val generic = (project in file("generic")).settings(
name := "generic",
libraryDependencies ++= Seq(
"org.scala-lang" % "scala-reflect" % scalaVersion.value
)
)
build.properties
sbt.version = 1.6.2
It seems that compilation order can change the list of annotations that can be seen from the macro. It the example above there are two key parts - macro module and the ordinary code.
In the macro module annotations collection is performed and annotations from constructor parameters, method symbols and term symbols are printed. According to the documentation (if I understand it correctly), @description annotation should appear only on constructor parameters and not on the class fields or getters of these fields. But running the example shows that unexpectedly some annotations appear on both class field and getter. Moreover, these annotations are the instances of scala.reflect.internal.AnnotationInfos.UnmappableAnnotation, which, in case of using in the output tree, fails the compilation.
The triggering point for the behavior is line 6 in Main.scala. Commenting it out and compiling project from scratch will print only expected one annotation on constructor parameter.
The code in the example available here but also copy-pasted to the issue for easier referencing. I would like to say thank you to @vladislavsheludchenkov for the great help in chasing the issue and writing minimal reproducer for it.
We've found another reproducer for the issue, it can be found in the repository.
Also copy-pasting it here
Project structure
.
├── README.md
├── build.sbt
├── core
│ └── src
│ └── main
│ └── scala
│ ├── Main.scala
│ └── foo
│ └── Foo.scala
├── generic
│ └── src
│ └── main
│ └── scala
│ └── Gen.scala
└── project
├── build.properties
└── project
11 directories, 6 files
build.sbt
ThisBuild / version := "0.1.0-SNAPSHOT"
ThisBuild / scalaVersion := "2.13.8"
lazy val root = (project in file("."))
.settings(
name := "annotations-bug-2"
)
.aggregate(core, generic)
lazy val core = (project in file("core"))
.settings(
name := "core"
)
.dependsOn(generic)
lazy val generic = (project in file("generic")).settings(
name := "generic",
libraryDependencies ++= Seq(
"org.scala-lang" % "scala-reflect" % scalaVersion.value
)
)
build.properties
sbt.version = 1.6.2
Gen.scala
import scala.language.experimental.macros
import scala.reflect.macros._
object Gen {
def derive[T]: Unit = macro gen[T]
def gen[T: c.WeakTypeTag](c: whitebox.Context): c.Tree = {
import c.universe._
val clazz = weakTypeOf[T].typeSymbol.asType.toType
val annotations = clazz.baseClasses
.flatMap { c =>
c.asType.toType.members
.filter(s => s.isTerm || s.isMethod)
.filter(_.toString.contains("foo"))
.flatMap(_.annotations)
}
annotations.foreach(_.tree) // touch annotations to evaluate lazy info
println(annotations) // prints List(<notype>, <notype>, <notype>, <notype>)
q"()"
}
}
Foo.scala
package foo
import scala.annotation.StaticAnnotation
class description(val text: String) extends StaticAnnotation
class Base(
@description("base foo")
val foo: String
)
case class Foo(
@description("foo!")
override val foo: String
) extends Base(foo)
Main.scala
import foo.Foo
object Main {
Gen.derive[Foo]
}
You've done a great job of isolating this. I wish I had some insight to offer into what the root cause might be. Are you planning to explore possible fixes yourself?
Moreover, these annotations are the instances of scala.reflect.internal.AnnotationInfos.UnmappableAnnotation, which, in case of using in the output tree, fails the compilation
Are you able to work around it by explicitly filtering those out?
Are you planning to explore possible fixes yourself?
It will be a great experience but we've not touched compiler code before - so, some insights about where to look for the issue will be very helpful
Are you able to work around it by explicitly filtering those out?
Yes, the workaround in magnolia is filtering NoType annotations: https://github.com/softwaremill/magnolia/pull/387/files#diff-702351a61e9bf40e33ae333369143a129284e6b9c2bb68a6f3621638045f6bc7R181
Is the problem reproducible without involving sbt, by doing the multiple rounds of compilation by invoking scalac multiple times?
If sbt and zinc can be cut out of the picture, that would narrow down the space of possible causes. It would also be a necessary step if we want to add a regression test for the problem to scala/scala.
It's unfortunate macros end up seeing UnmappableAnnotation. This was done dealay "forcing" of annotations, i.e., delay type-checking them. Doing it early, when Namers creates the Symbol of the definition carrying the annotation, would otherwise too often lead to "cyclic references", i.e., trying to get the type of something whose type is being computed. So the sym.annotations initially holds a list of non-completed annotations.
Annotations on class parameters may need to be copied to derived symbols: the constructor param, the field, the getter. That depends on the "target" annotations (https://github.com/scala/scala/blob/2.13.x/src/library/scala/annotation/meta/package.scala). We can only know where the annotation needs to go once it's completed. The current implementation puts lazy annotations on all targets, and then completes the ones that need to be dropped as UnmappableAnnotation.
So filtering out UnmappableAnnotation is correct.
So if it is an implementation limitation, is there a way to provide a more convenient interface for macros? Currently, ExtraLazyAnnotationInfo doesn't expose any info about actual AnnotationInfo type even after evaluation, as we've had to look into it with a debugger to figure out what's going on. That's why Magnolia has to compare the .tree result with NoType to filter out instances of ExtraLazyAnnotationInfo that are evaluated as UnmappableAnnotation.
Can we introduce a convenience method with result type Option[Tree] or Option[AnnotationInfo]?
We could maybe add def completedAnnotations next to def annotations. The implementation in internal/Symbols would be
def completedAnnotations = {
annotations.foreach(_.completeInfo())
setAnnotations(annotations.filter(_ != UnmappableAnnotation))
annotations
}
Sounds great! Does it look like a good first issue? If so, I'd like to prepare a PR for that.
Also, I'd like to see it in both 2.12 and 2.13, but I'm not sure if it'd be considered a new feature or a bug fix and if it would be reasonable to introduce the change to 2.12. What do you think?
Forwards binary compatibility might be the dealbreaker, not allowing us to add anything to reflect/api/Symbols... Adding it to internal/Symbols would be allowed (https://github.com/scala/scala/blob/0ef047b4036db8c5799de0ffaab11fc0d58fdbcf/project/MimaFilters.scala#L21), but wouldn't help for macros (unless you cast the universe to the compiler-internal SymbolTable...)
Why forwards compatibility matters in this case? reflect/api/Symbols is inside scala-reflect-2.13.* library. Should it guarantee forwards compatibility?
@danielleontiev yes; see https://docs.scala-lang.org/overviews/core/binary-compatibility-of-scala-releases.html