Merge pull request #2439 from retronym/ticket/7369

SI-7369 Avoid spurious unreachable warnings in patterns

6 files changed, 123 insertions, 11 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala
index 0fab48028e..dbe08315f4 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala
@@ -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))
diff --git a/test/files/neg/t7369.check b/test/files/neg/t7369.check
new file mode 100644
index 0000000000..4f101e145a
--- /dev/null
+++ b/test/files/neg/t7369.check
@@ -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
diff --git a/test/files/neg/t7369.flags b/test/files/neg/t7369.flags
new file mode 100644
index 0000000000..e8fb65d50c
--- /dev/null
+++ b/test/files/neg/t7369.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
\ No newline at end of file
diff --git a/test/files/neg/t7369.scala b/test/files/neg/t7369.scala
new file mode 100644
index 0000000000..87ddfe98b7
--- /dev/null
+++ b/test/files/neg/t7369.scala
@@ -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 _      =>
+  }
+}
diff --git a/test/files/pos/t7369.flags b/test/files/pos/t7369.flags
new file mode 100644
index 0000000000..85d8eb2ba2
--- /dev/null
+++ b/test/files/pos/t7369.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
diff --git a/test/files/pos/t7369.scala b/test/files/pos/t7369.scala
new file mode 100644
index 0000000000..2f31c93d29
--- /dev/null
+++ b/test/files/pos/t7369.scala
@@ -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")
+    }
+  }
+}
\ No newline at end of file