Merge pull request #1509 from paulp/issue/6537

Fix for SI-6537, inaccurate unchecked warning.

5 files changed, 58 insertions, 37 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
index 7e15cf91a7..ec097a9b08 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Checkable.scala
@@ -165,41 +165,43 @@ trait Checkable {
   /** X, P, [P1], etc. are all explained at the top of the file.
    */
   private object CheckabilityChecker {
-    /** A knowable class is one which is either effectively final
-     *  itself, or sealed with only knowable children.
-     */
-    def isKnowable(sym: Symbol): Boolean = /*logResult(s"isKnowable($sym)")*/(
-         sym.initialize.isEffectivelyFinal  // pesky .initialize requirement, or we receive lies about isSealed
-      || sym.isSealed && (sym.children forall isKnowable)
+    /** Are these symbols classes with no subclass relationship? */
+    def areUnrelatedClasses(sym1: Symbol, sym2: Symbol) = (
+         sym1.isClass
+      && sym2.isClass
+      && !(sym1 isSubClass sym2)
+      && !(sym2 isSubClass sym1)
     )
-    def knownSubclasses(sym: Symbol): List[Symbol] = /*logResult(s"knownSubclasses($sym)")*/(sym :: {
-      if (sym.isSealed) sym.children.toList flatMap knownSubclasses
-      else Nil
-    })
-    def excludable(s1: Symbol, s2: Symbol) = /*logResult(s"excludable($s1, $s2)")*/(
-         isKnowable(s1)
-      && !(s2 isSubClass s1)
-      && knownSubclasses(s1).forall(child => !(child isSubClass s2))
+    /** Are all children of these symbols pairwise irreconcilable? */
+    def allChildrenAreIrreconcilable(sym1: Symbol, sym2: Symbol) = (
+      sym1.children.toList forall (c1 =>
+        sym2.children.toList forall (c2 =>
+          areIrreconcilableAsParents(c1, c2)
+        )
+      )
     )
-
-    /** Given classes A and B, can it be shown that nothing which is
-     *  an A will ever be a subclass of something which is a B? This
-     *  entails not only showing that !(A isSubClass B) but that the
-     *  same is true of all their subclasses.  Restated for symmetry:
-     *  the same value cannot be a member of both A and B.
+    /** Is it impossible for the given symbols to be parents in the same class?
+     *  This means given A and B, can there be an instance of A with B? This is the
+     *  case if neither A nor B is a subclass of the other, and one of the following
+     *  additional conditions holds:
+     *   - either A or B is effectively final
+     *   - neither A nor B is a trait (i.e. both are actual classes, not eligible for mixin)
+     *   - both A and B are sealed, and every possible pairing of their children is irreconcilable
      *
-     *   1) A must not be a subclass of B, nor B of A (the trivial check)
-     *   2) One of A or B must be completely knowable (see isKnowable)
-     *   3) Assuming A is knowable, the proposition is true if
-     *      !(A' isSubClass B) for all A', where A' is a subclass of A.
-     *
-     *  Due to symmetry, the last condition applies as well in reverse.
+     *  TODO: the last two conditions of the last possibility (that the symbols are not of
+     *  classes being compiled in the current run) are because this currently runs too early,
+     *  and .children returns Nil for sealed classes because their children will not be
+     *  populated until typer.  It was too difficult to move things around for the moment,
+     *  so I will consult with moors about the optimal time to be doing this.
      */
-    def isNeverSubClass(sym1: Symbol, sym2: Symbol) = /*logResult(s"isNeverSubClass($sym1, $sym2)")*/(
-         sym1.isClass
-      && sym2.isClass
-      && (excludable(sym1, sym2) || excludable(sym2, sym1))
+    def areIrreconcilableAsParents(sym1: Symbol, sym2: Symbol): Boolean = areUnrelatedClasses(sym1, sym2) && (
+         sym1.initialize.isEffectivelyFinal // initialization important
+      || sym2.initialize.isEffectivelyFinal
+      || !sym1.isTrait && !sym2.isTrait
+      || sym1.isSealed && sym2.isSealed && allChildrenAreIrreconcilable(sym1, sym2) && !currentRun.compiles(sym1) && !currentRun.compiles(sym2)
     )
+    def isNeverSubClass(sym1: Symbol, sym2: Symbol) = areIrreconcilableAsParents(sym1, sym2)
+
     private def isNeverSubArgs(tps1: List[Type], tps2: List[Type], tparams: List[Symbol]): Boolean = /*logResult(s"isNeverSubArgs($tps1, $tps2, $tparams)")*/ {
       def isNeverSubArg(t1: Type, t2: Type, variance: Int) = {
         if (variance > 0) isNeverSubType(t2, t1)
@@ -210,10 +212,7 @@ trait Checkable {
     }
     private def isNeverSameType(tp1: Type, tp2: Type): Boolean = (tp1, tp2) match {
       case (TypeRef(_, sym1, args1), TypeRef(_, sym2, args2)) =>
-        (    isNeverSubClass(sym1, sym2)
-          || isNeverSubClass(sym2, sym1)
-          || ((sym1 == sym2) && isNeverSubArgs(args1, args2, sym1.typeParams))
-        )
+        isNeverSubClass(sym1, sym2) || ((sym1 == sym2) && isNeverSubArgs(args1, args2, sym1.typeParams))
       case _ =>
         false
     }
diff --git a/test/files/neg/unchecked-knowable.check b/test/files/neg/unchecked-knowable.check
index 3a6ef994b5..d279427327 100644
--- a/test/files/neg/unchecked-knowable.check
+++ b/test/files/neg/unchecked-knowable.check
@@ -1,4 +1,7 @@
-unchecked-knowable.scala:17: error: fruitless type test: a value of type Bippy cannot also be a A1
+unchecked-knowable.scala:18: error: fruitless type test: a value of type Bippy cannot also be a A1
   /*   warn */ (new Bippy).isInstanceOf[A1]
                                        ^
-one error found
+unchecked-knowable.scala:19: error: fruitless type test: a value of type Bippy cannot also be a B1
+  /*   warn */ (new Bippy).isInstanceOf[B1]
+                                       ^
+two errors found
diff --git a/test/files/neg/unchecked-knowable.scala b/test/files/neg/unchecked-knowable.scala
index 667b47f504..21624c4fb4 100644
--- a/test/files/neg/unchecked-knowable.scala
+++ b/test/files/neg/unchecked-knowable.scala
@@ -7,6 +7,7 @@ final class A4 extends A2
 /** Unknowable */
 sealed abstract class B1
 sealed abstract class B2 extends B1
+sealed trait B2B extends B1
 final class B3 extends B1
 trait B4 extends B2
 
@@ -15,6 +16,7 @@ trait Dingus
 
 class A {
   /*   warn */ (new Bippy).isInstanceOf[A1]
-  /* nowarn */ (new Bippy).isInstanceOf[B1]
+  /*   warn */ (new Bippy).isInstanceOf[B1]
+  /* nowarn */ (null: Dingus).isInstanceOf[B1]
   /* nowarn */ ((new Bippy): Any).isInstanceOf[A1]
 }
diff --git a/test/files/pos/t6537.flags b/test/files/pos/t6537.flags
new file mode 100644
index 0000000000..85d8eb2ba2
--- /dev/null
+++ b/test/files/pos/t6537.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
diff --git a/test/files/pos/t6537.scala b/test/files/pos/t6537.scala
new file mode 100644
index 0000000000..d0ca3ba435
--- /dev/null
+++ b/test/files/pos/t6537.scala
@@ -0,0 +1,16 @@
+package tester
+
+object PatMatWarning {
+
+  sealed trait X
+  sealed trait Y
+
+  def f(x: X) = x match {
+    case _: Y => false
+    case _    => true
+  }
+
+  class X1 extends X
+  class Y1 extends Y
+  class Z1 extends X with Y
+}