Skip to content

Commit b651b85

Browse files
liufengyunmichelou
authored andcommitted
Fix scala#12241: Make space computation lazy
1 parent 2e695d6 commit b651b85

File tree

3 files changed

+37
-40
lines changed

3 files changed

+37
-40
lines changed

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

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ case class Typ(tp: Type, decomposed: Boolean = true) extends Space
7272
case class Prod(tp: Type, unappTp: TermRef, params: List[Space]) extends Space
7373

7474
/** Union of spaces */
75-
case class Or(spaces: List[Space]) extends Space
75+
case class Or(spaces: Seq[Space]) extends Space
7676

7777
/** abstract space logic */
7878
trait SpaceLogic {
@@ -113,45 +113,40 @@ trait SpaceLogic {
113113
/** Display space in string format */
114114
def show(sp: Space): String
115115

116-
/** Simplify space using the laws, there's no nested union after simplify
117-
*
118-
* @param aggressive if true and OR space has less than 5 components, `simplify` will
119-
* collapse `sp1 | sp2` to `sp1` if `sp2` is a subspace of `sp1`.
120-
*
121-
* This reduces noise in counterexamples.
122-
*/
123-
def simplify(space: Space, aggressive: Boolean = false)(using Context): Space = trace(s"simplify ${show(space)}, aggressive = $aggressive --> ", debug, x => show(x.asInstanceOf[Space]))(space match {
116+
/** Simplify space using the laws, there's no nested union after simplify */
117+
def simplify(space: Space)(using Context): Space = trace(s"simplify ${show(space)} --> ", debug, x => show(x.asInstanceOf[Space]))(space match {
124118
case Prod(tp, fun, spaces) =>
125119
val sp = Prod(tp, fun, spaces.map(simplify(_)))
126120
if (sp.params.contains(Empty)) Empty
127121
else if (canDecompose(tp) && decompose(tp).isEmpty) Empty
128122
else sp
129123
case Or(spaces) =>
130-
val buf = new mutable.ListBuffer[Space]
131-
def include(s: Space) = if s != Empty then buf += s
132-
for space <- spaces do
133-
simplify(space) match
134-
case Or(ss) => ss.foreach(include)
135-
case s => include(s)
136-
val set = buf.toList
137-
138-
if (set.isEmpty) Empty
139-
else if (set.size == 1) set.toList(0)
140-
else if (aggressive && spaces.size < 5) {
141-
val res = set.map(sp => (sp, set.filter(_ ne sp))).find {
142-
case (sp, sps) =>
143-
isSubspace(sp, Or(sps))
144-
}
145-
if (res.isEmpty) Or(set)
146-
else simplify(Or(res.get._2), aggressive)
147-
}
148-
else Or(set)
124+
val spaces2 = spaces.map(simplify(_)).filter(_ != Empty)
125+
if spaces2.isEmpty then Empty
126+
else Or(spaces2)
149127
case Typ(tp, _) =>
150128
if (canDecompose(tp) && decompose(tp).isEmpty) Empty
151129
else space
152130
case _ => space
153131
})
154132

133+
/** Remove a space if it's a subspace of remaining spaces
134+
*
135+
* Note: `dedup` will return the same result if the sequence > 10
136+
*/
137+
def dedup(spaces: Seq[Space])(using Context): Seq[Space] = {
138+
val total = spaces.take(10)
139+
140+
if (total.size < 1 || total.size >= 10) total
141+
else {
142+
val res = spaces.map(sp => (sp, spaces.filter(_ ne sp))).find {
143+
case (sp, sps) => isSubspace(sp, Or(LazyList(sps: _*)))
144+
}
145+
if (res.isEmpty) spaces
146+
else res.get._2
147+
}
148+
}
149+
155150
/** Flatten space to get rid of `Or` for pretty print */
156151
def flatten(space: Space)(using Context): Seq[Space] = space match {
157152
case Prod(tp, fun, spaces) =>
@@ -205,8 +200,8 @@ trait SpaceLogic {
205200

206201
(a, b) match {
207202
case (Empty, _) | (_, Empty) => Empty
208-
case (_, Or(ss)) => Or(ss.map(intersect(a, _)).filterConserve(_ ne Empty))
209-
case (Or(ss), _) => Or(ss.map(intersect(_, b)).filterConserve(_ ne Empty))
203+
case (_, Or(ss)) => Or(ss.map(intersect(a, _)).filter(_ ne Empty))
204+
case (Or(ss), _) => Or(ss.map(intersect(_, b)).filter(_ ne Empty))
210205
case (Typ(tp1, _), Typ(tp2, _)) =>
211206
if (isSubType(tp1, tp2)) a
212207
else if (isSubType(tp2, tp1)) b
@@ -282,7 +277,7 @@ trait SpaceLogic {
282277
else if cache.forall(sub => isSubspace(sub, Empty)) then Empty
283278
else
284279
// `(_, _, _) - (Some, None, _)` becomes `(None, _, _) | (_, Some, _) | (_, _, Empty)`
285-
Or(range.map { i => Prod(tp1, fun1, ss1.updated(i, sub(i))) })
280+
Or(LazyList(range: _*).map { i => Prod(tp1, fun1, ss1.updated(i, sub(i))) })
286281
}
287282
}
288283
}
@@ -601,7 +596,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
601596
tp.dealias match {
602597
case AndType(tp1, tp2) =>
603598
intersect(Typ(tp1, false), Typ(tp2, false)) match {
604-
case Or(spaces) => spaces
599+
case Or(spaces) => spaces.toList
605600
case Empty => Nil
606601
case space => List(space)
607602
}
@@ -842,14 +837,15 @@ class SpaceEngine(using Context) extends SpaceLogic {
842837
val checkGADTSAT = shouldCheckExamples(selTyp)
843838

844839
val uncovered =
845-
flatten(simplify(minus(project(selTyp), patternSpace), aggressive = true)).filter { s =>
840+
flatten(simplify(minus(project(selTyp), patternSpace))).filter({ s =>
846841
s != Empty && (!checkGADTSAT || satisfiable(s))
847-
}
842+
})
848843

849844

850845
if uncovered.nonEmpty then
851846
val hasMore = uncovered.lengthCompare(6) > 0
852-
report.warning(PatternMatchExhaustivity(show(uncovered.take(6)), hasMore), sel.srcPos)
847+
val deduped = dedup(uncovered.take(6))
848+
report.warning(PatternMatchExhaustivity(show(deduped), hasMore), sel.srcPos)
853849
}
854850

855851
private def redundancyCheckable(sel: Tree): Boolean =
@@ -908,10 +904,11 @@ class SpaceEngine(using Context) extends SpaceLogic {
908904
// If explicit nulls are enabled, this check isn't needed because most of the cases
909905
// that would trigger it would also trigger unreachability warnings.
910906
if (!ctx.explicitNulls && i == cases.length - 1 && !isNullLit(pat) ) {
911-
simplify(minus(covered, prevs)) match {
912-
case Typ(`constantNullType`, _) =>
907+
dedup(flatten(simplify(minus(covered, prevs)))).toList match {
908+
case Typ(`constantNullType`, _) :: Nil =>
913909
report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
914-
case _ =>
910+
case s =>
911+
debug.println("`_` matches = " + s)
915912
}
916913
}
917914
}

tests/patmat/i8922c.check

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

tests/patmat/t10019.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
2: Pattern Match Exhaustivity: (List(_, _, _*), List(_, _*)), (Nil, List(_, _*)), (List(_, _*), List(_, _, _*)), (List(_, _*), Nil)
1+
2: Pattern Match Exhaustivity: (List(_, _, _*), List(_, _*)), (Nil, List(_, _*)), (List(_, _*), List(_, _, _*)), (List(_, _*), Nil), (_: List, List(_, _, _*))
22
11: Pattern Match Exhaustivity: (Foo(None), Foo(_))

0 commit comments

Comments
 (0)