Permalink
Browse files

SI-7369 Avoid spurious unreachable warnings in patterns

Unreachability analysis draws on the enumerated domain of types
(e.g sealed subclasses + null, or true/false), and also looks at all
stable identifier patterns tested for equality against the same 'slot'
in a pattern.

It was drawing the wrong conclusions about stable identifier patterns.
Unlike the domain constants, two such values may hold the same value,
so we can't assume that matching X precludes matching Y in the same
slot in a subsequent case.

For example:

    val X: Boolean = true; val Y: Boolean = true
    def m1(t1: Tuple1[Boolean]) = t1 match {
      case Tuple1(true) =>
      case Tuple1(false) =>
      case Tuple1(false) => // correctly unreachable
    }
    def m2(t1: Tuple1[Boolean]) = t1 match {
      case Tuple1(X) =>
      case Tuple1(Y) => // spurious unreachable warning
    }

    //
    // Before
    //
    reachability, vars:
    V2: Boolean ::= true | false// Set(false, Y, X, true) // = x1._1
    V1: (Boolean,) ::= null | ... // = x1
    equality axioms:
      V2=true#4  \/ V2=false#5  /\
     -V2=false#5 \/  -V2=Y#3    /\
     -V2=false#5 \/  -V2=X#2    /\
     -V2=false#5 \/ -V2=true#4  /\
       -V2=Y#3   \/  -V2=X#2    /\
       -V2=Y#3   \/ -V2=true#4  /\
       -V2=X#2   \/ -V2=true#4

    //
    // After
    //
    reachability, vars:
    V2: Boolean ::= true | false// Set(false, Y, X, true) // = x1._1
    V1: (Boolean,) ::= null | ... // = x1
    equality axioms:
      V2=true#4  \/ V2=false#5  /\
     -V2=false#5 \/ -V2=true#4
  • Loading branch information...
1 parent d484c17 commit 62713964b96f64f9c0fd0070c89aa6571679856d @retronym retronym committed Apr 23, 2013
@@ -385,18 +385,35 @@ trait ScalaLogic extends Interface with Logic with TreeAndTypeAnalysis {
// else debug.patmat("NOT implies: "+(lower, upper))
- /** does V = C preclude V having value `other`?
- (1) V = null is an exclusive assignment,
- (2) V = A and V = B, for A and B value constants, are mutually exclusive unless A == B
- we err on the safe side, for example:
- - assume `val X = 1; val Y = 1`, then
- (2: Int) match { case X => case Y => <falsely considered reachable> }
- - V = 1 does not preclude V = Int, or V = Any, it could be said to preclude V = String, but we don't model that
-
- (3) for types we could try to do something fancy, but be conservative and just say no
+ /** Does V=A preclude V=B?
+ *
+ * (0) A or B must be in the domain to draw any conclusions.
+ *
+ * For example, knowing the the scrutinee is *not* true does not
+ * statically exclude it from being `X`, because that is an opaque
+ * Boolean.
+ *
+ * val X = true
+ * (true: Boolean) match { case true => case X <reachable> }
+ *
+ * (1) V = null excludes assignment to any other constant (modulo point #0). This includes
+ * both values and type tests (which are both modelled here as `Const`)
+ * (2) V = A and V = B, for A and B domain constants, are mutually exclusive unless A == B
+ *
+ * (3) We only reason about test tests as being excluded by null assignments, otherwise we
+ * only consider value assignments.
+ * TODO: refine this, a == 0 excludes a: String, or `a: Int` excludes `a: String`
+ * (since no value can be of both types. See also SI-7211)
+ *
+ * NOTE: V = 1 does not preclude V = Int, or V = Any, it could be said to preclude
+ * V = String, but we don't model that.
*/
- def excludes(a: Const, b: Const): Boolean =
- a != b && ((a == NullConst || b == NullConst) || (a.isValue && b.isValue))
+ def excludes(a: Const, b: Const): Boolean = {
+ val bothInDomain = domain exists (d => d(a) && d(b))
+ val eitherIsNull = a == NullConst || b == NullConst
+ val bothAreValues = a.isValue && b.isValue
+ bothInDomain && (eitherIsNull || bothAreValues) && (a != b)
+ }
// if(r) debug.patmat("excludes : "+(a, a.tp, b, b.tp))
// else debug.patmat("NOT excludes: "+(a, b))
View
@@ -0,0 +1,13 @@
+t7369.scala:6: error: unreachable code
+ case Tuple1(X) => // unreachable
+ ^
+t7369.scala:13: error: unreachable code
+ case Tuple1(true) => // unreachable
+ ^
+t7369.scala:31: error: unreachable code
+ case Tuple1(X) => // unreachable
+ ^
+t7369.scala:40: error: unreachable code
+ case Tuple1(null) => // unreachable
+ ^
+four errors found
@@ -0,0 +1 @@
+-Xfatal-warnings
View
@@ -0,0 +1,43 @@
+object Test {
+ val X, Y = true
+ (null: Tuple1[Boolean]) match {
+ case Tuple1(X) =>
+ case Tuple1(Y) =>
+ case Tuple1(X) => // unreachable
+ case _ =>
+ }
+
+ (null: Tuple1[Boolean]) match {
+ case Tuple1(true) =>
+ case Tuple1(false) =>
+ case Tuple1(true) => // unreachable
+ case _ =>
+ }
+}
+
+
+sealed abstract class B;
+case object True extends B;
+case object False extends B;
+
+object Test2 {
+
+ val X: B = True
+ val Y: B = False
+
+ (null: Tuple1[B]) match {
+ case Tuple1(X) =>
+ case Tuple1(Y) =>
+ case Tuple1(X) => // unreachable
+ case _ =>
+ }
+}
+
+object Test3 {
+ (null: Tuple1[B]) match {
+ case Tuple1(null) =>
+ case Tuple1(True) =>
+ case Tuple1(null) => // unreachable
+ case _ =>
+ }
+}
@@ -0,0 +1 @@
+-Xfatal-warnings
View
@@ -0,0 +1,37 @@
+object Test {
+ val X, Y = true
+ (null: Tuple1[Boolean]) match {
+ case Tuple1(X) =>
+ case Tuple1(Y) => // unreachable
+ case _ =>
+ }
+}
+
+
+sealed abstract class B;
+case object True extends B;
+case object False extends B;
+
+object Test2 {
+
+ val X: B = True
+ val Y: B = False
+
+ (null: Tuple1[B]) match {
+ case Tuple1(X) =>
+ case Tuple1(Y) => // no warning
+ case _ =>
+ }
+}
+
+object Test3 {
+ val X, O = true
+ def classify(neighbourhood: (Boolean, Boolean, Boolean)): String = {
+ neighbourhood match {
+ case (X, X, X) => "middle"
+ case (X, X, O) => "right"
+ case (O, X, X) => "left"
+ case _ => throw new IllegalArgumentException("Invalid")
+ }
+ }
+}

0 comments on commit 6271396

Please sign in to comment.