Skip to content

Commit

Permalink
Unreachability analysis for pattern matches
Browse files Browse the repository at this point in the history
Analyze matches for unreachable cases.

A case is unreachable if it implies its preceding cases.
Call `C` the formula that is satisfiable if the considered case matches.
Call `P` the formula that is satisfiable if the cases preceding it match.
The case is reachable if there is a model for `-P /\ C`.
Thus, the case is unreachable if there is no model for `-(-P /\ C)`,
or, equivalently, `P \/ -C`, or `C => P`.

Unreachability needs a more precise model for type and value tests than exhaustivity.
Before, `{case _: Int => case 1 =>}` would be modeled as `X = Int \/ X = 1.type`,
and thus, the second case would be reachable if we can satisfy `X != Int /\ X = 1.type`.
Of course, the case isn't reachable, yet the formula is satisfiable, so we must augment
our model to take into account that `X = 1.type => X = Int`.

This is done by `removeVarEq`, which models the following axioms about equality.
It does so to retain the meaning of equality after replacing `V = C` (variable = constant)
by a literal (fresh symbol). For each variable:
 1. a sealed type test must result in exactly one of its partitions being chosen
    (the core of exhaustivity)
 2. when a type test is true, tests of super types must also be true,
    and unrelated type tests must be false

For example, `V : X ::= A | B | C`, and `A => B` (since `A extends B`).
Coverage (1) is formulated as: `A \/ B \/ C`, and the implications of (2) are simply
`V=A => V=B /\ V=X`, `V=B => V=X`, `V=C => V=X`.

Exclusion for unrelated types typically results from matches such as `{case SomeConst =>
case OtherConst => }`. Here, `V=SomeConst.type => !V=OtherConst.type`. This is a
conservative approximation. If these constants happen to be the same value dynamically
(but the types don't tell us this), the last case is actually unreachable. Of course we
must err on the safe side.

We simplify the equality axioms as follows (in principle this could be done by the
solver, but it's easy to do before solving). If we've already excluded a pair of
assignments of constants to a certain variable at some point, say `(-A \/ -B)`, then
don't exclude the symmetric one `(-B \/ -A)`. (Nor the positive implications `-B \/ A`,
or `-A \/ B`, which would entail the equality axioms falsifying the whole formula.)

TODO: We should also model dependencies between variables: if `V1` corresponds to
`x: List[_]` and `V2` is `x.hd`, `V2` cannot be assigned at all when `V1 = null` or
`V1 = Nil`. Right now this is implemented hackily by pruning counter-examples in exhaustivity.
Unreachability would also benefit from a more faithful representation.

I had to refactor some of the framework, but most of it is shared with exhaustivity. We
must allow approximating tree makers twice, sharing variables, but using different
approximations for values not statically known. When considering reachability of a case,
we must assume, for example, that its unknown guard succeeds (otherwise it would wrongly
be considered unreachable), whereas unknown guards in the preceding cases must be
considered to fail (otherwise we could never get to those case, and again, it would
falsely be considered unreachable).

Since this analysis is relatively expensive, you may opt-out using `-Xno-patmat-analysis`
(or annotating the selector with @unchecked). We hope to improve the performance in the
near future. -Ystatistics has also been extended to provide some numbers on time spent in
the equality-rewrite, solving and analyzing.
  • Loading branch information
adriaanm committed Jun 1, 2012
1 parent 01ac32a commit 379384c
Show file tree
Hide file tree
Showing 10 changed files with 807 additions and 249 deletions.
951 changes: 706 additions & 245 deletions src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala

Large diffs are not rendered by default.

29 changes: 26 additions & 3 deletions src/compiler/scala/tools/nsc/util/Statistics.scala
Expand Up @@ -60,6 +60,14 @@ class Statistics extends scala.reflect.internal.util.Statistics {

val macroExpandCount = new Counter
val macroExpandNanos = new Timer

val patmatNanos = new Timer
val patmatAnaDPLL = new Timer
val patmatAnaVarEq = new Timer
val patmatCNF = new Timer
val patmatAnaExhaust = new Timer
val patmatAnaReach = new Timer
val patmatCNFSizes = new collection.mutable.HashMap[Int, Int] withDefaultValue 0
}

object Statistics extends Statistics
Expand All @@ -71,7 +79,7 @@ abstract class StatisticsInfo {
val global: Global
import global._

var phasesShown = List("parser", "typer", "erasure", "cleanup")
var phasesShown = List("parser", "typer", "patmat", "erasure", "cleanup")

def countNodes(tree: Tree, counts: ClassCounts) {
for (t <- tree) counts(t.getClass) += 1
Expand All @@ -83,10 +91,15 @@ abstract class StatisticsInfo {
def showRelTyper(timer: Timer) =
timer+showPercent(timer.nanos, typerNanos.nanos)

def showCounts(counts: ClassCounts) =
def showRelPatmat(timer: Timer) =
timer+showPercent(timer.nanos, patmatNanos.nanos)

def showCounts[T](counts: scala.collection.mutable.Map[T, Int]) =
counts.toSeq.sortWith(_._2 > _._2).map {
case (cls, cnt) =>
case (cls: Class[_], cnt) =>
cls.toString.substring(cls.toString.lastIndexOf("$") + 1)+": "+cnt
case (o, cnt) =>
o.toString +": "+cnt
}

def print(phase: Phase) = if (phasesShown contains phase.name) {
Expand Down Expand Up @@ -169,6 +182,16 @@ abstract class StatisticsInfo {
if (timer1 != null) inform("#timer1 : " + timer1)
if (timer2 != null) inform("#timer2 : " + timer2)
//for (t <- uniques.iterator) println("unique: "+t)

if (phase.name == "patmat") {
inform("time spent in patmat : " + patmatNanos )
inform(" of which DPLL : " + showRelPatmat(patmatAnaDPLL ))
inform("of which in CNF conversion : " + showRelPatmat(patmatCNF ))
inform(" CNF size counts : " + showCounts(patmatCNFSizes ))
inform("of which variable equality : " + showRelPatmat(patmatAnaVarEq ))
inform(" of which in exhaustivity : " + showRelPatmat(patmatAnaExhaust))
inform("of which in unreachability : " + showRelPatmat(patmatAnaReach ))
}
}
}
}
5 changes: 4 additions & 1 deletion test/files/neg/patmatexhaust.check
Expand Up @@ -14,6 +14,9 @@ patmatexhaust.scala:49: error: match may not be exhaustive.
It would fail on the following inputs: Gp(), Gu
def ma4(x:Deep) = x match { // missing cases: Gu, Gp
^
patmatexhaust.scala:55: error: unreachable code
case _ if 1 == 0 =>
^
patmatexhaust.scala:53: error: match may not be exhaustive.
It would fail on the following input: Gp()
def ma5(x:Deep) = x match {
Expand All @@ -34,4 +37,4 @@ patmatexhaust.scala:126: error: match may not be exhaustive.
It would fail on the following input: C1()
def ma10(x: C) = x match { // not exhaustive: C1 is not abstract.
^
9 errors found
10 errors found
4 changes: 4 additions & 0 deletions test/files/neg/virtpatmat_reach_null.check
@@ -0,0 +1,4 @@
virtpatmat_reach_null.scala:13: error: unreachable code
case _ => // unreachable
^
one error found
1 change: 1 addition & 0 deletions test/files/neg/virtpatmat_reach_null.flags
@@ -0,0 +1 @@
-Xfatal-warnings
19 changes: 19 additions & 0 deletions test/files/neg/virtpatmat_reach_null.scala
@@ -0,0 +1,19 @@
sealed abstract class Const {
final def excludes(other: Const) =
(this, other) match {
case (_, NullConst) =>
case (NullConst, _) =>
case (_: ValueConst, _: ValueConst) =>
case (_: ValueConst, _: TypeConst) =>
case (_: TypeConst, _: ValueConst) =>
case (_: TypeConst, _: TypeConst) =>
case (null, _) =>
case (_, null) =>
case null =>
case _ => // unreachable
}
}

sealed class TypeConst extends Const
sealed class ValueConst extends Const
case object NullConst extends Const
14 changes: 14 additions & 0 deletions test/files/neg/virtpatmat_reach_sealed_unsealed.check
@@ -0,0 +1,14 @@
virtpatmat_reach_sealed_unsealed.scala:16: error: match may not be exhaustive.
It would fail on the following input: false
(true: Boolean) match { case true => } // not exhaustive, but reachable
^
virtpatmat_reach_sealed_unsealed.scala:18: error: unreachable code
(true: Boolean) match { case true => case false => case _ => } // exhaustive, last case is unreachable
^
virtpatmat_reach_sealed_unsealed.scala:19: error: unreachable code
(true: Boolean) match { case true => case false => case _: Boolean => } // exhaustive, last case is unreachable
^
virtpatmat_reach_sealed_unsealed.scala:20: error: unreachable code
(true: Boolean) match { case true => case false => case _: Any => } // exhaustive, last case is unreachable
^
four errors found
1 change: 1 addition & 0 deletions test/files/neg/virtpatmat_reach_sealed_unsealed.flags
@@ -0,0 +1 @@
-Xfatal-warnings
21 changes: 21 additions & 0 deletions test/files/neg/virtpatmat_reach_sealed_unsealed.scala
@@ -0,0 +1,21 @@
sealed abstract class X
sealed case class A(x: Int) extends X

// test reachability on mixed sealed / non-sealed matches
object Test extends App {
val B: X = A(0)
val C: X = A(1)

// all cases are reachable and the match is exhaustive
(C: X) match {
case B =>
case C =>
case A(_) =>
}

(true: Boolean) match { case true => } // not exhaustive, but reachable
(true: Boolean) match { case true => case false => } // exhaustive, reachable
(true: Boolean) match { case true => case false => case _ => } // exhaustive, last case is unreachable
(true: Boolean) match { case true => case false => case _: Boolean => } // exhaustive, last case is unreachable
(true: Boolean) match { case true => case false => case _: Any => } // exhaustive, last case is unreachable
}
11 changes: 11 additions & 0 deletions test/files/pos/virtpatmat_reach_const.scala
@@ -0,0 +1,11 @@
// check the interaction between constants and type tests in creating the equality axioms
object Test {
type Formula = List[String]
val TrueF: Formula = List()
def distribute(a: Formula, b: Formula) = (a, b) match {
case (TrueF, _) =>
case (_, TrueF) => // bug: considered unreachable
case (a :: Nil, b :: Nil) =>
case _ =>
}
}

0 comments on commit 379384c

Please sign in to comment.