Permalink
Browse files

SI-6863 root cause fixed using factory of scala.runtime.*Ref

This commit does away with an error-prone division of labor between
UnCurry and LambdaLift, a division of labor by which UnCurry had to
anticipate under which circumstances LambdaLift creates a
scala.runtime.*Ref whose initial value is given by a an expression
including a Try in non-statement position. That sounds complicated,
and it is.

The solution so far (fixing SI-6863) is replaced by a simpler approach,
at the cost of forward binary comptability with pre-2.11 releases,
this time fixing the root cause of SI-6863.

From now on, a s.r.*Ref is instantiated via invocation of
a static factory method in the s.r.*Ref class in question.
Unlike the code that was emitted so far (which involved
NEW refclass, DUP, expr, INVOKESPECIAL refclass.<init>)
the "expr" doesn't appear on the operand stack on top
of the *Ref value being initialized. In other words,
the *Ref initialization is in statement position provided "expr" is.
  • Loading branch information...
1 parent 0c5e2e8 commit 265fc6b230b8b48e50004a3cc568ca5711747616 @magarciaEPFL magarciaEPFL committed Apr 27, 2013
@@ -32,6 +32,21 @@ abstract class LambdaLift extends InfoTransform {
}
}
+ /** scala.runtime.*Ref classes */
+ private lazy val allRefClasses: Set[Symbol] = {
+ refClass.values.toSet ++ volatileRefClass.values.toSet ++ Set(VolatileObjectRefClass, ObjectRefClass)
+ }
+
+ /** Each scala.runtime.*Ref class has a static method `create(value)` that simply instantiates the Ref to carry that value. */
+ private lazy val refCreateMethod: Map[Symbol, Symbol] = {
+ mapFrom(allRefClasses.toList)(x => getMemberMethod(x.companionModule, nme.create))
+ }
+
+ /** Quite frequently a *Ref is initialized with its zero (e.g., null, 0.toByte, etc.) Method `zero()` of *Ref class encapsulates that pattern. */
+ private lazy val refZeroMethod: Map[Symbol, Symbol] = {
+ mapFrom(allRefClasses.toList)(x => getMemberMethod(x.companionModule, nme.zero))
+ }
+
def transformInfo(sym: Symbol, tp: Type): Type =
if (sym.isCapturedVariable) capturedVariableType(sym, tpe = lifted(tp), erasedTypes = true)
else lifted(tp)
@@ -444,56 +459,21 @@ abstract class LambdaLift extends InfoTransform {
case ValDef(mods, name, tpt, rhs) =>
if (sym.isCapturedVariable) {
val tpt1 = TypeTree(sym.tpe) setPos tpt.pos
- /* Creating a constructor argument if one isn't present. */
- val constructorArg = rhs match {
+
+ val refTypeSym = sym.tpe.typeSymbol
+
+ val factoryCall = typer.typedPos(rhs.pos) {
+ rhs match {
case EmptyTree =>
- sym.tpe.typeSymbol.primaryConstructor.info.paramTypes match {
- case List(tp) => gen.mkZero(tp)
- case _ =>
- debugwarn("Couldn't determine how to properly construct " + sym)
- rhs
- }
- case arg => arg
+ val zeroMSym = refZeroMethod(refTypeSym)
+ gen.mkMethodCall(zeroMSym, Nil)
+ case arg =>
+ val createMSym = refCreateMethod(refTypeSym)
+ gen.mkMethodCall(createMSym, arg :: Nil)
+ }
}
- /* 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 = 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 _ =>
- 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, refConstr(constructorArg))
+ treeCopy.ValDef(tree, mods, name, tpt1, factoryCall)
} else tree
case Return(Block(stats, value)) =>
Block(stats, treeCopy.Return(tree, value)) setType tree.tpe setPos tree.pos
@@ -17,4 +17,7 @@
public boolean elem;
public BooleanRef(boolean elem) { this.elem = elem; }
public String toString() { return String.valueOf(elem); }
+
+ public static BooleanRef create(boolean e) { return new BooleanRef(e); }
+ public static BooleanRef zero() { return new BooleanRef(false); }
}
@@ -17,4 +17,7 @@
public byte elem;
public ByteRef(byte elem) { this.elem = elem; }
public String toString() { return java.lang.Byte.toString(elem); }
+
+ public static ByteRef create(byte e) { return new ByteRef(e); }
+ public static ByteRef zero() { return new ByteRef((byte)0); }
}
@@ -17,4 +17,7 @@
public char elem;
public CharRef(char elem) { this.elem = elem; }
public String toString() { return java.lang.Character.toString(elem); }
+
+ public static CharRef create(char e) { return new CharRef(e); }
+ public static CharRef zero() { return new CharRef((char)0); }
}
@@ -17,4 +17,7 @@
public double elem;
public DoubleRef(double elem) { this.elem = elem; }
public String toString() { return java.lang.Double.toString(elem); }
+
+ public static DoubleRef create(double e) { return new DoubleRef(e); }
+ public static DoubleRef zero() { return new DoubleRef(0); }
}
@@ -17,4 +17,7 @@
public float elem;
public FloatRef(float elem) { this.elem = elem; }
public String toString() { return java.lang.Float.toString(elem); }
+
+ public static FloatRef create(float e) { return new FloatRef(e); }
+ public static FloatRef zero() { return new FloatRef(0); }
}
@@ -17,4 +17,7 @@
public int elem;
public IntRef(int elem) { this.elem = elem; }
public String toString() { return java.lang.Integer.toString(elem); }
+
+ public static IntRef create(int e) { return new IntRef(e); }
+ public static IntRef zero() { return new IntRef(0); }
}
@@ -17,4 +17,7 @@
public long elem;
public LongRef(long elem) { this.elem = elem; }
public String toString() { return java.lang.Long.toString(elem); }
+
+ public static LongRef create(long e) { return new LongRef(e); }
+ public static LongRef zero() { return new LongRef(0); }
}
@@ -17,4 +17,7 @@
public T elem;
public ObjectRef(T elem) { this.elem = elem; }
public String toString() { return String.valueOf(elem); }
+
+ public static <U> ObjectRef create(U e) { return new ObjectRef(e); }
+ public static ObjectRef zero() { return new ObjectRef(null); }
}
@@ -17,4 +17,7 @@
public short elem;
public ShortRef(short elem) { this.elem = elem; }
public String toString() { return java.lang.Short.toString(elem); }
+
+ public static ShortRef create(short e) { return new ShortRef(e); }
+ public static ShortRef zero() { return new ShortRef((short)0); }
}
@@ -17,4 +17,7 @@
volatile public boolean elem;
public VolatileBooleanRef(boolean elem) { this.elem = elem; }
public String toString() { return String.valueOf(elem); }
+
+ public static VolatileBooleanRef create(boolean e) { return new VolatileBooleanRef(e); }
+ public static VolatileBooleanRef zero() { return new VolatileBooleanRef(false); }
}
@@ -17,4 +17,7 @@
volatile public byte elem;
public VolatileByteRef(byte elem) { this.elem = elem; }
public String toString() { return java.lang.Byte.toString(elem); }
+
+ public static VolatileByteRef create(byte e) { return new VolatileByteRef(e); }
+ public static VolatileByteRef zero() { return new VolatileByteRef((byte)0); }
}
@@ -17,4 +17,7 @@
volatile public char elem;
public VolatileCharRef(char elem) { this.elem = elem; }
public String toString() { return java.lang.Character.toString(elem); }
+
+ public static VolatileCharRef create(char e) { return new VolatileCharRef(e); }
+ public static VolatileCharRef zero() { return new VolatileCharRef((char)0); }
}
@@ -16,4 +16,7 @@
volatile public double elem;
public VolatileDoubleRef(double elem) { this.elem = elem; }
public String toString() { return java.lang.Double.toString(elem); }
+
+ public static VolatileDoubleRef create(double e) { return new VolatileDoubleRef(e); }
+ public static VolatileDoubleRef zero() { return new VolatileDoubleRef(0); }
}
@@ -17,4 +17,7 @@
volatile public float elem;
public VolatileFloatRef(float elem) { this.elem = elem; }
public String toString() { return java.lang.Float.toString(elem); }
+
+ public static VolatileFloatRef create(float e) { return new VolatileFloatRef(e); }
+ public static VolatileFloatRef zero() { return new VolatileFloatRef(0); }
}
@@ -16,4 +16,7 @@
volatile public int elem;
public VolatileIntRef(int elem) { this.elem = elem; }
public String toString() { return java.lang.Integer.toString(elem); }
+
+ public static VolatileIntRef create(int e) { return new VolatileIntRef(e); }
+ public static VolatileIntRef zero() { return new VolatileIntRef(0); }
}
@@ -17,4 +17,7 @@
volatile public long elem;
public VolatileLongRef(long elem) { this.elem = elem; }
public String toString() { return java.lang.Long.toString(elem); }
+
+ public static VolatileLongRef create(long e) { return new VolatileLongRef(e); }
+ public static VolatileLongRef zero() { return new VolatileLongRef(0); }
}
@@ -17,4 +17,7 @@
volatile public T elem;
public VolatileObjectRef(T elem) { this.elem = elem; }
public String toString() { return String.valueOf(elem); }
+
+ public static <U> VolatileObjectRef create(U e) { return new VolatileObjectRef(e); }
+ public static VolatileObjectRef zero() { return new VolatileObjectRef(null); }
}
@@ -17,4 +17,7 @@
volatile public short elem;
public VolatileShortRef(short elem) { this.elem = elem; }
public String toString() { return java.lang.Short.toString(elem); }
+
+ public static VolatileShortRef create(short e) { return new VolatileShortRef(e); }
+ public static VolatileShortRef zero() { return new VolatileShortRef((short)0); }
}
@@ -589,6 +589,7 @@ trait StdNames {
val clone_ : NameType = "clone"
val conforms: NameType = "conforms"
val copy: NameType = "copy"
+ val create: NameType = "create"
val currentMirror: NameType = "currentMirror"
val delayedInit: NameType = "delayedInit"
val delayedInitArg: NameType = "delayedInit$body"
@@ -697,6 +698,7 @@ trait StdNames {
val view_ : NameType = "view"
val wait_ : NameType = "wait"
val withFilter: NameType = "withFilter"
+ val zero: NameType = "zero"
// unencoded operators
object raw {
@@ -15,11 +15,11 @@ package <empty> {
}
};
def bar(barParam: Int): Object = {
- @volatile var MethodLocalObject$module: runtime.VolatileObjectRef = new runtime.VolatileObjectRef(null);
+ @volatile var MethodLocalObject$module: runtime.VolatileObjectRef = scala.runtime.VolatileObjectRef.zero();
T.this.MethodLocalObject$1(barParam, MethodLocalObject$module)
};
def tryy(tryyParam: Int): Function0 = {
- var tryyLocal: runtime.IntRef = new runtime.IntRef(0);
+ var tryyLocal: runtime.IntRef = scala.runtime.IntRef.create(0);
{
(new anonymous class $anonfun$tryy$1(T.this, tryyParam, tryyLocal): Function0)
}

0 comments on commit 265fc6b

Please sign in to comment.