Skip to content

Commit 6ff9db6

Browse files
committed
Fix for SI-6537, inaccurate unchecked warning.
I found a more direct expression of the unchecked logic, which should be much easier for others to verify. But the bug being fixed here is that the unchecked checking happens too early, and the sealed children of a symbol are not yet visible if it is being simultaneously compiled.
1 parent 25ad787 commit 6ff9db6

File tree

5 files changed

+58
-37
lines changed

5 files changed

+58
-37
lines changed

src/compiler/scala/tools/nsc/typechecker/Checkable.scala

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -165,41 +165,43 @@ trait Checkable {
165165
/** X, P, [P1], etc. are all explained at the top of the file.
166166
*/
167167
private object CheckabilityChecker {
168-
/** A knowable class is one which is either effectively final
169-
* itself, or sealed with only knowable children.
170-
*/
171-
def isKnowable(sym: Symbol): Boolean = /*logResult(s"isKnowable($sym)")*/(
172-
sym.initialize.isEffectivelyFinal // pesky .initialize requirement, or we receive lies about isSealed
173-
|| sym.isSealed && (sym.children forall isKnowable)
168+
/** Are these symbols classes with no subclass relationship? */
169+
def areUnrelatedClasses(sym1: Symbol, sym2: Symbol) = (
170+
sym1.isClass
171+
&& sym2.isClass
172+
&& !(sym1 isSubClass sym2)
173+
&& !(sym2 isSubClass sym1)
174174
)
175-
def knownSubclasses(sym: Symbol): List[Symbol] = /*logResult(s"knownSubclasses($sym)")*/(sym :: {
176-
if (sym.isSealed) sym.children.toList flatMap knownSubclasses
177-
else Nil
178-
})
179-
def excludable(s1: Symbol, s2: Symbol) = /*logResult(s"excludable($s1, $s2)")*/(
180-
isKnowable(s1)
181-
&& !(s2 isSubClass s1)
182-
&& knownSubclasses(s1).forall(child => !(child isSubClass s2))
175+
/** Are all children of these symbols pairwise irreconcilable? */
176+
def allChildrenAreIrreconcilable(sym1: Symbol, sym2: Symbol) = (
177+
sym1.children.toList forall (c1 =>
178+
sym2.children.toList forall (c2 =>
179+
areIrreconcilableAsParents(c1, c2)
180+
)
181+
)
183182
)
184-
185-
/** Given classes A and B, can it be shown that nothing which is
186-
* an A will ever be a subclass of something which is a B? This
187-
* entails not only showing that !(A isSubClass B) but that the
188-
* same is true of all their subclasses. Restated for symmetry:
189-
* the same value cannot be a member of both A and B.
183+
/** Is it impossible for the given symbols to be parents in the same class?
184+
* This means given A and B, can there be an instance of A with B? This is the
185+
* case if neither A nor B is a subclass of the other, and one of the following
186+
* additional conditions holds:
187+
* - either A or B is effectively final
188+
* - neither A nor B is a trait (i.e. both are actual classes, not eligible for mixin)
189+
* - both A and B are sealed, and every possible pairing of their children is irreconcilable
190190
*
191-
* 1) A must not be a subclass of B, nor B of A (the trivial check)
192-
* 2) One of A or B must be completely knowable (see isKnowable)
193-
* 3) Assuming A is knowable, the proposition is true if
194-
* !(A' isSubClass B) for all A', where A' is a subclass of A.
195-
*
196-
* Due to symmetry, the last condition applies as well in reverse.
191+
* TODO: the last two conditions of the last possibility (that the symbols are not of
192+
* classes being compiled in the current run) are because this currently runs too early,
193+
* and .children returns Nil for sealed classes because their children will not be
194+
* populated until typer. It was too difficult to move things around for the moment,
195+
* so I will consult with moors about the optimal time to be doing this.
197196
*/
198-
def isNeverSubClass(sym1: Symbol, sym2: Symbol) = /*logResult(s"isNeverSubClass($sym1, $sym2)")*/(
199-
sym1.isClass
200-
&& sym2.isClass
201-
&& (excludable(sym1, sym2) || excludable(sym2, sym1))
197+
def areIrreconcilableAsParents(sym1: Symbol, sym2: Symbol): Boolean = areUnrelatedClasses(sym1, sym2) && (
198+
sym1.initialize.isEffectivelyFinal // initialization important
199+
|| sym2.initialize.isEffectivelyFinal
200+
|| !sym1.isTrait && !sym2.isTrait
201+
|| sym1.isSealed && sym2.isSealed && allChildrenAreIrreconcilable(sym1, sym2) && !currentRun.compiles(sym1) && !currentRun.compiles(sym2)
202202
)
203+
def isNeverSubClass(sym1: Symbol, sym2: Symbol) = areIrreconcilableAsParents(sym1, sym2)
204+
203205
private def isNeverSubArgs(tps1: List[Type], tps2: List[Type], tparams: List[Symbol]): Boolean = /*logResult(s"isNeverSubArgs($tps1, $tps2, $tparams)")*/ {
204206
def isNeverSubArg(t1: Type, t2: Type, variance: Int) = {
205207
if (variance > 0) isNeverSubType(t2, t1)
@@ -210,10 +212,7 @@ trait Checkable {
210212
}
211213
private def isNeverSameType(tp1: Type, tp2: Type): Boolean = (tp1, tp2) match {
212214
case (TypeRef(_, sym1, args1), TypeRef(_, sym2, args2)) =>
213-
( isNeverSubClass(sym1, sym2)
214-
|| isNeverSubClass(sym2, sym1)
215-
|| ((sym1 == sym2) && isNeverSubArgs(args1, args2, sym1.typeParams))
216-
)
215+
isNeverSubClass(sym1, sym2) || ((sym1 == sym2) && isNeverSubArgs(args1, args2, sym1.typeParams))
217216
case _ =>
218217
false
219218
}
Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
unchecked-knowable.scala:17: error: fruitless type test: a value of type Bippy cannot also be a A1
1+
unchecked-knowable.scala:18: error: fruitless type test: a value of type Bippy cannot also be a A1
22
/* warn */ (new Bippy).isInstanceOf[A1]
33
^
4-
one error found
4+
unchecked-knowable.scala:19: error: fruitless type test: a value of type Bippy cannot also be a B1
5+
/* warn */ (new Bippy).isInstanceOf[B1]
6+
^
7+
two errors found

test/files/neg/unchecked-knowable.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ final class A4 extends A2
77
/** Unknowable */
88
sealed abstract class B1
99
sealed abstract class B2 extends B1
10+
sealed trait B2B extends B1
1011
final class B3 extends B1
1112
trait B4 extends B2
1213

@@ -15,6 +16,7 @@ trait Dingus
1516

1617
class A {
1718
/* warn */ (new Bippy).isInstanceOf[A1]
18-
/* nowarn */ (new Bippy).isInstanceOf[B1]
19+
/* warn */ (new Bippy).isInstanceOf[B1]
20+
/* nowarn */ (null: Dingus).isInstanceOf[B1]
1921
/* nowarn */ ((new Bippy): Any).isInstanceOf[A1]
2022
}

test/files/pos/t6537.flags

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-Xfatal-warnings

test/files/pos/t6537.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package tester
2+
3+
object PatMatWarning {
4+
5+
sealed trait X
6+
sealed trait Y
7+
8+
def f(x: X) = x match {
9+
case _: Y => false
10+
case _ => true
11+
}
12+
13+
class X1 extends X
14+
class Y1 extends Y
15+
class Z1 extends X with Y
16+
}

0 commit comments

Comments
 (0)