Skip to content

Commit

Permalink
SI-6863 Fix verify error in captured var inited from expr with try/catch
Browse files Browse the repository at this point in the history
If a captured var was inited from a try/catch we did something
reasonable. But if the var was inited from a more complicated expression
(if/else, a block, match/case, etc) that ended with
a try/catch then we didn't and we were generating faulty byte code.
This fix patches LambdaLift to add the missing cases.

For known simple expressions, the translation is just new *Ref(expr).

For try/catch, if/else, match/case, and blocks this recursively
walks down the internal result expressions to translate them. E.g.
if(cond) trueExpr else falseExpr becomes if(cone) translate(trueExpr)
else translate(falseExpr)

For unknown expression types, the translation is {val temp = expr; new
  *Ref(expr) }
  • Loading branch information
JamesIry committed Jan 25, 2013
1 parent 2fa859e commit 0b52a51
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 10 deletions.
41 changes: 33 additions & 8 deletions src/compiler/scala/tools/nsc/transform/LambdaLift.scala
Original file line number Diff line number Diff line change
Expand Up @@ -451,20 +451,45 @@ abstract class LambdaLift extends InfoTransform {
}
case arg => arg
}
/** Wrap expr argument in new *Ref(..) constructor, but make
* sure that Try expressions stay at toplevel.

/** Wrap expr argument in new *Ref(..) constructor. But try/catch
* is a problem because a throw will clear the stack and post catch
* we would expect the partially-constructed object to be on the stack
* for the call to init. So we recursively
* search for "leaf" result expressions where we know its safe
* to put the new *Ref(..) constructor or, if all else fails, transform
* an expr to { val temp=expr; new *Ref(temp) }.
* The reason we narrowly look for try/catch in captured var definitions
* is because other try/catch expression have already been lifted
* see SI-6863
*/
def refConstr(expr: Tree): Tree = expr match {
def refConstr(expr: Tree): Tree = typer.typedPos(expr.pos) {expr match {
// very simple expressions can be wrapped in a new *Ref(expr) because they can't have
// a try/catch in final expression position.
case Ident(_) | Apply(_, _) | Literal(_) | New(_) | Select(_, _) | Throw(_) | Assign(_, _) | ValDef(_, _, _, _) | Return(_) | EmptyTree =>
New(sym.tpe, expr)
case Try(block, catches, finalizer) =>
Try(refConstr(block), catches map refConstrCase, finalizer)
case Block(stats, expr) =>
Block(stats, refConstr(expr))
case If(cond, trueBranch, falseBranch) =>
If(cond, refConstr(trueBranch), refConstr(falseBranch))
case Match(selector, cases) =>
Match(selector, cases map refConstrCase)
// if we can't figure out what else to do, turn expr into {val temp1 = expr; new *Ref(temp1)} to avoid
// any possibility of try/catch in the *Ref constructor. This should be a safe tranformation as a default
// though it potentially wastes a variable slot. In particular this case handles LabelDefs.
case _ =>
New(sym.tpe, expr)
}
debuglog("assigning expr to temp: " + (expr.pos))
val tempSym = currentOwner.newValue(unit.freshTermName("temp"), expr.pos) setInfo expr.tpe
val tempDef = ValDef(tempSym, expr) setPos expr.pos
val tempRef = Ident(tempSym) setPos expr.pos
Block(tempDef, New(sym.tpe, tempRef))
}}
def refConstrCase(cdef: CaseDef): CaseDef =
CaseDef(cdef.pat, cdef.guard, refConstr(cdef.body))
treeCopy.ValDef(tree, mods, name, tpt1, typer.typedPos(rhs.pos) {
refConstr(constructorArg)
})

treeCopy.ValDef(tree, mods, name, tpt1, refConstr(constructorArg))
} else tree
case Return(Block(stats, value)) =>
Block(stats, treeCopy.Return(tree, value)) setType tree.tpe setPos tree.pos
Expand Down
2 changes: 0 additions & 2 deletions src/compiler/scala/tools/nsc/transform/UnCurry.scala
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,6 @@ abstract class UnCurry extends InfoTransform
}
case ValDef(_, _, _, rhs) =>
if (sym eq NoSymbol) throw new IllegalStateException("Encountered Valdef without symbol: "+ tree + " in "+ unit)
// a local variable that is mutable and free somewhere later should be lifted
// as lambda lifting (coming later) will wrap 'rhs' in an Ref object.
if (!sym.owner.isSourceMethod)
withNeedLift(true) { super.transform(tree) }
else
Expand Down
114 changes: 114 additions & 0 deletions test/files/run/t6863.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/** Make sure that when a variable is captured its initialization expression is handled properly */
object Test {
def lazyVal() = {
// internally lazy vals become vars which are initialized with "_", so they need to be tested just like vars do
lazy val x = "42"
assert({ () => x }.apply == "42")
}
def ident() = {
val y = "42"
var x = y
assert({ () => x }.apply == "42")
}
def apply() = {
def y(x : Int) = x.toString
var x = y(42)
assert({ () => x }.apply == "42")
}
def literal() = {
var x = "42"
assert({ () => x }.apply == "42")
}
def `new`() = {
var x = new String("42")
assert({ () => x }.apply == "42")
}
def select() = {
object Foo{val bar = "42"}
var x = Foo.bar
assert({ () => x }.apply == "42")
}
def `throw`() = {
var x = if (true) "42" else throw new Exception("42")
assert({ () => x }.apply == "42")
}
def assign() = {
var y = 1
var x = y = 42
assert({ () => x}.apply == ())
}
def valDef() = {
var x = {val y = 42}
assert({ () => x}.apply == ())
}
def `return`(): String = {
var x = if (true) return "42" else ()
assert({ () => x}.apply == ())
"42"
}
def tryFinally() = {
var x = try { "42" } finally ()
assert({ () => x }.apply == "42")
}
def tryCatch() = {
var x = try { "42" } catch { case _ => "43" }
assert({ () => x }.apply == "42")
}
def `if`() = {
var x = if (true) ()
assert({ () => x }.apply == ())
}
def ifElse() = {
var x = if(true) "42" else "43"
assert({ () => x }.apply == "42")
}
def matchCase() = {
var x = 100 match {
case 100 => "42"
case _ => "43"
}
assert({ () => x }.apply == "42")
}
def block() = {
var x = {
val y = 42
"42"
}
assert({ () => x }.apply == "42")
}
def labelDef() = {
var x = 100 match {
case 100 => try "42" finally ()
}
assert({ () => x }.apply == "42")
}
def nested() = {
var x = {
val y = 42
if(true) try "42" catch {case _ => "43"}
else "44"
}
assert({ () => x }.apply == "42")
}
def main(args: Array[String]) {
lazyVal()
ident()
apply()
literal()
`new`()
select()
`throw`()
assign()
valDef()
`return`()
tryFinally()
tryCatch()
ifElse()
`if`()
matchCase()
block()
labelDef()
nested()
}
}

0 comments on commit 0b52a51

Please sign in to comment.