Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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
    }
  • Loading branch information...
commit 493197fce6c9c493dfa4dcdd8fd5e29bad82d49b 1 parent 766bb97
Jason Zaugg retronym authored
119 src/compiler/scala/tools/nsc/transform/UnCurry.scala
View
@@ -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[<dependent type>]; ...; <rhsSubstituted> }
+ 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.
7 test/files/neg/t6443c.check
View
@@ -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
21 test/files/neg/t6443c.scala
View
@@ -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
+}
13 test/files/run/t6135.scala
View
@@ -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)
+}
15 test/files/run/t6443.scala
View
@@ -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
+}
16 test/files/run/t6443b.scala
View
@@ -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
+}
Please sign in to comment.
Something went wrong with that request. Please try again.