Skip to content

Commit db552da

Browse files
oderskyBlaisorblade
authored andcommitted
Fix scala#4031: Check arguments of dependent methods for realizability
1 parent 63535f0 commit db552da

File tree

10 files changed

+121
-15
lines changed

10 files changed

+121
-15
lines changed

compiler/src/dotty/tools/dotc/core/Types.scala

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import Periods._
1919
import util.Positions.{Position, NoPosition}
2020
import util.Stats._
2121
import util.{DotClass, SimpleIdentitySet}
22+
import CheckRealizable._
2223
import reporting.diagnostic.Message
2324
import ast.tpd._
2425
import ast.TreeTypeMap
@@ -158,6 +159,12 @@ object Types {
158159
case _ => false
159160
}
160161

162+
/** Does this type denote a realizable stable reference? Much more expensive to checl
163+
* than isStable, that's why some of the checks are done later in PostTyper.
164+
*/
165+
final def isRealizable(implicit ctx: Context): Boolean =
166+
isStable && realizability(this) == Realizable
167+
161168
/** Is this type a (possibly refined or applied or aliased) type reference
162169
* to the given type symbol?
163170
* @sym The symbol to compare to. It must be a class symbol or abstract type.
@@ -4190,6 +4197,8 @@ object Types {
41904197
else if (variance < 0) lo
41914198
else Range(lower(lo), upper(hi))
41924199

4200+
protected def emptyRange = range(defn.NothingType, defn.AnyType)
4201+
41934202
protected def isRange(tp: Type) = tp.isInstanceOf[Range]
41944203

41954204
protected def lower(tp: Type) = tp match {
@@ -4258,7 +4267,7 @@ object Types {
42584267
forwarded.orElse(
42594268
range(super.derivedSelect(tp, preLo), super.derivedSelect(tp, preHi)))
42604269
case _ =>
4261-
super.derivedSelect(tp, pre)
4270+
if (pre == defn.AnyType) pre else super.derivedSelect(tp, pre)
42624271
}
42634272

42644273
override protected def derivedRefinedType(tp: RefinedType, parent: Type, info: Type) =
@@ -4306,7 +4315,7 @@ object Types {
43064315
else tp.derivedTypeBounds(lo, hi)
43074316

43084317
override protected def derivedSuperType(tp: SuperType, thistp: Type, supertp: Type) =
4309-
if (isRange(thistp) || isRange(supertp)) range(defn.NothingType, defn.AnyType)
4318+
if (isRange(thistp) || isRange(supertp)) emptyRange
43104319
else tp.derivedSuperType(thistp, supertp)
43114320

43124321
override protected def derivedAppliedType(tp: AppliedType, tycon: Type, args: List[Type]): Type =

compiler/src/dotty/tools/dotc/typer/Applications.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,8 +434,9 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
434434
def addTyped(arg: Arg, formal: Type): Type => Type = {
435435
addArg(typedArg(arg, formal), formal)
436436
if (methodType.isParamDependent)
437-
safeSubstParam(_, methodType.paramRefs(n), typeOfArg(arg))
438-
else identity
437+
safeSubstParam(_, methodType.paramRefs(n), typeOfArg(arg), initVariance = -1)
438+
else
439+
identity
439440
}
440441

441442
def missingArg(n: Int): Unit = {

compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import collection.mutable
1414
import reporting.diagnostic.Message
1515
import reporting.diagnostic.messages._
1616
import Checking.checkNoPrivateLeaks
17+
import CheckRealizable._
1718

1819
trait TypeAssigner {
1920
import tpd._
@@ -106,7 +107,7 @@ trait TypeAssigner {
106107
case info: ClassInfo =>
107108
range(defn.NothingType, apply(classBound(info)))
108109
case _ =>
109-
range(defn.NothingType, defn.AnyType) // should happen only in error cases
110+
emptyRange // should happen only in error cases
110111
}
111112
case tp: ThisType if toAvoid(tp.cls) =>
112113
range(defn.NothingType, apply(classBound(tp.cls.classInfo)))
@@ -340,13 +341,31 @@ trait TypeAssigner {
340341
}
341342
}
342343

343-
/** Substitute argument type `argType` for parameter `pref` in type `tp`,
344-
* skolemizing the argument type if it is not stable and `pref` occurs in `tp`.
344+
/** Substitute argument type `argType` for parameter `pref` in type `tp`, but
345+
* take special measures if the argument is not realizable:
346+
* 1. If the widened argument type is known to have good bounds,
347+
* substitute the skolemized argument type.
348+
* 2. If the widened argument type is not known to have good bounds, eliminate all references
349+
* to the parameter in `tp`.
350+
* (2) is necessary since even with a skolemized type we might break subtyping if
351+
* bounds are bad. This could lead to errors not being detected. A test case is the second
352+
* failure in neg/i4031.scala
345353
*/
346-
def safeSubstParam(tp: Type, pref: ParamRef, argType: Type)(implicit ctx: Context) = {
354+
def safeSubstParam(tp: Type, pref: ParamRef, argType: Type, initVariance: Int = 1)(implicit ctx: Context) = {
347355
val tp1 = tp.substParam(pref, argType)
348-
if ((tp1 eq tp) || argType.isStable) tp1
349-
else tp.substParam(pref, SkolemType(argType.widen))
356+
if ((tp1 eq tp) || argType.isRealizable) tp1
357+
else {
358+
val widenedArgType = argType.widen
359+
if (realizability(widenedArgType) == Realizable)
360+
tp.substParam(pref, SkolemType(widenedArgType))
361+
else {
362+
val avoidParam = new ApproximatingTypeMap {
363+
variance = initVariance
364+
def apply(t: Type) = if (t `eq` pref) emptyRange else mapOver(t)
365+
}
366+
avoidParam(tp)
367+
}
368+
}
350369
}
351370

352371
/** Substitute types of all arguments `args` for corresponding `params` in `tp`.

tests/neg/i4031-posttyper.scala

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
object App {
2+
trait A { type L >: Any}
3+
def upcast(a: A, x: Any): a.L = x
4+
lazy val p: A { type L <: Nothing } = p
5+
val q = new A { type L = Any }
6+
def coerce2(x: Any): Int = upcast(p, x): p.L // error: not a legal path
7+
}

tests/neg/i4031.scala

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
object App {
2+
trait A { type L >: Any}
3+
def upcast(a: A, x: Any): a.L = x
4+
lazy val p: A { type L <: Nothing } = p
5+
val q = new A { type L = Any }
6+
def coerce(x: Any): Int = upcast(p, x) // error: not a legal path
7+
8+
def compare(x: A, y: x.L) = assert(x == y)
9+
def compare2(x: A)(y: x.type) = assert(x == y)
10+
11+
12+
def main(args: Array[String]): Unit = {
13+
println(coerce("Uh oh!"))
14+
compare(p, p) // error: not a legal path
15+
compare2(p)(p) // error: not a legal path
16+
}
17+
}

tests/neg/i50-volatile.scala

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,21 @@ class Test {
1212

1313
class Client extends o.Inner // old-error // old-error
1414

15-
def xToString(x: o.X): String = x // old-error
15+
def xToString(x: o.X): String = x // error
1616

1717
def intToString(i: Int): String = xToString(i)
1818
}
19-
object Test2 {
2019

21-
import Test.o._ // error
20+
object Test2 {
21+
trait A {
22+
type X = String
23+
}
24+
trait B {
25+
type X = Int
26+
}
27+
lazy val o: A & B = ???
2228

23-
def xToString(x: X): String = x
29+
def xToString(x: o.X): String = x // error
2430

31+
def intToString(i: Int): String = xToString(i)
2532
}

tests/neg/realizability.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
class C { type T }
2+
3+
class Test {
4+
5+
type D <: C
6+
7+
lazy val a: C = ???
8+
final lazy val b: C = ???
9+
val c: D = ???
10+
final lazy val d: D = ???
11+
12+
val x1: a.T = ??? // error: not a legal path, since a is lazy & non-final
13+
val x2: b.T = ??? // OK, b is lazy but concrete
14+
val x3: c.T = ??? // OK, c is abstract but strict
15+
val x4: d.T = ??? // error: not a legal path since d is abstract and lazy
16+
17+
val y1: Singleton = a
18+
val y2: Singleton = a
19+
val y3: Singleton = a
20+
val y4: Singleton = a
21+
22+
}

tests/neg/z1720.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package test
2+
3+
class Thing {
4+
def info: Info[this.type] = InfoRepository.getInfo(this)
5+
def info2: Info[this.type] = {
6+
def self: this.type = this
7+
InfoRepository.getInfo(self) // error: not a legal path
8+
}
9+
}
10+
11+
trait Info[T]
12+
case class InfoImpl[T](thing: T) extends Info[T]
13+
14+
object InfoRepository {
15+
def getInfo(t: Thing): Info[t.type] = InfoImpl(t)
16+
}

tests/pos/i4031.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
object App {
2+
trait A { type L >: Any}
3+
def upcast(a: A, x: Any): a.L = x
4+
lazy val p: A { type L <: Nothing } = p
5+
val q = new A { type L = Any }
6+
def coerce1(x: Any): Any = upcast(q, x) // ok
7+
def coerce3(x: Any): Any = upcast(p, x) // ok, since dependent result type is not needed
8+
}

tests/pos/z1720.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package test
33
class Thing {
44
def info: Info[this.type] = InfoRepository.getInfo(this)
55
def info2: Info[this.type] = {
6-
def self: this.type = this
6+
val self: this.type = this
77
InfoRepository.getInfo(self)
88
}
99
}

0 commit comments

Comments
 (0)