Skip to content

Commit 6daf9c6

Browse files
committed
Merge pull request scala#1509 from paulp/issue/6537
Fix for SI-6537, inaccurate unchecked warning.
2 parents 117bb2a + 6ff9db6 commit 6daf9c6

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)