SI-9920 Avoid linkage errors with captured local objects + self types#5397
SI-9920 Avoid linkage errors with captured local objects + self types#5397adriaanm merged 1 commit intoscala:2.12.0from
Conversation
|
TODO:
|
|
Review by @lrytz |
|
Sorry about the lengthy comment... I modified the example a little bit and moved the object In this case there should not be a cast, and indeed we get This is correct, but it seems to conflict with what you say in the commit message: "An outer parameter of a nested class is typed with the self type of the enclosing class". When I spin up the debugger in When using the example that your patch actually fixes (move object We enter method So I'm wondering if that's all intended.. |
I don't think there is a conflict (maybe my commit message was ambiguous and you've interpreted it differently?) The outer parameter of the nested class |
|
I guess it is more accurate to say that the outer parameter is typed with the |
|
My concern is the following: scala> trait T { _: C => class D }; class C
defined trait T
defined class C
scala> val entering = enteringErasure(typeOf[T#D].member(newTermName("$outer ")).info)
entering: $r.intp.global.Type = <refinement>.type
scala> val exiting = exitingErasure(typeOf[T#D].member(newTermName("$outer ")).info)
exiting: $r.intp.global.Type = C
scala> val c = symbolOf[C]
c: $r.intp.global.TypeSymbol = class C
scala> entering.typeSymbol.isNonBottomSubClass(c)
res31: Boolean = false
scala> exiting.typeSymbol.isNonBottomSubClass(c)
res32: Boolean = trueYour patch uses the check happens to be executed For the example that your patch fixes, the check happens to be executed This looks a bit like we're just lucky. |
|
@lrytz Thanks for digging deep. Another irregularity that I don't understand is why inner classes and traits differ in whether they use the self type of the class type of the outer class. |
|
I don't know if it's the same problem, but all tests in MiMa fail with an AbstractMethodError in both, RC1 and this version: Update: This could be a classpath problem in MiMa instead. I'm still working on a standalone reproduction. |
|
Never mind, the MiMa problem was indeed caused by mixing the wrong classpaths. |
3cac6be to
e9765d9
Compare
|
@lrytz I've pushed a new patch that changes the caller to |
|
BTW, @szeiger, sbt's error output is humongous, check https://scala-ci.typesafe.com/job/scala-2.12.0-validate-test/29/console |
|
@lrytz That's the downside of running all tests and then printing all results. I should be able to prune it down quite a bit by not printing the same failure twice. |
|
Minimized regression by the latest diff: crashes the compiler with I'll take a look at it today. |
|
We have a phase ordering issue with recursive local methods, not sure how to get around it. Given: LambdaLift's The problem is that only At this point, Note that this check / inserting cast is exactly the logic that fixes the actual bug. It would work to change add |
An outer parameter of a nested class is typed with the self type
of the enclosing class:
```
class C; trait T { _: C => def x = 42; class D { x } }
```
leads to:
```
class D extends Object {
def <init>($outer: C): T.this.D = {
D.super.<init>();
()
};
D.this.$outer().$asInstanceOf[T]().x();
```
Note that a cast is inserted before the call to `x`.
If we modify that a little, to instead capture a local module:
```
class C; trait T { _: C => def y { object O; class D { O } } }
```
Scala 2.11 used to generate (after lambdalift):
```
class D$1 extends Object {
def <init>($outer: C, O$module$1: runtime.VolatileObjectRef): C#D$1 = {
D$1.super.<init>();
()
};
D$1.this.$outer().O$1(O$module$1);
```
That isn't type correct, `D$1.this.$outer() : C` does not
have a member `O$1`.
However, the old trait encoding would rewrite this in mixin to:
```
T$class.O$1($outer, O$module$1);
```
Trait implementation methods also used to accept the self type:
```
trait T$class {
final <stable> def O$1($this: C, O$module$1: runtime.VolatileObjectRef): T$O$2.type
}
```
So the problem was hidden.
This commit changes replaces manual typecheckin of the selection in LambdaLift with
a use of the local (erasure) typer, which will add casts as needed.
For `run/t9220.scala`, this changes the post LambdaLift AST as follows:
```
class C1$1 extends Object {
def <init>($outer: C0, Local$module$1: runtime.VolatileObjectRef): T#C1$1 = {
C1$1.super.<init>();
()
};
- C1$1.this.$outer.Local$1(Local$module$1);
+ C1$1.this.$outer.$asInstanceOf[T]().Local$1(Local$module$1);
<synthetic> <paramaccessor> <artifact> private[this] val $outer: C0 = _;
<synthetic> <stable> <artifact> def $outer(): C0 = C1$1.this.$outer
}
```
e9765d9 to
e3e1e30
Compare
|
@lrytz Hmmm, it is tricky. I'm trying a middle ground between my first and second attempts: I'm manually adding the cast, but only doing this in |
|
LGTM |
|
@adriaanm will leave you to hit merge on this after you pass another set of eyes over it. |
|
👀 |
An outer parameter of a nested class is typed with the self type
of the enclosing class:
leads to:
Note that a cast is inserted before the call to
x.If we modify that a little, to instead capture a local module:
Scala 2.11 used to generate (after lambdalift):
That isn't type correct,
D$1.this.$outer() : Cdoes nothave a member
O$1.However, the old trait encoding would rewrite this in mixin to:
Trait implementation methods also used to accept the self type:
So the problem was hidden.
This commit changes replaces manual typecheckin of the selection in LambdaLift with
a use of the local (erasure) typer, which will add casts as needed.
For
run/t9220.scala, this changes the post LambdaLift AST as follows: