Skip to content

Fix #480 in LambdaLift #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 17, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 42 additions & 15 deletions src/dotty/tools/dotc/transform/LambdaLift.scala
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
free.getOrElse(sym, Nil).toList.map(pm)
}

/** Set `liftedOwner(sym)` to `owner` if `owner` is more deeply nested
* than the previous value of `liftedowner(sym)`.
*/
def narrowLiftedOwner(sym: Symbol, owner: Symbol)(implicit ctx: Context) = {
if (sym.owner.isTerm &&
owner.isProperlyContainedIn(liftedOwner(sym)) &&
Expand All @@ -100,11 +103,22 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
}
}

/** Mark symbol `sym` as being free in `enclosure`, unless `sym`
* is defined in `enclosure` or there is a class between `enclosure`s owner
* and the owner of `sym`.
* Return `true` if there is no class between `enclosure` and
* the owner of sym.
/** Mark symbol `sym` as being free in `enclosure`, unless `sym` is defined
* in `enclosure` or there is an intermediate class properly containing `enclosure`
* in which `sym` is also free. Also, update `liftedOwner` of `enclosure` so
* that `enclosure` can access `sym`, or its proxy in an intermediate class.
* This means:
*
* 1. If there is an intermediate class in which `sym` is free, `enclosure`
* must be contained in that class (in order to access the `sym proxy stored
* in the class).
*
* 2. If there is no intermediate class, `enclosure` must be contained
* in the class enclosing `sym`.
*
* Return the closest enclosing intermediate class between `enclosure` and
* the owner of sym, or NoSymbol if none exists.
*
* pre: sym.owner.isTerm, (enclosure.isMethod || enclosure.isClass)
*
* The idea of `markFree` is illustrated with an example:
Expand All @@ -130,22 +144,30 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
* }
* }
*/
private def markFree(sym: Symbol, enclosure: Symbol)(implicit ctx: Context): Boolean = try {
private def markFree(sym: Symbol, enclosure: Symbol)(implicit ctx: Context): Symbol = try {
if (!enclosure.exists) throw new NoPath
ctx.log(i"mark free: ${sym.showLocated} with owner ${sym.maybeOwner} marked free in $enclosure")
narrowLiftedOwner(enclosure, sym.enclosingClass)
(enclosure == sym.enclosure) || {
if (enclosure == sym.enclosure) NoSymbol
else {
ctx.log(i"mark free: ${sym.showLocated} with owner ${sym.maybeOwner} marked free in $enclosure")
ctx.debuglog(i"$enclosure != ${sym.enclosure}")
if (enclosure.is(PackageClass) ||
!markFree(sym, enclosure.skipConstructor.enclosure)) false
val intermediate =
if (enclosure.is(PackageClass)) enclosure
else markFree(sym, enclosure.skipConstructor.enclosure)
// `enclosure` might be a constructor, in which case we want the enclosure
// of the enclosing class, so skipConstructor is needed here.
if (intermediate.exists) {
narrowLiftedOwner(enclosure, intermediate)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you either add documentation to narrowLiftedOwner method, or add description what happens in both branches of this if?

intermediate
}
else {
narrowLiftedOwner(enclosure, sym.enclosingClass)
val ss = symSet(free, enclosure)
if (!ss(sym)) {
ss += sym
changedFreeVars = true
ctx.debuglog(i"$sym is free in $enclosure")
}
!enclosure.isClass
if (enclosure.isClass) enclosure else NoSymbol
}
}
} catch {
Expand Down Expand Up @@ -272,7 +294,7 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
local.copySymDenotation(
owner = newOwner,
name = newName(local),
initFlags = local.flags | Private | maybeStatic | maybeNotJavaPrivate,
initFlags = local.flags &~ InSuperCall | Private | maybeStatic | maybeNotJavaPrivate,
info = liftedInfo(local)).installAfter(thisTransform)
if (local.isClass)
for (member <- local.asClass.info.decls)
Expand Down Expand Up @@ -372,8 +394,13 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
val sym = tree.symbol
tree.tpe match {
case tpe @ TermRef(prefix, _) =>
if ((prefix eq NoPrefix) && sym.enclosure != currentEnclosure && !sym.isStatic)
(if (sym is Method) memberRef(sym) else proxyRef(sym)).withPos(tree.pos)
if (prefix eq NoPrefix)
if (sym.enclosure != currentEnclosure && !sym.isStatic)
(if (sym is Method) memberRef(sym) else proxyRef(sym)).withPos(tree.pos)
else if (sym.owner.isClass) // sym was lifted out
ref(sym).withPos(tree.pos)
else
tree
else if (!prefixIsElidable(tpe)) ref(tpe)
else tree
case _ =>
Expand Down
10 changes: 10 additions & 0 deletions tests/pos/i342.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
object Test {
def test2: Int = {
var ds: String = null
def s = {
ds = "abs"
ds
}
s.length
}
}
4 changes: 4 additions & 0 deletions tests/pos/i480.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object O {
val x: Function1[String, String] = a => a
val x2: Function1[String, String] = a => "1"
}
15 changes: 15 additions & 0 deletions tests/pos/i480a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package test

/** A class defining symbols and types of standard definitions */
class Definitions {

trait LazyType { def complete(): Unit }

def f(vcs: List[Int]): Unit = {
val completer = new LazyType {
def complete(): Unit =
for (i <- 0 until vcs.length if vcs(i) != 0)
f(vcs.updated(i, 0))
}
}
}