From 493197fce6c9c493dfa4dcdd8fd5e29bad82d49b Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 3 Dec 2012 13:14:22 +0100 Subject: SI-6443 Widen dependent param types in uncurry Bridge building operates on unusual method signatures: after uncurry, so parameter lists are collapsed; but before erasure, so dependently typed parameters are still around. Original: def foo(a: T)(b: a.type, c: a.U): Unit During computeBridges: (a: T, b: a.type, c: a.U)Unit This signature no longer appears to override the corresponding one in a superclass, because the types of `b` and `c` are dependent on method parameters. The root of the problem is uncurry, which leaves the trees in a poor state. This commit changes uncurry to remedy this. An example illustrates it best: // source def foo(a: A)(b: a.type): b.type = b // post uncurry before this patch. // not well typed code! def foo(a: A, b: a.type): a.type = { // post uncurry after this patch def foo(a: A, b: A): A = { val b$1 = b.asInstanceOf[a.type] b$1 } --- .../scala/tools/nsc/transform/UnCurry.scala | 119 +++++++++++++++++++-- test/files/neg/t6443c.check | 7 ++ test/files/neg/t6443c.scala | 21 ++++ test/files/run/t6135.scala | 13 +++ test/files/run/t6443.scala | 15 +++ test/files/run/t6443b.scala | 16 +++ 6 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 test/files/neg/t6443c.check create mode 100644 test/files/neg/t6443c.scala create mode 100644 test/files/run/t6135.scala create mode 100644 test/files/run/t6443.scala create mode 100644 test/files/run/t6443b.scala diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index 838ea7d5a0..2ea406c214 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -746,15 +746,22 @@ abstract class UnCurry extends InfoTransform } case dd @ DefDef(_, _, _, vparamss0, _, rhs0) => - val vparamss1 = vparamss0 match { - case _ :: Nil => vparamss0 - case _ => vparamss0.flatten :: Nil - } + val (newParamss, newRhs): (List[List[ValDef]], Tree) = + if (dependentParamTypeErasure isDependent dd) + dependentParamTypeErasure erase dd + else { + val vparamss1 = vparamss0 match { + case _ :: Nil => vparamss0 + case _ => vparamss0.flatten :: Nil + } + (vparamss1, rhs0) + } + val flatdd = copyDefDef(dd)( - vparamss = vparamss1, + vparamss = newParamss, rhs = nonLocalReturnKeys get dd.symbol match { - case Some(k) => atPos(rhs0.pos)(nonLocalReturnTry(rhs0, k, dd.symbol)) - case None => rhs0 + case Some(k) => atPos(newRhs.pos)(nonLocalReturnTry(newRhs, k, dd.symbol)) + case None => newRhs } ) addJavaVarargsForwarders(dd, flatdd) @@ -780,6 +787,104 @@ abstract class UnCurry extends InfoTransform } } + /** + * When we concatenate parameter lists, formal parameter types that were dependent + * on prior parameter values will no longer be correctly scoped. + * + * For example: + * + * {{{ + * def foo(a: A)(b: a.B): a.type = {b; b} + * // after uncurry + * def foo(a: A, b: a/* NOT IN SCOPE! */.B): a.B = {b; b} + * }}} + * + * This violates the principle that each compiler phase should produce trees that + * can be retyped (see [[scala.tools.nsc.typechecker.TreeCheckers]]), and causes + * a practical problem in `erasure`: it is not able to correctly determine if + * such a signature overrides a corresponding signature in a parent. (SI-6443). + * + * This transformation erases the dependent method types by: + * - Widening the formal parameter type to existentially abstract + * over the prior parameters (using `packSymbols`) + * - Inserting casts in the method body to cast to the original, + * precise type. + * + * For the example above, this results in: + * + * {{{ + * def foo(a: A, b: a.B forSome { val a: A }): a.B = { val b$1 = b.asInstanceOf[a.B]; b$1; b$1 } + * }}} + */ + private object dependentParamTypeErasure { + sealed abstract class ParamTransform { + def param: ValDef + } + final case class Identity(param: ValDef) extends ParamTransform + final case class Packed(param: ValDef, tempVal: ValDef) extends ParamTransform + + def isDependent(dd: DefDef): Boolean = + beforeUncurry { + val methType = dd.symbol.info + methType.isDependentMethodType && mexists(methType.paramss)(_.info exists (_.isImmediatelyDependent)) + } + + /** + * @return (newVparamss, newRhs) + */ + def erase(dd: DefDef): (List[List[ValDef]], Tree) = { + import dd.{ vparamss, rhs } + val vparamSyms = vparamss flatMap (_ map (_.symbol)) + + val paramTransforms: List[ParamTransform] = + vparamss.flatten.map { p => + val declaredType = p.symbol.info + // existentially abstract over value parameters + val packedType = typer.packSymbols(vparamSyms, declaredType) + if (packedType =:= declaredType) Identity(p) + else { + // Change the type of the param symbol + p.symbol updateInfo packedType + + // Create a new param tree + val newParam: ValDef = copyValDef(p)(tpt = TypeTree(packedType)) + + // Within the method body, we'll cast the parameter to the originally + // declared type and assign this to a synthetic val. Later, we'll patch + // the method body to refer to this, rather than the parameter. + val tempVal: ValDef = { + val tempValName = unit freshTermName (p.name + "$") + val newSym = dd.symbol.newTermSymbol(tempValName, p.pos, SYNTHETIC).setInfo(declaredType) + atPos(p.pos)(ValDef(newSym, gen.mkAttributedCast(Ident(p.symbol), declaredType))) + } + Packed(newParam, tempVal) + } + } + + val allParams = paramTransforms map (_.param) + val (packedParams, tempVals) = paramTransforms.collect { + case Packed(param, tempVal) => (param, tempVal) + }.unzip + + val rhs1 = localTyper.typedPos(rhs.pos) { + // Patch the method body to refer to the temp vals + val rhsSubstituted = rhs.substituteSymbols(packedParams map (_.symbol), tempVals map (_.symbol)) + // The new method body: { val p$1 = p.asInstanceOf[]; ...; } + Block(tempVals, rhsSubstituted) + } + + // update the type of the method after uncurry. + dd.symbol updateInfo { + val GenPolyType(tparams, tp) = dd.symbol.info + logResult("erased dependent param types for ${dd.symbol.info}") { + GenPolyType(tparams, MethodType(allParams map (_.symbol), tp.finalResultType)) + } + } + (allParams :: Nil, rhs1) + } + } + + /* Analyzes repeated params if method is annotated as `varargs`. * If the repeated params exist, it saves them into the `repeatedParams` map, * which is used later. diff --git a/test/files/neg/t6443c.check b/test/files/neg/t6443c.check new file mode 100644 index 0000000000..7cf8d23f4b --- /dev/null +++ b/test/files/neg/t6443c.check @@ -0,0 +1,7 @@ +t6443c.scala:16: error: double definition: +method foo:(d: B.D)(a: Any)(d2: d.type)Unit and +method foo:(d: B.D)(a: Any, d2: d.type)Unit at line 11 +have same type after erasure: (d: B.D, a: Object, d2: B.D)Unit + def foo(d: D)(a: Any)(d2: d.type): Unit = () + ^ +one error found diff --git a/test/files/neg/t6443c.scala b/test/files/neg/t6443c.scala new file mode 100644 index 0000000000..817224e043 --- /dev/null +++ b/test/files/neg/t6443c.scala @@ -0,0 +1,21 @@ +trait A { + type D >: Null <: C + def foo(d: D)(a: Any, d2: d.type): Unit + trait C { + def bar: Unit = foo(null)(null, null) + } +} +object B extends A { + class D extends C + + def foo(d: D)(a: Any, d2: d.type): Unit = () // Bridge method required here! + + // No bridge method should be added, but we'll be happy enough if + // the "same type after erasure" error kicks in before the duplicated + // bridge causes a problem. + def foo(d: D)(a: Any)(d2: d.type): Unit = () +} + +object Test extends App { + new B.D().bar +} diff --git a/test/files/run/t6135.scala b/test/files/run/t6135.scala new file mode 100644 index 0000000000..c0f8f3fd1d --- /dev/null +++ b/test/files/run/t6135.scala @@ -0,0 +1,13 @@ +object Test extends App { + class A { class V } + + abstract class B[S] { + def foo(t: S, a: A)(v: a.V) + } + + val b1 = new B[String] { + def foo(t: String, a: A)(v: a.V) = () // Bridge method required here! + } + + b1.foo("", null)(null) +} diff --git a/test/files/run/t6443.scala b/test/files/run/t6443.scala new file mode 100644 index 0000000000..67fe2cab22 --- /dev/null +++ b/test/files/run/t6443.scala @@ -0,0 +1,15 @@ +class Base +class Derived extends Base + +trait A { + def foo(d: String)(d2: d.type): Base + val s = "" + def bar: Unit = foo(s)(s) +} +object B extends A { + def foo(d: String)(d2: d.type): D forSome { type D <: S; type S <: Derived } = {d2.isEmpty; null} // Bridge method required here! +} + +object Test extends App { + B.bar +} diff --git a/test/files/run/t6443b.scala b/test/files/run/t6443b.scala new file mode 100644 index 0000000000..9320b1dcfe --- /dev/null +++ b/test/files/run/t6443b.scala @@ -0,0 +1,16 @@ +trait A { + type D >: Null <: C + def foo(d: D)(d2: d.type): Unit + trait C { + def bar: Unit = foo(null)(null) + } +} +object B extends A { + class D extends C + + def foo(d: D)(d2: d.type): Unit = () // Bridge method required here! +} + +object Test extends App { + new B.D().bar +} -- cgit v1.2.3