Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SI-6443 Widen dependent param types in uncurry #1857

Merged
merged 2 commits into from Jan 28, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
119 changes: 112 additions & 7 deletions src/compiler/scala/tools/nsc/transform/UnCurry.scala
Expand Up @@ -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)
Expand All @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions 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
21 changes: 21 additions & 0 deletions 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
}
13 changes: 13 additions & 0 deletions 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)
}
3 changes: 3 additions & 0 deletions test/files/run/t6443-by-name.check
@@ -0,0 +1,3 @@
1
foo
foo
18 changes: 18 additions & 0 deletions test/files/run/t6443-by-name.scala
@@ -0,0 +1,18 @@
object Test {

def main(args: Array[String]) {
def foo = {println("foo"); 0}
lazyDep(X)(foo)
}

trait T {
type U
}
object X extends T { type U = Int }

def lazyDep(t: T)(u: => t.U) {
println("1")
u
u
}
}
1 change: 1 addition & 0 deletions test/files/run/t6443-varargs.check
@@ -0,0 +1 @@
foo
16 changes: 16 additions & 0 deletions test/files/run/t6443-varargs.scala
@@ -0,0 +1,16 @@
object Test {

def main(args: Array[String]) {
def foo = {println("foo"); 0}
lazyDep(X)(foo)
}

trait T {
type U
}
object X extends T { type U = Int }

def lazyDep(t: T)(us: t.U*) {
List(us: _*)
}
}
15 changes: 15 additions & 0 deletions 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
}
16 changes: 16 additions & 0 deletions 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
}