From e6c0b58d9a346c521945743eb2ccad243082e024 Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Mon, 1 May 2023 13:55:36 +0200 Subject: [PATCH 1/8] Fix #16899: Better handle X instanceOf P where X is T1 | T2 --- .../tools/dotc/transform/TypeTestsCasts.scala | 46 +++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 3763af243881..378ad0679fe9 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -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 @@ -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`, (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 @@ -129,6 +132,34 @@ 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.dealias match + case AndType(x1, x2) => + isCheckDefinitelyFalse(x1, P) || isCheckDefinitelyFalse(x2, P) + + case x => + P.dealias 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.exists(x => isCheckDefinitelyFalse(x, p)) + else + val xClass = effectiveClass(x.widen) + val pClass = effectiveClass(p.widen) + + !xClass.derivesFrom(pClass) + && (xClass.is(Final) || pClass.is(Final) || !xClass.is(Trait) && !pClass.is(Trait)) + } + def recur(X: Type, P: Type): String = (X <:< P) ||| (P.dealias match { case _: SingletonType => "" case _: TypeProxy @@ -146,7 +177,14 @@ 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 isCheckDefinitelyFalse(tp1, P) && res2.isEmpty then res2 + else if res1.isEmpty && isCheckDefinitelyFalse(tp2, P) then res1 + else res1 + case _ => // always false test warnings are emitted elsewhere X.classSymbol.exists && P.classSymbol.exists && @@ -302,8 +340,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) From 6f370fd997df863730f16e35e94272f36b2edca0 Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Mon, 1 May 2023 13:56:22 +0200 Subject: [PATCH 2/8] Add test --- tests/pos-special/isInstanceOf/i16899.scala | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 tests/pos-special/isInstanceOf/i16899.scala diff --git a/tests/pos-special/isInstanceOf/i16899.scala b/tests/pos-special/isInstanceOf/i16899.scala new file mode 100644 index 000000000000..650e1e5c7b23 --- /dev/null +++ b/tests/pos-special/isInstanceOf/i16899.scala @@ -0,0 +1,5 @@ +sealed trait Unset + +def foo(v: Unset|Option[Int]): Unit = v match + case v: Unset => () + case v: Option[Int] => () From 8b3eee87544530464dfe8bfabbfcaeade6287775 Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Mon, 1 May 2023 13:56:38 +0200 Subject: [PATCH 3/8] Fix tests --- .../dotty/tools/dotc/transform/TypeTestsCasts.scala | 13 +++++++++---- tests/neg-custom-args/isInstanceOf/i5826.scala | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 378ad0679fe9..7684d7f2867e 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -151,7 +151,7 @@ object TypeTestsCasts { ps.forall(p => isCheckDefinitelyFalse(x, p)) else if xSpace.canDecompose then val xs = xSpace.decompose.map(_.tp) - xs.exists(x => isCheckDefinitelyFalse(x, p)) + xs.forall(x => isCheckDefinitelyFalse(x, p)) else val xClass = effectiveClass(x.widen) val pClass = effectiveClass(p.widen) @@ -181,9 +181,14 @@ object TypeTestsCasts { val res2 = recur(tp2, P) if res1.isEmpty && res2.isEmpty then res1 - else if isCheckDefinitelyFalse(tp1, P) && res2.isEmpty then res2 - else if res1.isEmpty && isCheckDefinitelyFalse(tp2, P) then res1 - else res1 + else if res2.isEmpty then + if isCheckDefinitelyFalse(tp1, P) then res2 + else i"it cannot be checked against the type $tp1" + else if res1.isEmpty then + if isCheckDefinitelyFalse(tp2, P) then res1 + else i"it cannot be checked against the type $tp2" + else + i"it cannot be checked against the type $tp2" case _ => // always false test warnings are emitted elsewhere diff --git a/tests/neg-custom-args/isInstanceOf/i5826.scala b/tests/neg-custom-args/isInstanceOf/i5826.scala index bff95e740b4f..89af39958541 100644 --- a/tests/neg-custom-args/isInstanceOf/i5826.scala +++ b/tests/neg-custom-args/isInstanceOf/i5826.scala @@ -1,6 +1,6 @@ class Foo { def test[A]: List[Int] | A => Int = { - case ls: List[Int] => ls.head // error + case ls: List[Int] => ls.head // ok, List decomposes to Some and None case _ => 0 } From 128c7a1682738d5167c393ee2c74e1ed7ab8acc3 Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Mon, 1 May 2023 13:57:39 +0200 Subject: [PATCH 4/8] Fix doc --- compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 7684d7f2867e..15503b475d65 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -53,7 +53,7 @@ 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. if `X` is `T1 | T2`, (isCheckDefinitelyFalse(T1, P) && checkable(T2, P)) or (checkable(T1, P) && isCheckDefinitelyFalse(T2, P)). + * 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) { From b8098cee7f435df38e58e22442e9456b4946b2ea Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Mon, 1 May 2023 15:07:30 +0200 Subject: [PATCH 5/8] Correctly handle abstract type in isCheckDefinitelyFalse --- .../dotty/tools/dotc/transform/TypeTestsCasts.scala | 11 +++++++---- tests/neg-custom-args/isInstanceOf/i5826.scala | 9 +++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 15503b475d65..6faf5be98e34 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -153,11 +153,14 @@ object TypeTestsCasts { val xs = xSpace.decompose.map(_.tp) xs.forall(x => isCheckDefinitelyFalse(x, p)) else - val xClass = effectiveClass(x.widen) - val pClass = effectiveClass(p.widen) + if x.typeSymbol.isClass && p.typeSymbol.isClass then + val xClass = effectiveClass(x.widen) + val pClass = effectiveClass(p.widen) - !xClass.derivesFrom(pClass) - && (xClass.is(Final) || pClass.is(Final) || !xClass.is(Trait) && !pClass.is(Trait)) + !xClass.derivesFrom(pClass) + && (xClass.is(Final) || pClass.is(Final) || !xClass.is(Trait) && !pClass.is(Trait)) + else + false } def recur(X: Type, P: Type): String = (X <:< P) ||| (P.dealias match { diff --git a/tests/neg-custom-args/isInstanceOf/i5826.scala b/tests/neg-custom-args/isInstanceOf/i5826.scala index 89af39958541..e14142e974a8 100644 --- a/tests/neg-custom-args/isInstanceOf/i5826.scala +++ b/tests/neg-custom-args/isInstanceOf/i5826.scala @@ -1,6 +1,6 @@ class Foo { - def test[A]: List[Int] | A => Int = { - case ls: List[Int] => ls.head // ok, List decomposes to Some and None + def test[A]: (List[Int] | A) => Int = { + case ls: List[Int] => ls.head // error, A = List[String] case _ => 0 } @@ -17,4 +17,9 @@ 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 // ok, List decomposes to Some and None + case _ => 0 + } } From 5344b06e0ab4f38c12900ad96ef7b082fbb4c1c4 Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Mon, 1 May 2023 15:13:33 +0200 Subject: [PATCH 6/8] Refine error message --- .../src/dotty/tools/dotc/transform/TypeTestsCasts.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 6faf5be98e34..6dc9287262c0 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -186,12 +186,12 @@ object TypeTestsCasts { if res1.isEmpty && res2.isEmpty then res1 else if res2.isEmpty then if isCheckDefinitelyFalse(tp1, P) then res2 - else i"it cannot be checked against the type $tp1" + else res1 else if res1.isEmpty then if isCheckDefinitelyFalse(tp2, P) then res1 - else i"it cannot be checked against the type $tp2" + else res2 else - i"it cannot be checked against the type $tp2" + res1 case _ => // always false test warnings are emitted elsewhere From 879ebdeb07c70484abde7070afbcbcb17afc4828 Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Tue, 2 May 2023 07:25:49 +0200 Subject: [PATCH 7/8] Refine code --- .../src/dotty/tools/dotc/transform/TypeTestsCasts.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index 6dc9287262c0..ef0fefecd49e 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -134,12 +134,12 @@ 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.dealias match + X.widenDealias match case AndType(x1, x2) => isCheckDefinitelyFalse(x1, P) || isCheckDefinitelyFalse(x2, P) case x => - P.dealias match + P.widenDealias match case AndType(p1, p2) => isCheckDefinitelyFalse(x, p1) || isCheckDefinitelyFalse(x, p2) @@ -154,8 +154,8 @@ object TypeTestsCasts { xs.forall(x => isCheckDefinitelyFalse(x, p)) else if x.typeSymbol.isClass && p.typeSymbol.isClass then - val xClass = effectiveClass(x.widen) - val pClass = effectiveClass(p.widen) + val xClass = effectiveClass(x) + val pClass = effectiveClass(p) !xClass.derivesFrom(pClass) && (xClass.is(Final) || pClass.is(Final) || !xClass.is(Trait) && !pClass.is(Trait)) From 1f12656e75819f549b40ee3a8306773b037cb355 Mon Sep 17 00:00:00 2001 From: Fengyun Liu Date: Tue, 2 May 2023 21:08:46 +0200 Subject: [PATCH 8/8] Fix logic in isCheckDefinitelyFalse --- .../tools/dotc/transform/TypeTestsCasts.scala | 11 ++++++++--- tests/neg-custom-args/isInstanceOf/i5826.scala | 18 +++++++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index ef0fefecd49e..979538b56fc5 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala @@ -157,8 +157,12 @@ object TypeTestsCasts { val xClass = effectiveClass(x) val pClass = effectiveClass(p) - !xClass.derivesFrom(pClass) - && (xClass.is(Final) || pClass.is(Final) || !xClass.is(Trait) && !pClass.is(Trait)) + 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 } @@ -183,7 +187,8 @@ object TypeTestsCasts { val res1 = recur(tp1, P) val res2 = recur(tp2, P) - if res1.isEmpty && res2.isEmpty then res1 + if res1.isEmpty && res2.isEmpty then + res1 else if res2.isEmpty then if isCheckDefinitelyFalse(tp1, P) then res2 else res1 diff --git a/tests/neg-custom-args/isInstanceOf/i5826.scala b/tests/neg-custom-args/isInstanceOf/i5826.scala index e14142e974a8..c63bf3ab4aef 100644 --- a/tests/neg-custom-args/isInstanceOf/i5826.scala +++ b/tests/neg-custom-args/isInstanceOf/i5826.scala @@ -19,7 +19,23 @@ class Foo { } def test4[A](x: List[Int] | (A => Int)) = x match { - case ls: List[Int] => ls.head // ok, List decomposes to Some and None + 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 } }