Skip to content

Commit 9dcb217

Browse files
dwijnanddwijnand
authored
Make unchecked cases non-@unchecked and non-unreachable (#16958)
2 parents 0df5ae2 + 6b36cd9 commit 9dcb217

File tree

15 files changed

+187
-54
lines changed

15 files changed

+187
-54
lines changed

‎compiler/src/dotty/tools/dotc/core/TypeOps.scala

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ object TypeOps:
765765
*
766766
* Otherwise, return NoType.
767767
*/
768-
private def instantiateToSubType(tp1: NamedType, tp2: Type, mixins: List[Type])(using Context): Type = {
768+
private def instantiateToSubType(tp1: NamedType, tp2: Type, mixins: List[Type])(using Context): Type = trace(i"instantiateToSubType($tp1, $tp2, $mixins)", typr) {
769769
// In order for a child type S to qualify as a valid subtype of the parent
770770
// T, we need to test whether it is possible S <: T.
771771
//
@@ -854,6 +854,12 @@ object TypeOps:
854854
case tp: TypeRef if tp.symbol.isAbstractOrParamType =>
855855
gadtSyms += tp.symbol
856856
traverseChildren(tp)
857+
val owners = Iterator.iterate(tp.symbol)(_.maybeOwner).takeWhile(_.exists)
858+
for sym <- owners do
859+
// add ThisType's for the classes symbols in the ownership of `tp`
860+
// for example, i16451.CanForward.scala, add `Namer.this`, as one of the owners of the type parameter `A1`
861+
if sym.isClass && !sym.isAnonymousClass && !sym.isStaticOwner then
862+
traverse(sym.thisType)
857863
case _ =>
858864
traverseChildren(tp)
859865
}

‎compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,13 @@ class ExpandSAMs extends MiniPhase:
145145

146146
def translateMatch(tree: Match, pfParam: Symbol, cases: List[CaseDef], defaultValue: Tree)(using Context) = {
147147
val selector = tree.selector
148-
val selectorTpe = selector.tpe.widen
149-
val defaultSym = newSymbol(pfParam.owner, nme.WILDCARD, SyntheticCase, selectorTpe)
150-
val defaultCase =
151-
CaseDef(
152-
Bind(defaultSym, Underscore(selectorTpe)),
153-
EmptyTree,
154-
defaultValue)
155-
val unchecked = selector.annotated(New(ref(defn.UncheckedAnnot.typeRef)))
156-
cpy.Match(tree)(unchecked, cases :+ defaultCase)
148+
val cases1 = if cases.exists(isDefaultCase) then cases
149+
else
150+
val selectorTpe = selector.tpe.widen
151+
val defaultSym = newSymbol(pfParam.owner, nme.WILDCARD, SyntheticCase, selectorTpe)
152+
val defaultCase = CaseDef(Bind(defaultSym, Underscore(selectorTpe)), EmptyTree, defaultValue)
153+
cases :+ defaultCase
154+
cpy.Match(tree)(selector, cases1)
157155
.subst(param.symbol :: Nil, pfParam :: Nil)
158156
// Needed because a partial function can be written as:
159157
// param => param match { case "foo" if foo(param) => param }

‎compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import TypeUtils._
99
import Contexts._
1010
import Flags._
1111
import ast._
12-
import Decorators._
12+
import Decorators.{ show => _, * }
1313
import Symbols._
1414
import StdNames._
1515
import NameOps._
@@ -26,6 +26,8 @@ import util.{SrcPos, NoSourcePosition}
2626
import scala.annotation.internal.sharable
2727
import scala.collection.mutable
2828

29+
import SpaceEngine.*
30+
2931
/* Space logic for checking exhaustivity and unreachability of pattern matching
3032
*
3133
* Space can be thought of as a set of possible values. A type or a pattern
@@ -57,7 +59,6 @@ import scala.collection.mutable
5759

5860
/** space definition */
5961
sealed trait Space:
60-
import SpaceEngine.*
6162

6263
@sharable private val isSubspaceCache = mutable.HashMap.empty[Space, Boolean]
6364

@@ -95,14 +96,14 @@ case object Empty extends Space
9596
case class Typ(tp: Type, decomposed: Boolean = true) extends Space:
9697
private var myDecompose: List[Typ] | Null = null
9798

98-
def canDecompose(using Context): Boolean = decompose != SpaceEngine.ListOfTypNoType
99+
def canDecompose(using Context): Boolean = decompose != ListOfTypNoType
99100

100101
def decompose(using Context): List[Typ] =
101102
val decompose = myDecompose
102103
if decompose == null then
103104
val decompose = tp match
104-
case SpaceEngine.Parts(parts) => parts.map(Typ(_, decomposed = true))
105-
case _ => SpaceEngine.ListOfTypNoType
105+
case Parts(parts) => parts.map(Typ(_, decomposed = true))
106+
case _ => ListOfTypNoType
106107
myDecompose = decompose
107108
decompose
108109
else decompose
@@ -346,7 +347,7 @@ object SpaceEngine {
346347
}
347348

348349
/** Return the space that represents the pattern `pat` */
349-
def project(pat: Tree)(using Context): Space = pat match {
350+
def project(pat: Tree)(using Context): Space = trace(i"project($pat ${pat.className} ${pat.tpe})", debug, show)(pat match {
350351
case Literal(c) =>
351352
if (c.value.isInstanceOf[Symbol])
352353
Typ(c.value.asInstanceOf[Symbol].termRef, decomposed = false)
@@ -407,7 +408,7 @@ object SpaceEngine {
407408
case _ =>
408409
// Pattern is an arbitrary expression; assume a skolem (i.e. an unknown value) of the pattern type
409410
Typ(pat.tpe.narrow, decomposed = false)
410-
}
411+
})
411412

412413
private def project(tp: Type)(using Context): Space = tp match {
413414
case OrType(tp1, tp2) => Or(project(tp1) :: project(tp2) :: Nil)
@@ -461,16 +462,23 @@ object SpaceEngine {
461462
* If `isValue` is true, then pattern-bound symbols are erased to its upper bound.
462463
* This is needed to avoid spurious unreachable warnings. See tests/patmat/i6197.scala.
463464
*/
464-
private def erase(tp: Type, inArray: Boolean = false, isValue: Boolean = false)(using Context): Type = trace(i"$tp erased to", debug) {
465-
466-
tp match {
465+
private def erase(tp: Type, inArray: Boolean = false, isValue: Boolean = false)(using Context): Type =
466+
trace(i"erase($tp${if inArray then " inArray" else ""}${if isValue then " isValue" else ""})", debug)(tp match {
467467
case tp @ AppliedType(tycon, args) if tycon.typeSymbol.isPatternBound =>
468468
WildcardType
469469

470470
case tp @ AppliedType(tycon, args) =>
471471
val args2 =
472-
if (tycon.isRef(defn.ArrayClass)) args.map(arg => erase(arg, inArray = true, isValue = false))
473-
else args.map(arg => erase(arg, inArray = false, isValue = false))
472+
if tycon.isRef(defn.ArrayClass) then
473+
args.map(arg => erase(arg, inArray = true, isValue = false))
474+
else tycon.typeParams.lazyZip(args).map { (tparam, arg) =>
475+
if isValue && tparam.paramVarianceSign == 0 then
476+
// when matching against a value,
477+
// any type argument for an invariant type parameter will be unchecked,
478+
// meaning it won't fail to match against anything; thus the wildcard replacement
479+
WildcardType
480+
else erase(arg, inArray = false, isValue = false)
481+
}
474482
tp.derivedAppliedType(erase(tycon, inArray, isValue = false), args2)
475483

476484
case tp @ OrType(tp1, tp2) =>
@@ -488,8 +496,7 @@ object SpaceEngine {
488496
else WildcardType
489497

490498
case _ => tp
491-
}
492-
}
499+
})
493500

494501
/** Space of the pattern: unapplySeq(a, b, c: _*)
495502
*/
@@ -873,16 +880,11 @@ object SpaceEngine {
873880
case _ => tp
874881
})
875882

876-
def checkExhaustivity(_match: Match)(using Context): Unit = {
877-
val Match(sel, cases) = _match
878-
debug.println(i"checking exhaustivity of ${_match}")
879-
880-
if (!exhaustivityCheckable(sel)) return
881-
882-
val selTyp = toUnderlying(sel.tpe).dealias
883+
def checkExhaustivity(m: Match)(using Context): Unit = if exhaustivityCheckable(m.selector) then trace(i"checkExhaustivity($m)", debug) {
884+
val selTyp = toUnderlying(m.selector.tpe).dealias
883885
debug.println(i"selTyp = $selTyp")
884886

885-
val patternSpace = Or(cases.foldLeft(List.empty[Space]) { (acc, x) =>
887+
val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) =>
886888
val space = if (x.guard.isEmpty) project(x.pat) else Empty
887889
debug.println(s"${x.pat.show} ====> ${show(space)}")
888890
space :: acc
@@ -899,7 +901,7 @@ object SpaceEngine {
899901
if uncovered.nonEmpty then
900902
val hasMore = uncovered.lengthCompare(6) > 0
901903
val deduped = dedup(uncovered.take(6))
902-
report.warning(PatternMatchExhaustivity(showSpaces(deduped), hasMore), sel.srcPos)
904+
report.warning(PatternMatchExhaustivity(showSpaces(deduped), hasMore), m.selector)
903905
}
904906

905907
private def redundancyCheckable(sel: Tree)(using Context): Boolean =
@@ -912,14 +914,10 @@ object SpaceEngine {
912914
&& !sel.tpe.widen.isRef(defn.QuotedExprClass)
913915
&& !sel.tpe.widen.isRef(defn.QuotedTypeClass)
914916

915-
def checkRedundancy(_match: Match)(using Context): Unit = {
916-
val Match(sel, _) = _match
917-
val cases = _match.cases.toIndexedSeq
918-
debug.println(i"checking redundancy in $_match")
919-
920-
if (!redundancyCheckable(sel)) return
917+
def checkRedundancy(m: Match)(using Context): Unit = if redundancyCheckable(m.selector) then trace(i"checkRedundancy($m)", debug) {
918+
val cases = m.cases.toIndexedSeq
921919

922-
val selTyp = toUnderlying(sel.tpe).dealias
920+
val selTyp = toUnderlying(m.selector.tpe).dealias
923921
debug.println(i"selTyp = $selTyp")
924922

925923
val isNullable = selTyp.classSymbol.isNullableClass
@@ -953,6 +951,7 @@ object SpaceEngine {
953951
for (pat <- deferred.reverseIterator)
954952
report.warning(MatchCaseUnreachable(), pat.srcPos)
955953
if pat != EmptyTree // rethrow case of catch uses EmptyTree
954+
&& !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
956955
&& isSubspace(covered, prev)
957956
then {
958957
val nullOnly = isNullable && i == len - 1 && isWildcardArg(pat)

‎compiler/src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,9 +1618,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
16181618
}
16191619
else {
16201620
val (protoFormals, _) = decomposeProtoFunction(pt, 1, tree.srcPos)
1621-
val checkMode =
1622-
if (pt.isRef(defn.PartialFunctionClass)) desugar.MatchCheck.None
1623-
else desugar.MatchCheck.Exhaustive
1621+
val checkMode = desugar.MatchCheck.Exhaustive
16241622
typed(desugar.makeCaseLambda(tree.cases, checkMode, protoFormals.length).withSpan(tree.span), pt)
16251623
}
16261624
case _ =>

‎tests/neg-custom-args/fatal-warnings/i15662.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ case class Composite[T](v: T)
33
def m(composite: Composite[_]): Unit =
44
composite match {
55
case Composite[Int](v) => println(v) // error: cannot be checked at runtime
6-
case _ => println("OTHER")
76
}
87

98
def m2(composite: Composite[_]): Unit =

‎tests/neg-custom-args/fatal-warnings/suppressed-type-test-warnings.scala

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,10 @@ object Test {
1818
def err2[A, B](value: Foo[A, B], a: A => Int): B = value match {
1919
case b: Bar[B] => // spurious // error
2020
b.x
21-
case _ => ??? // avoid fatal inexhaustivity warnings suppressing the uncheckable warning
2221
}
2322

2423
def fail[A, B](value: Foo[A, B], a: A => Int): B = value match {
2524
case b: Bar[Int] => // error
2625
b.x
27-
case _ => ??? // avoid fatal inexhaustivity warnings suppressing the uncheckable warning
2826
}
2927
}

‎tests/neg-custom-args/isInstanceOf/enum-approx2.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,5 @@ case class Fun[A, B](f: Exp[A => B]) extends Exp[A => B]
44
class Test {
55
def eval(e: Fun[Int, Int]) = e match {
66
case Fun(x: Fun[Int, Double]) => ??? // error
7-
case Fun(x: Exp[Int => String]) => ??? // error
8-
case _ =>
97
}
10-
}
8+
}

‎tests/neg-custom-args/isInstanceOf/i11178.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ object Test1 {
1212
def test[A](bar: Bar[A]) =
1313
bar match {
1414
case _: Bar[Boolean] => ??? // error
15-
case _ => ???
1615
}
1716
}
1817

@@ -36,4 +35,4 @@ object Test3 {
3635
case _: Bar[Boolean] => ??? // error
3736
case _ => ???
3837
}
39-
}
38+
}

‎tests/neg-custom-args/isInstanceOf/i8932.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ class Dummy extends Bar[Nothing] with Foo[String]
66
def bugReport[A](foo: Foo[A]): Foo[A] =
77
foo match {
88
case bar: Bar[A] => bar // error
9-
case dummy: Dummy => ???
109
}
1110

12-
def test = bugReport(new Dummy: Foo[String])
11+
def test = bugReport(new Dummy: Foo[String])

‎tests/neg/i16451.check

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-- Error: tests/neg/i16451.scala:13:9 ----------------------------------------------------------------------------------
2+
13 | case x: Wrapper[Color.Red.type] => Some(x) // error
3+
| ^
4+
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
5+
-- Error: tests/neg/i16451.scala:21:9 ----------------------------------------------------------------------------------
6+
21 | case x: Wrapper[Color.Red.type] => Some(x) // error
7+
| ^
8+
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Any
9+
-- Error: tests/neg/i16451.scala:25:9 ----------------------------------------------------------------------------------
10+
25 | case x: Wrapper[Color.Red.type] => Some(x) // error
11+
| ^
12+
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
13+
-- Error: tests/neg/i16451.scala:29:9 ----------------------------------------------------------------------------------
14+
29 | case x: Wrapper[Color.Red.type] => Some(x) // error
15+
| ^
16+
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from A1
17+
-- Error: tests/neg/i16451.scala:34:11 ---------------------------------------------------------------------------------
18+
34 | case x: Wrapper[Color.Red.type] => x // error
19+
| ^
20+
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
21+
-- Error: tests/neg/i16451.scala:39:11 ---------------------------------------------------------------------------------
22+
39 | case x: Wrapper[Color.Red.type] => x // error
23+
| ^
24+
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]

‎tests/neg/i16451.scala

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// scalac: -Werror
2+
enum Color:
3+
case Red, Green
4+
//sealed trait Color
5+
//object Color:
6+
// case object Red extends Color
7+
// case object Green extends Color
8+
9+
case class Wrapper[A](value: A)
10+
11+
object Test:
12+
def test_correct(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match
13+
case x: Wrapper[Color.Red.type] => Some(x) // error
14+
case null => None
15+
16+
def test_different(x: Wrapper[Color]): Option[Wrapper[Color]] = x match
17+
case x @ Wrapper(_: Color.Red.type) => Some(x)
18+
case x @ Wrapper(_: Color.Green.type) => None
19+
20+
def test_any(x: Any): Option[Wrapper[Color.Red.type]] = x match
21+
case x: Wrapper[Color.Red.type] => Some(x) // error
22+
case _ => None
23+
24+
def test_wrong(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match
25+
case x: Wrapper[Color.Red.type] => Some(x) // error
26+
case null => None
27+
28+
def t2[A1 <: Wrapper[Color]](x: A1): Option[Wrapper[Color.Red.type]] = x match
29+
case x: Wrapper[Color.Red.type] => Some(x) // error
30+
case null => None
31+
32+
def test_wrong_seq(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] =
33+
xs.collect {
34+
case x: Wrapper[Color.Red.type] => x // error
35+
}
36+
37+
def test_wrong_seq2(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] =
38+
xs.collect { x => x match
39+
case x: Wrapper[Color.Red.type] => x // error
40+
}
41+
42+
def main(args: Array[String]): Unit =
43+
println(test_wrong_seq(Seq(Wrapper(Color.Red), Wrapper(Color.Green))))
44+
// outputs: List(Wrapper(Red), Wrapper(Green))

‎tests/pos/i16451.CanForward.scala

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// scalac: -Werror
2+
abstract class Namer:
3+
private enum CanForward:
4+
case Yes
5+
case No(whyNot: String)
6+
case Skip // for members that have never forwarders
7+
8+
class Mbr
9+
private def canForward(mbr: Mbr): CanForward = CanForward.Yes
10+
11+
private def applyOrElse[A1 <: CanForward, B1 >: String](x: A1, default: A1 => B1): B1 = x match
12+
case CanForward.No(whyNot @ _) => whyNot
13+
case _ => ""
14+
15+
def addForwardersNamed(mbrs: List[Mbr]) =
16+
val reason = mbrs.map(canForward).collect {
17+
case CanForward.No(whyNot) => whyNot
18+
}.headOption.getOrElse("")
19+
20+
class ClassCompleter:
21+
def addForwardersNamed(mbrs: List[Mbr]) =
22+
val reason = mbrs.map(canForward).collect {
23+
case CanForward.No(whyNot) => whyNot
24+
}.headOption.getOrElse("")
25+
26+
private def exportForwarders =
27+
def addForwardersNamed(mbrs: List[Mbr]) =
28+
val reason = mbrs.map(canForward).collect {
29+
case CanForward.No(whyNot) => whyNot
30+
}.headOption.getOrElse("")
31+
if mbrs.size == 4 then
32+
val reason = mbrs.map(canForward).collect {
33+
case CanForward.No(whyNot) => whyNot
34+
}.headOption.getOrElse("")

‎tests/pos/i16451.DiffUtil.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// scalac: -Werror
2+
object DiffUtil:
3+
private sealed trait Patch
4+
private final case class Unmodified(str: String) extends Patch
5+
private final case class Modified(original: String, str: String) extends Patch
6+
private final case class Deleted(str: String) extends Patch
7+
private final case class Inserted(str: String) extends Patch
8+
9+
private def test(diff: Array[Patch]) =
10+
diff.collect {
11+
case Unmodified(str) => str
12+
case Inserted(str) => s"+$str"
13+
case Modified(orig, str) => s"{$orig,$str}"
14+
case Deleted(str) => s"-$str"
15+
}.mkString

0 commit comments

Comments
 (0)