Skip to content

Commit

Permalink
Fix #16899: Better handle X instanceOf P where X is T1 | T2 (#17382)
Browse files Browse the repository at this point in the history
  • Loading branch information
dwijnand authored May 4, 2023
2 parents be32eb9 + 1f12656 commit d2d3da9
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 6 deletions.
59 changes: 55 additions & 4 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import util.Spans._
import reporting._
import config.Printers.{ transforms => debug }

import patmat.Typ

/** This transform normalizes type tests and type casts,
* also replacing type tests with singleton argument type with reference equality check
* Any remaining type tests
Expand Down Expand Up @@ -51,7 +53,8 @@ object TypeTestsCasts {
* 6. if `P = T1 | T2` or `P = T1 & T2`, checkable(X, T1) && checkable(X, T2).
* 7. if `P` is a refinement type, "it's a refinement type"
* 8. if `P` is a local class which is not statically reachable from the scope where `X` is defined, "it's a local class"
* 9. otherwise, ""
* 9. if `X` is `T1 | T2`, checkable(T1, P) && checkable(T2, P) or (isCheckDefinitelyFalse(T1, P) && checkable(T2, P)) or (checkable(T1, P) && isCheckDefinitelyFalse(T2, P)).
* 10. otherwise, ""
*/
def whyUncheckable(X: Type, P: Type, span: Span)(using Context): String = atPhase(Phases.refchecksPhase.next) {
extension (inline s1: String) inline def &&(inline s2: String): String = if s1 == "" then s2 else s1
Expand Down Expand Up @@ -129,6 +132,41 @@ object TypeTestsCasts {

}

/** Whether the check X.isInstanceOf[P] is definitely false? */
def isCheckDefinitelyFalse(X: Type, P: Type)(using Context): Boolean = trace(s"isCheckDefinitelyFalse(${X.show}, ${P.show})") {
X.widenDealias match
case AndType(x1, x2) =>
isCheckDefinitelyFalse(x1, P) || isCheckDefinitelyFalse(x2, P)

case x =>
P.widenDealias match
case AndType(p1, p2) =>
isCheckDefinitelyFalse(x, p1) || isCheckDefinitelyFalse(x, p2)

case p =>
val pSpace = Typ(p)
val xSpace = Typ(x)
if pSpace.canDecompose then
val ps = pSpace.decompose.map(_.tp)
ps.forall(p => isCheckDefinitelyFalse(x, p))
else if xSpace.canDecompose then
val xs = xSpace.decompose.map(_.tp)
xs.forall(x => isCheckDefinitelyFalse(x, p))
else
if x.typeSymbol.isClass && p.typeSymbol.isClass then
val xClass = effectiveClass(x)
val pClass = effectiveClass(p)

val bothAreClasses = !xClass.is(Trait) && !pClass.is(Trait)
val notXsubP = !xClass.derivesFrom(pClass)
val notPsubX = !pClass.derivesFrom(xClass)
bothAreClasses && notXsubP && notPsubX
|| xClass.is(Final) && notXsubP
|| pClass.is(Final) && notPsubX
else
false
}

def recur(X: Type, P: Type): String = (X <:< P) ||| (P.dealias match {
case _: SingletonType => ""
case _: TypeProxy
Expand All @@ -146,7 +184,20 @@ object TypeTestsCasts {
// - T1 <:< T2 | T3
// - T1 & T2 <:< T3
// See TypeComparer#either
recur(tp1, P) && recur(tp2, P)
val res1 = recur(tp1, P)
val res2 = recur(tp2, P)

if res1.isEmpty && res2.isEmpty then
res1
else if res2.isEmpty then
if isCheckDefinitelyFalse(tp1, P) then res2
else res1
else if res1.isEmpty then
if isCheckDefinitelyFalse(tp2, P) then res1
else res2
else
res1

case _ =>
// always false test warnings are emitted elsewhere
X.classSymbol.exists && P.classSymbol.exists &&
Expand Down Expand Up @@ -302,8 +353,8 @@ object TypeTestsCasts {

/** Transform isInstanceOf
*
* expr.isInstanceOf[A | B] ~~> expr.isInstanceOf[A] | expr.isInstanceOf[B]
* expr.isInstanceOf[A & B] ~~> expr.isInstanceOf[A] & expr.isInstanceOf[B]
* expr.isInstanceOf[A | B] ~~> expr.isInstanceOf[A] | expr.isInstanceOf[B]
* expr.isInstanceOf[A & B] ~~> expr.isInstanceOf[A] & expr.isInstanceOf[B]
* expr.isInstanceOf[Tuple] ~~> scala.runtime.Tuples.isInstanceOfTuple(expr)
* expr.isInstanceOf[EmptyTuple] ~~> scala.runtime.Tuples.isInstanceOfEmptyTuple(expr)
* expr.isInstanceOf[NonEmptyTuple] ~~> scala.runtime.Tuples.isInstanceOfNonEmptyTuple(expr)
Expand Down
25 changes: 23 additions & 2 deletions tests/neg-custom-args/isInstanceOf/i5826.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class Foo {
def test[A]: List[Int] | A => Int = {
case ls: List[Int] => ls.head // error
def test[A]: (List[Int] | A) => Int = {
case ls: List[Int] => ls.head // error, A = List[String]
case _ => 0
}

Expand All @@ -17,4 +17,25 @@ class Foo {
case ls: A[X] => 4 // error
case _ => 0
}

def test4[A](x: List[Int] | (A => Int)) = x match {
case ls: List[Int] => ls.head // error, List extends Int => T
case _ => 0
}

final class C[T] extends A[T]

def test5[T](x: A[T] | B[T] | Option[T]): Boolean = x.isInstanceOf[C[String]] // error

def test6[T](x: A[T] | B[T] | Option[T]): Boolean = x.isInstanceOf[C[T]]

def test7[A](x: Option[Int] | (A => Int)) = x match {
case ls: Option[Int] => ls.head // OK, Option decomposes to Some and None
case _ => 0
}

def test8(x: List[Int] | A[String]) = x match {
case ls: List[Int] => ls.head // OK, List decomposes to :: and Nil
case _ => 0
}
}
5 changes: 5 additions & 0 deletions tests/pos-special/isInstanceOf/i16899.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
sealed trait Unset

def foo(v: Unset|Option[Int]): Unit = v match
case v: Unset => ()
case v: Option[Int] => ()

0 comments on commit d2d3da9

Please sign in to comment.