Fixes for lambda lift #1133
Fixes for lambda lift #1133
Conversation
Test will come in llift.scala.
If lambda lift needs to create an outer path from a constructor, the path needs to start from the $outer parameter of the constructor, not the this of the enclosing class, which is not yet available.
|
Thanks! This helped enormously to develop a fix for what was blocking M4: see the last few commits that change LambdaLift of https://github.com/scala/scala/compare/2.12.x...adriaanm:traits-fields-lambda?expand=1 (WIP, but included test case is passing) |
Simplifications in order to avoid the freqent special casing of constructors and prepare the way for proper handling of trait constructors (which cause problems; see pending/pos/llift.scala.
odersky
force-pushed
the
change-lambdalift
branch
from
285974d to
25da215
Compare
|
@adriaanm I discovered more problems which are all reflected in run/llift.scala. I had to touch quite a lot of places in LambdaLift to make these work. Fortunately, they make the logic less quirky, because |
| } else Nil | ||
| } | ||
|
|
||
| /** The path of outer accessors that references `toCls.this` starting from |
There was a problem hiding this comment.
Comment should be updated to say "starting from the node start" I think.
|
Cool, looks much nicer! I'll port these over hopefully tomorrow. |
odersky
changed the title
1. Make clearer what markFree is supposed to do and get rid of `propagated` mode bit. 2. Harden copyParams so that we make sure corresponding parameters and fields are copied.
|
Now everything is done. @adriaanm do you want to review? |
|
Sure, happy to provide feedback while I'm backporting this. It'll be a few more days, though. I'm working on this on and off today, but traveling back to SF tomorrow. |
| // Conversely, local traits do not get free variables. | ||
| if (!enclosure.is(Trait)) { | ||
| val ss = symSet(free, enclosure) | ||
| if (!ss(sym)) { |
There was a problem hiding this comment.
The lines val ss = ....; if (...) ; ss += sym could be simplified to if (symSet(free, enclosure) add sym).
|
(I'll start with some easy suggestions, but I hope to provide some more high-level feedback once I've digested it all.) |
| def proxies(sym: Symbol): List[Symbol] = { | ||
| val pm: Map[Symbol, Symbol] = proxyMap.getOrElse(sym, Map.empty) // Dotty deviation: Type annotation needed. TODO: figure out why | ||
| free.getOrElse(sym, Nil).toList.map(pm) | ||
| def freeVars(sym: Symbol): List[Symbol] = free.getOrElse(sym, Nil).toList |
There was a problem hiding this comment.
Could we come up with a more descriptive name than free? storedCapturesOf?
There was a problem hiding this comment.
Not sure that's better.
based on scala/scala3#1133 lambdalift updates info of trait members, so must preserve this when mixing in
based on scala/scala3#1133 lambdalift updates info of trait members, so must preserve this when mixing in
based on scala/scala3#1133 lambdalift updates info of trait members, so must preserve this when mixing in
based on scala/scala3#1133 lambdalift updates info of trait members, so must preserve this when mixing in
Made some changes to fjbg so when people run into scala#1133 at least it will tell them what method was the cause. The fact that ten files are touched in this commit instead of one or two is a testament to the wonder of checked exceptions. No review.
Made some changes to fjbg so when people run into scala#1133 at least it will tell them what method was the cause. The fact that ten files are touched in this commit instead of one or two is a testament to the wonder of checked exceptions. No review.
3 participants
Lambda lift previously ignored the problem of local traits. The transformation would add fields
for any free variables of such traits, ignoring the problem that these fields cannot be transformed anymore because LambdaLift runs after addGetters and Mixin.
The problem is fixed by letting local traits not cache fields.