Permalink
Browse files

SI-6863 Fix verify error in captured var inited from expr with try/catch

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 19, 2013
1 parent 2fa859e commit 0b52a5199ba8b949bd80de1eaf589420ef91d6f6
@@ -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
@@ -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
View
@@ -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.