Emit deferred reachability warnings

There are three kinds of unreachable cases:

1. the cases that are provably disjoint from the scrutinee type
2. the cases that are not subtypes of the scrutinee type
3. the cases that are covered by a previous cases, aka an overlap

The first one should be (I didn't review or stress test it) handled when
type-checking the pattern.

The third one is the safest, in terms of not emitting false positives to
the user, because it's within the conservative approximation of the
scrutinee space and the previous cases spaces.

The second one is where it gets tricky, because we don't know they're
part of the scrutinee type but we also don't know they're _not_ part of
the scrutinee type.  Erasure is the simplest example: if the scrutinee
type is `type ThisTree <: Tree` then if you write cases for subtypes of
Tree that aren't subtypes of ThisTree, then they're not subtypes nor are
they provably disjoint from the scrutinee type.

Martin mentioned that it's been tried a few times to restrict patterns
to subtypes, but that it's been found too restrictive and provably
disjoint is the right choice for type errors.

So the logic that I recently reimplemented was to only emit reachability
warnings after a reachable a case was found, therefore avoiding to emit
any warnings in the ThisTree case, emitting for all the overlap
unreachable cases.

The problem was that accidentally introduced a dependency on the
ordering of the cases, where unreachable initial cases aren't warned but
reordered they would.  So I hold onto those references and unbuffer as
soon as we find a provably reachable case (covered != Empty) or a
provably unreachable case (prev != Empty).

Also, I've had separate conversations with Seth and Lukas about whether
match analysis should just widen those non-class types, but it's also
plausible that a user has created a whole type hierarchy with type
members and would prefer for it to not discard it all by widening to
class/object types, but instead reason within the bounds of the given
scrutinee type.

Author: dwijnand

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

@@ -900,7 +900,9 @@ class SpaceEngine(using Context) extends SpaceLogic {
   def checkRedundancy(_match: Match): Unit = {
     debug.println(s"---------------checking redundant patterns ${_match.show}")
 
-    val Match(sel, cases) = _match
+    val Match(sel, _) = _match
+    val cases = _match.cases.toIndexedSeq
+
     val selTyp = sel.tpe.widen.dealias
 
     if (!redundancyCheckable(sel)) return
@@ -911,7 +913,14 @@ class SpaceEngine(using Context) extends SpaceLogic {
       else project(selTyp)
     debug.println(s"targetSpace: ${show(targetSpace)}")
 
-    cases.iterator.zipWithIndex.foldLeft(Nil: List[Space]) { case (prevs, (CaseDef(pat, guard, _), i)) =>
+    var i        = 0
+    val len      = cases.length
+    var prevs    = List.empty[Space]
+    var deferred = List.empty[Tree]
+
+    while (i < len) {
+      val CaseDef(pat, guard, _) = cases(i)
+
       debug.println(i"case pattern: $pat")
 
       val curr = project(pat)
@@ -923,18 +932,24 @@ class SpaceEngine(using Context) extends SpaceLogic {
       val covered = simplify(intersect(curr, targetSpace))
       debug.println(s"covered: ${show(covered)}")
 
-      if pat != EmptyTree // rethrow case of catch uses EmptyTree
-        && prev != Empty // avoid isSubspace(Empty, Empty) - one of the previous cases much be reachable
-        && isSubspace(covered, prev)
-      then {
-        if isNullable && i == cases.length - 1 && isWildcardArg(pat) then
-          report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
-        else
+      if prev == Empty && covered == Empty then // defer until a case is reachable
+        deferred ::= pat
+      else {
+        for (pat <- deferred.reverseIterator)
           report.warning(MatchCaseUnreachable(), pat.srcPos)
+        if pat != EmptyTree // rethrow case of catch uses EmptyTree
+            && isSubspace(covered, prev)
+        then {
+          val nullOnly = isNullable && i == len - 1 && isWildcardArg(pat)
+          val msg = if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
+          report.warning(msg, pat.srcPos)
+        }
+        deferred = Nil
       }
 
       // in redundancy check, take guard as false in order to soundly approximate
-      (if guard.isEmpty then covered else Empty) :: prevs
+      prevs ::= (if guard.isEmpty then covered else Empty)
+      i += 1
     }
   }
 }

tests/patmat/i13485.check

@@ -1 +1,2 @@
+11: Match case Unreachable
 16: Match case Unreachable