Skip to content

Commit 493197f

Browse files
committed
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 }
1 parent 766bb97 commit 493197f

File tree

6 files changed

+184
-7
lines changed

6 files changed

+184
-7
lines changed

src/compiler/scala/tools/nsc/transform/UnCurry.scala

Lines changed: 112 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -746,15 +746,22 @@ abstract class UnCurry extends InfoTransform
746746
}
747747

748748
case dd @ DefDef(_, _, _, vparamss0, _, rhs0) =>
749-
val vparamss1 = vparamss0 match {
750-
case _ :: Nil => vparamss0
751-
case _ => vparamss0.flatten :: Nil
752-
}
749+
val (newParamss, newRhs): (List[List[ValDef]], Tree) =
750+
if (dependentParamTypeErasure isDependent dd)
751+
dependentParamTypeErasure erase dd
752+
else {
753+
val vparamss1 = vparamss0 match {
754+
case _ :: Nil => vparamss0
755+
case _ => vparamss0.flatten :: Nil
756+
}
757+
(vparamss1, rhs0)
758+
}
759+
753760
val flatdd = copyDefDef(dd)(
754-
vparamss = vparamss1,
761+
vparamss = newParamss,
755762
rhs = nonLocalReturnKeys get dd.symbol match {
756-
case Some(k) => atPos(rhs0.pos)(nonLocalReturnTry(rhs0, k, dd.symbol))
757-
case None => rhs0
763+
case Some(k) => atPos(newRhs.pos)(nonLocalReturnTry(newRhs, k, dd.symbol))
764+
case None => newRhs
758765
}
759766
)
760767
addJavaVarargsForwarders(dd, flatdd)
@@ -780,6 +787,104 @@ abstract class UnCurry extends InfoTransform
780787
}
781788
}
782789

790+
/**
791+
* When we concatenate parameter lists, formal parameter types that were dependent
792+
* on prior parameter values will no longer be correctly scoped.
793+
*
794+
* For example:
795+
*
796+
* {{{
797+
* def foo(a: A)(b: a.B): a.type = {b; b}
798+
* // after uncurry
799+
* def foo(a: A, b: a/* NOT IN SCOPE! */.B): a.B = {b; b}
800+
* }}}
801+
*
802+
* This violates the principle that each compiler phase should produce trees that
803+
* can be retyped (see [[scala.tools.nsc.typechecker.TreeCheckers]]), and causes
804+
* a practical problem in `erasure`: it is not able to correctly determine if
805+
* such a signature overrides a corresponding signature in a parent. (SI-6443).
806+
*
807+
* This transformation erases the dependent method types by:
808+
* - Widening the formal parameter type to existentially abstract
809+
* over the prior parameters (using `packSymbols`)
810+
* - Inserting casts in the method body to cast to the original,
811+
* precise type.
812+
*
813+
* For the example above, this results in:
814+
*
815+
* {{{
816+
* def foo(a: A, b: a.B forSome { val a: A }): a.B = { val b$1 = b.asInstanceOf[a.B]; b$1; b$1 }
817+
* }}}
818+
*/
819+
private object dependentParamTypeErasure {
820+
sealed abstract class ParamTransform {
821+
def param: ValDef
822+
}
823+
final case class Identity(param: ValDef) extends ParamTransform
824+
final case class Packed(param: ValDef, tempVal: ValDef) extends ParamTransform
825+
826+
def isDependent(dd: DefDef): Boolean =
827+
beforeUncurry {
828+
val methType = dd.symbol.info
829+
methType.isDependentMethodType && mexists(methType.paramss)(_.info exists (_.isImmediatelyDependent))
830+
}
831+
832+
/**
833+
* @return (newVparamss, newRhs)
834+
*/
835+
def erase(dd: DefDef): (List[List[ValDef]], Tree) = {
836+
import dd.{ vparamss, rhs }
837+
val vparamSyms = vparamss flatMap (_ map (_.symbol))
838+
839+
val paramTransforms: List[ParamTransform] =
840+
vparamss.flatten.map { p =>
841+
val declaredType = p.symbol.info
842+
// existentially abstract over value parameters
843+
val packedType = typer.packSymbols(vparamSyms, declaredType)
844+
if (packedType =:= declaredType) Identity(p)
845+
else {
846+
// Change the type of the param symbol
847+
p.symbol updateInfo packedType
848+
849+
// Create a new param tree
850+
val newParam: ValDef = copyValDef(p)(tpt = TypeTree(packedType))
851+
852+
// Within the method body, we'll cast the parameter to the originally
853+
// declared type and assign this to a synthetic val. Later, we'll patch
854+
// the method body to refer to this, rather than the parameter.
855+
val tempVal: ValDef = {
856+
val tempValName = unit freshTermName (p.name + "$")
857+
val newSym = dd.symbol.newTermSymbol(tempValName, p.pos, SYNTHETIC).setInfo(declaredType)
858+
atPos(p.pos)(ValDef(newSym, gen.mkAttributedCast(Ident(p.symbol), declaredType)))
859+
}
860+
Packed(newParam, tempVal)
861+
}
862+
}
863+
864+
val allParams = paramTransforms map (_.param)
865+
val (packedParams, tempVals) = paramTransforms.collect {
866+
case Packed(param, tempVal) => (param, tempVal)
867+
}.unzip
868+
869+
val rhs1 = localTyper.typedPos(rhs.pos) {
870+
// Patch the method body to refer to the temp vals
871+
val rhsSubstituted = rhs.substituteSymbols(packedParams map (_.symbol), tempVals map (_.symbol))
872+
// The new method body: { val p$1 = p.asInstanceOf[<dependent type>]; ...; <rhsSubstituted> }
873+
Block(tempVals, rhsSubstituted)
874+
}
875+
876+
// update the type of the method after uncurry.
877+
dd.symbol updateInfo {
878+
val GenPolyType(tparams, tp) = dd.symbol.info
879+
logResult("erased dependent param types for ${dd.symbol.info}") {
880+
GenPolyType(tparams, MethodType(allParams map (_.symbol), tp.finalResultType))
881+
}
882+
}
883+
(allParams :: Nil, rhs1)
884+
}
885+
}
886+
887+
783888
/* Analyzes repeated params if method is annotated as `varargs`.
784889
* If the repeated params exist, it saves them into the `repeatedParams` map,
785890
* which is used later.

test/files/neg/t6443c.check

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
t6443c.scala:16: error: double definition:
2+
method foo:(d: B.D)(a: Any)(d2: d.type)Unit and
3+
method foo:(d: B.D)(a: Any, d2: d.type)Unit at line 11
4+
have same type after erasure: (d: B.D, a: Object, d2: B.D)Unit
5+
def foo(d: D)(a: Any)(d2: d.type): Unit = ()
6+
^
7+
one error found

test/files/neg/t6443c.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
trait A {
2+
type D >: Null <: C
3+
def foo(d: D)(a: Any, d2: d.type): Unit
4+
trait C {
5+
def bar: Unit = foo(null)(null, null)
6+
}
7+
}
8+
object B extends A {
9+
class D extends C
10+
11+
def foo(d: D)(a: Any, d2: d.type): Unit = () // Bridge method required here!
12+
13+
// No bridge method should be added, but we'll be happy enough if
14+
// the "same type after erasure" error kicks in before the duplicated
15+
// bridge causes a problem.
16+
def foo(d: D)(a: Any)(d2: d.type): Unit = ()
17+
}
18+
19+
object Test extends App {
20+
new B.D().bar
21+
}

test/files/run/t6135.scala

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
object Test extends App {
2+
class A { class V }
3+
4+
abstract class B[S] {
5+
def foo(t: S, a: A)(v: a.V)
6+
}
7+
8+
val b1 = new B[String] {
9+
def foo(t: String, a: A)(v: a.V) = () // Bridge method required here!
10+
}
11+
12+
b1.foo("", null)(null)
13+
}

test/files/run/t6443.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class Base
2+
class Derived extends Base
3+
4+
trait A {
5+
def foo(d: String)(d2: d.type): Base
6+
val s = ""
7+
def bar: Unit = foo(s)(s)
8+
}
9+
object B extends A {
10+
def foo(d: String)(d2: d.type): D forSome { type D <: S; type S <: Derived } = {d2.isEmpty; null} // Bridge method required here!
11+
}
12+
13+
object Test extends App {
14+
B.bar
15+
}

test/files/run/t6443b.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
trait A {
2+
type D >: Null <: C
3+
def foo(d: D)(d2: d.type): Unit
4+
trait C {
5+
def bar: Unit = foo(null)(null)
6+
}
7+
}
8+
object B extends A {
9+
class D extends C
10+
11+
def foo(d: D)(d2: d.type): Unit = () // Bridge method required here!
12+
}
13+
14+
object Test extends App {
15+
new B.D().bar
16+
}

0 commit comments

Comments
 (0)