Skip to content

Commit

Permalink
Fix scala#4031: Check arguments of dependent methods for realizability
Browse files Browse the repository at this point in the history
  • Loading branch information
odersky authored and Blaisorblade committed Dec 2, 2018
1 parent 20a576f commit dbbc0b5
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 15 deletions.
13 changes: 11 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Expand Up @@ -18,6 +18,7 @@ import Denotations._
import Periods._
import util.Stats._
import util.SimpleIdentitySet
import CheckRealizable._
import reporting.diagnostic.Message
import ast.tpd._
import ast.TreeTypeMap
Expand Down Expand Up @@ -157,6 +158,12 @@ object Types {
case _ => false
}

/** Does this type denote a realizable stable reference? Much more expensive to check
* than isStable, that's why some of the checks are done later in PostTyper.
*/
final def isRealizable(implicit ctx: Context): Boolean =
isStable && realizability(this) == Realizable

/** Is this type a (possibly refined or applied or aliased) type reference
* to the given type symbol?
* @sym The symbol to compare to. It must be a class symbol or abstract type.
Expand Down Expand Up @@ -4506,6 +4513,8 @@ object Types {
else if (lo `eq` hi) lo
else Range(lower(lo), upper(hi))

protected def emptyRange: Type = range(defn.NothingType, defn.AnyType)

protected def isRange(tp: Type): Boolean = tp.isInstanceOf[Range]

protected def lower(tp: Type): Type = tp match {
Expand Down Expand Up @@ -4579,7 +4588,7 @@ object Types {
forwarded.orElse(
range(super.derivedSelect(tp, preLo).loBound, super.derivedSelect(tp, preHi).hiBound))
case _ =>
super.derivedSelect(tp, pre) match {
if (pre == defn.AnyType) pre else super.derivedSelect(tp, pre) match {
case TypeBounds(lo, hi) => range(lo, hi)
case tp => tp
}
Expand Down Expand Up @@ -4630,7 +4639,7 @@ object Types {
else tp.derivedTypeBounds(lo, hi)

override protected def derivedSuperType(tp: SuperType, thistp: Type, supertp: Type): Type =
if (isRange(thistp) || isRange(supertp)) range(defn.NothingType, defn.AnyType)
if (isRange(thistp) || isRange(supertp)) emptyRange
else tp.derivedSuperType(thistp, supertp)

override protected def derivedAppliedType(tp: AppliedType, tycon: Type, args: List[Type]): Type =
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Expand Up @@ -478,8 +478,9 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
addArg(typedArg(arg, formal), formal)
if (methodType.isParamDependent && typeOfArg(arg).exists)
// `typeOfArg(arg)` could be missing because the evaluation of `arg` produced type errors
safeSubstParam(_, methodType.paramRefs(n), typeOfArg(arg))
else identity
safeSubstParam(_, methodType.paramRefs(n), typeOfArg(arg), initVariance = -1)
else
identity
}

def missingArg(n: Int): Unit = {
Expand Down
31 changes: 25 additions & 6 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Expand Up @@ -13,6 +13,7 @@ import NameOps._
import collection.mutable
import reporting.diagnostic.messages._
import Checking.checkNoPrivateLeaks
import CheckRealizable._

trait TypeAssigner {
import tpd._
Expand Down Expand Up @@ -105,7 +106,7 @@ trait TypeAssigner {
case info: ClassInfo =>
range(defn.NothingType, apply(classBound(info)))
case _ =>
range(defn.NothingType, defn.AnyType) // should happen only in error cases
emptyRange // should happen only in error cases
}
case tp: ThisType if toAvoid(tp.cls) =>
range(defn.NothingType, apply(classBound(tp.cls.classInfo)))
Expand Down Expand Up @@ -340,13 +341,31 @@ trait TypeAssigner {
}
}

/** Substitute argument type `argType` for parameter `pref` in type `tp`,
* skolemizing the argument type if it is not stable and `pref` occurs in `tp`.
/** Substitute argument type `argType` for parameter `pref` in type `tp`, but
* take special measures if the argument is not realizable:
* 1. If the widened argument type is known to have good bounds,
* substitute the skolemized argument type.
* 2. If the widened argument type is not known to have good bounds, eliminate all references
* to the parameter in `tp`.
* (2) is necessary since even with a skolemized type we might break subtyping if
* bounds are bad. This could lead to errors not being detected. A test case is the second
* failure in neg/i4031.scala
*/
def safeSubstParam(tp: Type, pref: ParamRef, argType: Type)(implicit ctx: Context): Type = {
def safeSubstParam(tp: Type, pref: ParamRef, argType: Type, initVariance: Int = 1)(implicit ctx: Context): Type = {
val tp1 = tp.substParam(pref, argType)
if ((tp1 eq tp) || argType.isStable) tp1
else tp.substParam(pref, SkolemType(argType.widen))
if ((tp1 eq tp) || argType.isRealizable) tp1
else {
val widenedArgType = argType.widen
if (realizability(widenedArgType) == Realizable)
tp.substParam(pref, SkolemType(widenedArgType))
else {
val avoidParam = new ApproximatingTypeMap {
variance = initVariance
def apply(t: Type) = if (t `eq` pref) emptyRange else mapOver(t)
}
avoidParam(tp)
}
}
}

/** Substitute types of all arguments `args` for corresponding `params` in `tp`.
Expand Down
7 changes: 7 additions & 0 deletions tests/neg/i4031-posttyper.scala
@@ -0,0 +1,7 @@
object App {
trait A { type L >: Any}
def upcast(a: A, x: Any): a.L = x
lazy val p: A { type L <: Nothing } = p
val q = new A { type L = Any }
def coerce2(x: Any): Int = upcast(p, x): p.L // error: not a legal path
}
17 changes: 17 additions & 0 deletions tests/neg/i4031.scala
@@ -0,0 +1,17 @@
object App {
trait A { type L >: Any}
def upcast(a: A, x: Any): a.L = x
lazy val p: A { type L <: Nothing } = p
val q = new A { type L = Any }
def coerce(x: Any): Int = upcast(p, x) // error: not a legal path

def compare(x: A, y: x.L) = assert(x == y)
def compare2(x: A)(y: x.type) = assert(x == y)


def main(args: Array[String]): Unit = {
println(coerce("Uh oh!"))
compare(p, p) // error: not a legal path
compare2(p)(p) // error: not a legal path
}
}
15 changes: 11 additions & 4 deletions tests/neg/i50-volatile.scala
Expand Up @@ -12,14 +12,21 @@ class Test {

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

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

def intToString(i: Int): String = xToString(i)
}
object Test2 {

import Test.o._ // error
object Test2 {
trait A {
type X = String
}
trait B {
type X = Int
}
lazy val o: A & B = ???

def xToString(x: X): String = x
def xToString(x: o.X): String = x // error

def intToString(i: Int): String = xToString(i)
}
22 changes: 22 additions & 0 deletions tests/neg/realizability.scala
@@ -0,0 +1,22 @@
class C { type T }

class Test {

type D <: C

lazy val a: C = ???
final lazy val b: C = ???
val c: D = ???
final lazy val d: D = ???

val x1: a.T = ??? // error: not a legal path, since a is lazy & non-final
val x2: b.T = ??? // OK, b is lazy but concrete
val x3: c.T = ??? // OK, c is abstract but strict
val x4: d.T = ??? // error: not a legal path since d is abstract and lazy

val y1: Singleton = a
val y2: Singleton = a
val y3: Singleton = a
val y4: Singleton = a

}
16 changes: 16 additions & 0 deletions tests/neg/z1720.scala
@@ -0,0 +1,16 @@
package test

class Thing {
def info: Info[this.type] = InfoRepository.getInfo(this)
def info2: Info[this.type] = {
def self: this.type = this
InfoRepository.getInfo(self) // error: not a legal path
}
}

trait Info[T]
case class InfoImpl[T](thing: T) extends Info[T]

object InfoRepository {
def getInfo(t: Thing): Info[t.type] = InfoImpl(t)
}
8 changes: 8 additions & 0 deletions tests/pos/i4031.scala
@@ -0,0 +1,8 @@
object App {
trait A { type L >: Any}
def upcast(a: A, x: Any): a.L = x
lazy val p: A { type L <: Nothing } = p
val q = new A { type L = Any }
def coerce1(x: Any): Any = upcast(q, x) // ok
def coerce3(x: Any): Any = upcast(p, x) // ok, since dependent result type is not needed
}
2 changes: 1 addition & 1 deletion tests/pos/z1720.scala
Expand Up @@ -3,7 +3,7 @@ package test
class Thing {
def info: Info[this.type] = InfoRepository.getInfo(this)
def info2: Info[this.type] = {
def self: this.type = this
val self: this.type = this
InfoRepository.getInfo(self)
}
}
Expand Down

0 comments on commit dbbc0b5

Please sign in to comment.