Skip to content

Commit 495ed99

Browse files
committed
Fix #2363: better handle extractors in exhaustivity check
1. Better handle exactors 2. Translate the pattern `List(pat1, pat2, c:_*)` to space semantically 3. Deal with extractors that return `Some[_]` Previously we approximate extractor either to the extracted type or Empty, which is too coarse, and it causes some false positives about unreachable case. Introducing the concept extractor into the theory enables us to inspect the patterns inside extractors and check more unreachable cases, which is not supported by Scalac. Scalac is silent with following code, but Dotty reports both unexhaustive and unreachable warnings. sealed trait Node case class NodeA(i: Int) extends Node case class NodeB(b: Boolean) extends Node case class NodeC(s: String) extends Node object Node { def unapply(node: Node): Option[(Node, Node)] = ??? } object Test { def foo(x: Node): Boolean = x match { // unexhaustive case Node(NodeA(_), NodeB(_)) => true case Node(NodeA(4), NodeB(false)) => true // unreachable code } }
1 parent cc4533d commit 495ed99

File tree

13 files changed

+164
-42
lines changed

13 files changed

+164
-42
lines changed

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

Lines changed: 123 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import config.Printers.{ exhaustivity => debug }
2929
* 3. A union of spaces `S1 | S2 | ...` is a space
3030
* 4. For a case class Kon(x1: T1, x2: T2, .., xn: Tn), if S1, S2, ..., Sn
3131
* are spaces, then `Kon(S1, S2, ..., Sn)` is a space.
32+
* 5. `Fun(S1, S2, ..., Sn)` is an extractor space.
3233
*
3334
* For the problem of exhaustivity check, its formulation in terms of space is as follows:
3435
*
@@ -63,6 +64,9 @@ case class Typ(tp: Type, decomposed: Boolean) extends Space
6364
/** Space representing a constructor pattern */
6465
case class Kon(tp: Type, params: List[Space]) extends Space
6566

67+
/** Space representing an extractor pattern */
68+
case class Fun(tp: Type, fun: Type, params: List[Space]) extends Space
69+
6670
/** Union of spaces */
6771
case class Or(spaces: List[Space]) extends Space
6872

@@ -98,19 +102,31 @@ trait SpaceLogic {
98102
def show(sp: Space): String
99103

100104
/** Simplify space using the laws, there's no nested union after simplify */
101-
def simplify(space: Space): Space = space match {
105+
def simplify(space: Space, aggressive: Boolean = false): Space = space match {
102106
case Kon(tp, spaces) =>
103-
val sp = Kon(tp, spaces.map(simplify _))
107+
val sp = Kon(tp, spaces.map(simplify(_)))
108+
if (sp.params.contains(Empty)) Empty
109+
else sp
110+
case Fun(tp, fun, spaces) =>
111+
val sp = Fun(tp, fun, spaces.map(simplify(_)))
104112
if (sp.params.contains(Empty)) Empty
105113
else sp
106114
case Or(spaces) =>
107-
val set = spaces.map(simplify _).flatMap {
115+
val set = spaces.map(simplify(_)).flatMap {
108116
case Or(ss) => ss
109117
case s => Seq(s)
110118
} filter (_ != Empty)
111119

112120
if (set.isEmpty) Empty
113121
else if (set.size == 1) set.toList(0)
122+
else if (aggressive && spaces.size < 5) {
123+
val res = set.map(sp => (sp, set.filter(_ ne sp))).find {
124+
case (sp, sps) =>
125+
isSubspace(sp, Or(sps))
126+
}
127+
if (res.isEmpty) Or(set)
128+
else simplify(Or(res.get._2), aggressive)
129+
}
114130
else Or(set)
115131
case Typ(tp, _) =>
116132
if (canDecompose(tp) && decompose(tp).isEmpty) Empty
@@ -137,26 +153,33 @@ trait SpaceLogic {
137153
def tryDecompose1(tp: Type) = canDecompose(tp) && isSubspace(Or(decompose(tp)), b)
138154
def tryDecompose2(tp: Type) = canDecompose(tp) && isSubspace(a, Or(decompose(tp)))
139155

140-
val res = (a, b) match {
156+
val res = (simplify(a), b) match {
141157
case (Empty, _) => true
142158
case (_, Empty) => false
143-
case (Or(ss), _) => ss.forall(isSubspace(_, b))
159+
case (Or(ss), _) =>
160+
ss.forall(isSubspace(_, b))
144161
case (Typ(tp1, _), Typ(tp2, _)) =>
145-
isSubType(tp1, tp2) || tryDecompose1(tp1) || tryDecompose2(tp2)
146-
case (Typ(tp1, _), Or(ss)) =>
162+
isSubType(tp1, tp2)
163+
case (Typ(tp1, _), Or(ss)) => // optimization
147164
ss.exists(isSubspace(a, _)) || tryDecompose1(tp1)
165+
case (_, Or(_)) =>
166+
simplify(minus(a, b)) == Empty
148167
case (Typ(tp1, _), Kon(tp2, ss)) =>
149-
isSubType(tp1, tp2) && isSubspace(Kon(tp2, signature(tp2).map(Typ(_, false))), b) ||
150-
tryDecompose1(tp1)
168+
isSubType(tp1, tp2) && isSubspace(Kon(tp2, signature(tp2).map(Typ(_, false))), b)
151169
case (Kon(tp1, ss), Typ(tp2, _)) =>
152-
isSubType(tp1, tp2) ||
153-
simplify(a) == Empty ||
154-
(isSubType(tp2, tp1) && tryDecompose1(tp1)) ||
155-
tryDecompose2(tp2)
156-
case (Kon(_, _), Or(_)) =>
157-
simplify(minus(a, b)) == Empty
170+
isSubType(tp1, tp2)
158171
case (Kon(tp1, ss1), Kon(tp2, ss2)) =>
159172
isEqualType(tp1, tp2) && ss1.zip(ss2).forall((isSubspace _).tupled)
173+
case (Fun(tp1, fun, ss), Typ(tp2, _)) =>
174+
isSubType(tp1, tp2)
175+
case (Typ(tp2, _), Fun(tp1, fun, ss)) =>
176+
false // approximation: assume a type can never be fully matched by an extractor
177+
case (Kon(_, _), Fun(_, _, _)) =>
178+
false // approximation
179+
case (Fun(_, _, _), Kon(_, _)) =>
180+
false // approximation
181+
case (Fun(_, fun1, ss1), Fun(_, fun2, ss2)) =>
182+
isEqualType(fun1, fun2) && ss1.zip(ss2).forall((isSubspace _).tupled)
160183
}
161184

162185
debug.println(s"${show(a)} < ${show(b)} = $res")
@@ -169,7 +192,7 @@ trait SpaceLogic {
169192
def tryDecompose1(tp: Type) = intersect(Or(decompose(tp)), b)
170193
def tryDecompose2(tp: Type) = intersect(a, Or(decompose(tp)))
171194

172-
val res = (a, b) match {
195+
val res: Space = (a, b) match {
173196
case (Empty, _) | (_, Empty) => Empty
174197
case (_, Or(ss)) => Or(ss.map(intersect(a, _)).filterConserve(_ ne Empty))
175198
case (Or(ss), _) => Or(ss.map(intersect(_, b)).filterConserve(_ ne Empty))
@@ -193,6 +216,24 @@ trait SpaceLogic {
193216
if (!isEqualType(tp1, tp2)) Empty
194217
else if (ss1.zip(ss2).exists(p => simplify(intersect(p._1, p._2)) == Empty)) Empty
195218
else Kon(tp1, ss1.zip(ss2).map((intersect _).tupled))
219+
case (Typ(tp1, _), Fun(tp2, _, _)) =>
220+
if (isSubType(tp1, tp2) || isSubType(tp2, tp1)) b // prefer extractor space for better approximation
221+
else if (canDecompose(tp1)) tryDecompose1(tp1)
222+
else Empty
223+
case (Fun(tp1, _, _), Typ(tp2, _)) =>
224+
if (isSubType(tp1, tp2) || isSubType(tp2, tp1)) a
225+
else if (canDecompose(tp2)) tryDecompose2(tp2)
226+
else Empty
227+
case (Fun(tp1, _, _), Kon(tp2, _)) =>
228+
if (isSubType(tp1, tp2) || isSubType(tp2, tp1)) a
229+
else Empty
230+
case (Kon(tp1, _), Fun(tp2, _, _)) =>
231+
if (isSubType(tp1, tp2) || isSubType(tp2, tp1)) b
232+
else Empty
233+
case (Fun(tp1, fun1, ss1), Fun(tp2, fun2, ss2)) =>
234+
if (!isEqualType(fun1, fun2)) Empty
235+
else if (ss1.zip(ss2).exists(p => simplify(intersect(p._1, p._2)) == Empty)) Empty
236+
else Fun(tp1, fun1, ss1.zip(ss2).map((intersect _).tupled))
196237
}
197238

198239
debug.println(s"${show(a)} & ${show(b)} = ${show(res)}")
@@ -238,6 +279,25 @@ trait SpaceLogic {
238279
Or(ss1.zip(ss2).map((minus _).tupled).zip(0 to ss2.length - 1).map {
239280
case (ri, i) => Kon(tp1, ss1.updated(i, ri))
240281
})
282+
case (Fun(tp1, _, _), Typ(tp2, _)) =>
283+
if (isSubType(tp1, tp2)) Empty
284+
else a
285+
case (Typ(tp1, _), Fun(tp2, _, _)) =>
286+
a // approximation
287+
case (Fun(_, _, _), Kon(_, _)) =>
288+
a
289+
case (Kon(_, _), Fun(_, _, _)) =>
290+
a
291+
case (Fun(tp1, fun1, ss1), Fun(tp2, fun2, ss2)) =>
292+
if (!isEqualType(fun1, fun2)) a
293+
else if (ss1.zip(ss2).exists(p => simplify(intersect(p._1, p._2)) == Empty)) a
294+
else if (ss1.zip(ss2).forall((isSubspace _).tupled)) Empty
295+
else
296+
// `(_, _, _) - (Some, None, _)` becomes `(None, _, _) | (_, Some, _) | (_, _, Empty)`
297+
Or(ss1.zip(ss2).map((minus _).tupled).zip(0 to ss2.length - 1).map {
298+
case (ri, i) => Fun(tp1, fun1, ss1.updated(i, ri))
299+
})
300+
241301
}
242302

243303
debug.println(s"${show(a)} - ${show(b)} = ${show(res)}")
@@ -263,6 +323,12 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
263323
import SpaceEngine._
264324
import tpd._
265325

326+
private val scalaSomeClass = ctx.requiredClassRef("scala.Some".toTermName).symbol.asClass
327+
private val scalaSeqFactoryClass = ctx.requiredClass("scala.collection.generic.SeqFactory".toTypeName)
328+
private val scalaListType = ctx.requiredClassRef("scala.collection.immutable.List".toTypeName)
329+
private val scalaNilType = ctx.requiredModuleRef("scala.collection.immutable.Nil".toTermName)
330+
private val scalaConType = ctx.requiredClassRef("scala.collection.immutable.::".toTypeName)
331+
266332
/** Checks if it's possible to create a trait/class which is a subtype of `tp`.
267333
*
268334
* - doesn't handle member collisions (will not declare a type unimplementable because of one)
@@ -337,11 +403,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
337403
}
338404

339405
/** Return the space that represents the pattern `pat`
340-
*
341-
* If roundUp is true, approximate extractors to its type,
342-
* otherwise approximate extractors to Empty
343406
*/
344-
def project(pat: Tree, roundUp: Boolean = true)(implicit ctx: Context): Space = pat match {
407+
def project(pat: Tree): Space = pat match {
345408
case Literal(c) =>
346409
if (c.value.isInstanceOf[Symbol])
347410
Typ(c.value.asInstanceOf[Symbol].termRef, false)
@@ -350,20 +413,41 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
350413
case _: BackquotedIdent => Typ(pat.tpe, false)
351414
case Ident(_) | Select(_, _) =>
352415
Typ(pat.tpe.stripAnnots, false)
353-
case Alternative(trees) => Or(trees.map(project(_, roundUp)))
416+
case Alternative(trees) => Or(trees.map(project(_)))
354417
case Bind(_, pat) => project(pat)
355-
case UnApply(_, _, pats) =>
418+
case UnApply(fun, _, pats) =>
356419
if (pat.tpe.classSymbol.is(CaseClass))
357420
// FIXME: why dealias is needed here?
358-
Kon(pat.tpe.stripAnnots.dealias, pats.map(pat => project(pat, roundUp)))
359-
else if (roundUp) Typ(pat.tpe.stripAnnots, false)
360-
else Empty
421+
Kon(pat.tpe.stripAnnots.dealias, pats.map(pat => project(pat)))
422+
else if (fun.symbol.owner == scalaSeqFactoryClass && fun.symbol.name == nme.unapplySeq)
423+
projectList(pats)
424+
else if (fun.symbol.info.resultType.isRef(scalaSomeClass))
425+
Kon(pat.tpe.stripAnnots.dealias, pats.map(pat => project(pat)))
426+
else
427+
Fun(pat.tpe.stripAnnots.dealias, fun.tpe, pats.map(pat => project(pat)))
361428
case Typed(pat @ UnApply(_, _, _), _) => project(pat)
362429
case Typed(expr, _) => Typ(expr.tpe.stripAnnots, true)
363430
case _ =>
364431
Empty
365432
}
366433

434+
435+
/** Space of the pattern: List(a, b, c: _*)
436+
*/
437+
def projectList(pats: List[Tree]): Space = {
438+
if (pats.isEmpty) return Typ(scalaNilType, false)
439+
440+
val (items, zero) = if (pats.last.tpe.isRepeatedParam)
441+
(pats.init, Typ(scalaListType.appliedTo(pats.head.tpe.widen), false))
442+
else
443+
(pats, Typ(scalaNilType, false))
444+
445+
items.foldRight[Space](zero) { (pat, acc) =>
446+
Kon(scalaConType.appliedTo(pats.head.tpe.widen), project(pat) :: acc :: Nil)
447+
}
448+
}
449+
450+
367451
/* Erase a type binding according to erasure semantics in pattern matching */
368452
def erase(tp: Type): Type = {
369453
def doErase(tp: Type): Type = tp match {
@@ -549,6 +633,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
549633

550634
if (ctx.definitions.isTupleType(tp))
551635
signature(tp).map(_ => "_").mkString("(", ", ", ")")
636+
else if (sym.showFullName == "scala.collection.immutable.List")
637+
if (mergeList) "_*" else "_: List"
552638
else if (sym.showFullName == "scala.collection.immutable.::")
553639
if (mergeList) "_" else "List(_)"
554640
else if (tp.classSymbol.is(CaseClass))
@@ -566,6 +652,8 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
566652
else params.map(doShow(_, true)).filter(_ != "Nil").mkString("List(", ", ", ")")
567653
else
568654
showType(tp) + params.map(doShow(_)).mkString("(", ", ", ")")
655+
case Fun(tp, fun, params) =>
656+
showType(fun) + params.map(doShow(_)).mkString("(", ", ", ")")
569657
case Or(_) =>
570658
throw new Exception("incorrect flatten result " + s)
571659
}
@@ -664,31 +752,35 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
664752

665753
val patternSpace = cases.map({ x =>
666754
val space = project(x.pat)
667-
debug.println(s"${x.pat.show} projects to ${show(space)}")
755+
debug.println(s"${x.pat.show} ====> ${show(space)}")
668756
space
669757
}).reduce((a, b) => Or(List(a, b)))
670-
val uncovered = simplify(minus(Typ(selTyp, true), patternSpace))
758+
val uncovered = simplify(minus(Typ(selTyp, true), patternSpace), aggressive = true)
671759

672760
if (uncovered != Empty)
673-
ctx.warning(PatternMatchExhaustivity(show(uncovered)), _match.pos)
761+
ctx.warning(PatternMatchExhaustivity(show(uncovered)), sel.pos)
674762
}
675763

676764
def checkRedundancy(_match: Match): Unit = {
677765
val Match(sel, cases) = _match
678766
// ignore selector type for now
679767
// val selTyp = sel.tpe.widen.deAnonymize.dealias
680768

769+
if (cases.length == 1) return
770+
681771
// starts from the second, the first can't be redundant
682772
(1 until cases.length).foreach { i =>
683-
// in redundancy check, take guard as false, take extractor as match
684-
// nothing in order to soundly approximate
773+
// in redundancy check, take guard as false in order to soundly approximate
685774
val prevs = cases.take(i).map { x =>
686-
if (x.guard.isEmpty) project(x.pat, false)
775+
if (x.guard.isEmpty) project(x.pat)
687776
else Empty
688777
}.reduce((a, b) => Or(List(a, b)))
689778

690779
val curr = project(cases(i).pat)
691780

781+
debug.println(s"---------------reachable? ${show(curr)}")
782+
debug.println(s"prev: ${show(prevs)}")
783+
692784
if (isSubspace(curr, prevs)) {
693785
ctx.warning(MatchCaseUnreachable(), cases(i).body.pos)
694786
}

tests/patmat/exhausting.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
21: Pattern Match Exhaustivity: List(_), List(_, _, _)
22
27: Pattern Match Exhaustivity: Nil
3-
32: Pattern Match Exhaustivity: List(_, _)
3+
32: Pattern Match Exhaustivity: List(_, _*)
44
39: Pattern Match Exhaustivity: Bar3
55
44: Pattern Match Exhaustivity: (Bar2, Bar2)
66
53: Pattern Match Exhaustivity: (Bar2, Bar2), (Bar2, Bar1), (Bar1, Bar3), (Bar1, Bar2)

tests/patmat/i2363.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
15: Pattern Match Exhaustivity: List(_, _*)
2+
21: Pattern Match Exhaustivity: _: Expr

tests/patmat/i2363.scala

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
sealed trait Expr
2+
class IntExpr extends Expr
3+
class BooleanExpr extends Expr
4+
5+
object IntExpr {
6+
def unapply(expr: Expr): Option[IntExpr] = ???
7+
}
8+
9+
object BooleanExpr {
10+
def unapply(expr: Expr): Option[BooleanExpr] = ???
11+
}
12+
13+
14+
class Test {
15+
def foo(x: List[Expr]): Int = x match {
16+
case IntExpr(_) :: xs => 1
17+
case BooleanExpr(_) :: xs => 1
18+
case Nil => 2
19+
}
20+
21+
def bar(x: Expr): Int = x match {
22+
case IntExpr(_) => 1
23+
case BooleanExpr(_) => 2
24+
}
25+
}

tests/patmat/patmat-adt.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
7: Pattern Match Exhaustivity: Bad(Good(_)), Good(Bad(_))
22
19: Pattern Match Exhaustivity: Some(_)
3-
24: Pattern Match Exhaustivity: (None, Some(_)), (_, Some(_))
3+
24: Pattern Match Exhaustivity: (_, Some(_))
44
29: Pattern Match Exhaustivity: (None, None), (Some(_), Some(_))
55
50: Pattern Match Exhaustivity: LetL(BooleanLit), LetL(IntLit)

tests/patmat/patmat-extractor.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
13: Pattern Match Exhaustivity: _: Node
2+
15: Match case Unreachable

tests/patmat/patmatexhaust.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
11: Pattern Match Exhaustivity: Bar(_)
33
23: Pattern Match Exhaustivity: (Qult(), Qult()), (Kult(_), Kult(_))
44
49: Pattern Match Exhaustivity: _: Gp
5+
59: Pattern Match Exhaustivity: Nil
56
75: Pattern Match Exhaustivity: _: B
67
100: Pattern Match Exhaustivity: _: C1
78
114: Pattern Match Exhaustivity: D2(), D1

tests/patmat/t6420.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
5: Pattern Match Exhaustivity: (Nil, _), (List(true, _), _), (List(false, _), _), (_, Nil), (_, List(true, _)), (_, List(false, _))
1+
5: Pattern Match Exhaustivity: (_: List, Nil), (_: List, List(true, _*)), (_: List, List(false, _*))

tests/patmat/t7020.check

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
3: Pattern Match Exhaustivity: List(_, _)
2-
10: Pattern Match Exhaustivity: List(_, _)
3-
17: Pattern Match Exhaustivity: List(_, _)
4-
24: Pattern Match Exhaustivity: List(_, _)
1+
3: Pattern Match Exhaustivity: List(_, _*)
2+
10: Pattern Match Exhaustivity: List(_, _*)
3+
17: Pattern Match Exhaustivity: List(_, _*)
4+
24: Pattern Match Exhaustivity: List(_, _*)

tests/patmat/t7466.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8: Pattern Match Exhaustivity: (true, _), (false, _), (_, true), (_, false)
1+
8: Pattern Match Exhaustivity: (_, true), (_, false)

tests/patmat/t9129.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
21: Pattern Match Exhaustivity: Two(B2, A2), Two(_, A2)
1+
21: Pattern Match Exhaustivity: Two(_, A2)

tests/patmat/t9232.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
13: Pattern Match Exhaustivity: Node2()
1+
13: Pattern Match Exhaustivity: Node2(), Node1(Foo(_))

tests/patmat/t9351.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
8: Pattern Match Exhaustivity: _: A
2-
17: Pattern Match Exhaustivity: (_, _), (_, None), (_, Some(_))
2+
17: Pattern Match Exhaustivity: (_, None), (_, Some(_))
33
28: Pattern Match Exhaustivity: (_, _)

0 commit comments

Comments
 (0)