Lift arguments of explicitly constructed annotations #22553
Conversation
mbovel
force-pushed
the
mb/annot-default-new
branch
2 times, most recently
from
8cc0706 to
a9557b6
Compare
mbovel
force-pushed
the
mb/annot-default-new
branch
from
a9557b6 to
5f8ae12
Compare
| @@ -13,3 +13,6 @@ def test = | |||
| @annot(44) val z3 = 45 | |||
| @annot2(y = Array("Hello", y)) val z4 = 45 | |||
|
|
|||
| // Arguments are still lifted if the annotation class is instantiated | |||
| // explicitly. See #22526. | |||
| val z5 = new annot2(y = Array("World"), x = 1) | |||
There was a problem hiding this comment.
What happens on @annot(y = new A, x = new annot(y = new B, x = new C))?
I think in principle, the AST should be
new annot(
x = { val y$1 = new B; val x$1 = new C; new annot(x = x$1, y = y$1) }
y = new A)There was a problem hiding this comment.
I have just pushed a commit that addresses that.
I can agree with the principle in this case. But in general, it should be noted that trees in annotation arguments are currently not guaranteed to be valid by any definition. We should spec it.
There was a problem hiding this comment.
Looks good to me.
The Scala 3 typer seems to use modes a bit different than Scala 2.
Scala 2 clears mode bits in several places, e.g. in typedArg. Mode flags can be declared to stay active when passing those thresholds. I didn't spot anything like that in Scala 3.
Scala 3 also seems to use expected types (FunProto, PolyProto) for things that are done with modes in Scala 2 (FUNmode, POLYmode).
I'm just saying I'm not familiar with modes in Scala 3, so maybe someone else should also review this.
| if wideFormal eq formal then ctx.retractMode(Mode.InAnnotation) | ||
| else ctx.retractMode(Mode.InAnnotation).withNotNullInfos(ctx.notNullInfos.retractMutables) |
There was a problem hiding this comment.
Can this flag be added to argCtx in
instead ? That seems more generic. Also the reason for doing that should be documented (and the flag InAnnotation should document that it's turned off for arguments).There was a problem hiding this comment.
Can this flag be added to
argCtx
No, that would remove the mode too early. argCtx is passed to ApplyToUntyped and is then used from the initializer of the parent class TypedApply, where we still need the mode to be set when typing an annotation.
So I think it's only in ApplyToUntyped.typedArg that we can remove the flag, which directly calls ProtoTypes.typedArg.
There was a problem hiding this comment.
I added documentation, tell me if that's clearer.
mbovel
force-pushed
the
mb/annot-default-new
branch
from
7d1b62a to
337856c
Compare
Fixes #22526.