Fix AnnotationInfo when using defaults
lrytz opened this issue · 6 comments
scala> class ann(x: Int = 1, y: Int = 2) extends annotation.StaticAnnotation
class ann
scala> @ann(y = Nil.size) class K
^
warning: Usage of named or default arguments transformed this annotation
constructor call into a block. The corresponding AnnotationInfo
will contain references to local values and default getters instead
of the actual argument trees
class K
scala> typeOf[K].typeSymbol.annotations.head
val res0: $r.intp.global.AnnotationInfo = ann(x$2, x$1)
scala> typeOf[K].typeSymbol.annotations.head.original
val res1: $r.intp.global.Tree = <empty>Before typing, the annotation is represented as new ann(y = Nil.size). To construct the AnnotationInfo, that expression is type checked like ordinary code, resulting in
{
val x$1 = Nil.size
val x$2 = ann.<init>$default$1
new ann(x$2, x$1)
}The AnnotationInfo only has ann(x$2, x$1), the block is thrown away it seems, there's now way get to the actual arguments.
Ideas
- Minimum: attach the typed Block AST to the
AnnotationInfo. But IMO this is not a good solution, it leaves processing annotations too difficult. - Never create blocks in NamesDefaults when typing an annotation. Evaluation order is not a concern, so
new ann(y = Nil.size)can be typed asnew ann(<init>$default$1, Nil.size) - Improve handling of defaults. It would be great if default values defined in the annotation declaration ended up in the
AnnotationInfo, i.e.,new ann(1, Nil.size).
For the last one, the question is how to get to the default value AST. One option is to make the compiler attach it to to a symbol, so the class ann above would become
class ann(@defaultArg(1) x: Int, @defaultArg(2) y: Int) extends StaticAnnotation
object ann {
<synthetic> def <init>$default$1 : Int = 1
<synthetic> def <init>$default$2 : Int = 2
}
When constructing the AnnotationInfo from new ann(<init>$default$1, Nil.size) we can get the AST for the default from the annotation parameter symbol.
Earlier tickets: scala/bug#7656, scala/bug#9612
I did some prototyping when discussing @apiStatus (scala/scala#8820 / scala/scala@2.13.x...lrytz:constAnnDefaults). The goal here was to allow subclasses of annotations to define certain defaults, which would be great for @nowarn: scala/bug#12367.
Fixing that at the same time would be good.
There was a comment on dotty that they might re-engineer defaults to be inline. Your bullet 3 is very appealing, or to coin a phrase, "it would be great if".
A workaround, at least for the case when the annotation parameters have distinct types, is defining constructor overloads:
class ann(x: Int, y: String) extends annotation.StaticAnnotation {
def this(x: Int) = this(x, null)
def this(s: String) = this(null, s)
}scala> @ann(s = "ka") class K
scala> typeOf[K].typeSymbol.annotations.head
val res0: $r.intp.global.AnnotationInfo = ann("ka")Another item to fix - though it will be less relevant after the other fixes. For an annotation with auxiliary constructors, the AnnotationInfo doesn't tell which overload was selected. Probably just add a constructorSymbol to the AnnotationInfo.
... sometimes the simple ideas are not that simple.
https://github.com/scala/scala/compare/2.13.x...lrytz:scala:annot-overload?expand=1 adds the constructor symbol to AnnotationInfo, but
- we cannot pickle / unpickle a reference to a method / constructor symbol, at least not easily (would need overload resolution in unpickling)
- we cannot change the pickle format
What the user really needs is to match up the args: List[Tree] with the corresponding parameter names
- we could include the parameter names, we could even re-use the
assocs: List[(Name, ClassfileAnnotArg)]for that which we could pickle. Thoughassert(args.isEmpty || assocs.isEmpty, atp)inCompleteAnnotationInfowould fail when an older Scala unpickles... - add a method that type-checks a
new Ann(args: _*)AST and extracts the resolved constructor symbol from there - attach the parameter names to the
argsASTs as annotations somehow
chatted with Lukas about this today. he clarified that he doesn't think we should continue down the path of attempting to pickle a method/constructor symbol
and he pointed out that if we fix this, it would make it possible for users to e.g. have a custom subclass of @nowarn that fixes some of the parameters. this would be useful if people are doing the same @nowarn invocations over and over again
One option would be to add this method to AnnotationInfo:
def constructorSymbol(typer: Tree => Tree): Symbol = {
typer(New(atp, args: _*)) match {
case Apply(constr @ Select(New(_), nme.CONSTRUCTOR), _) => constr.symbol
case _ => atp.typeSymbol.primaryConstructor
}
}The users have to provide a type checker, so basically call it as annot.constructorSymbol(exitingTyper(typer.typed(_))). The reason is that AnnotationInfo lives in the SymbolTable where we don't have access to Typer. Compiler plugins that work with AnnotationInfo have a typer available.
I tested it, it works well. That seems to be the most straightforward solution.