Skip to content

Commit 83093d0

Browse files
dwijnanddwijnand
committed
Make unchecked cases non-@unchecked and non-unreachable
1 parent 0416992 commit 83093d0

File tree

12 files changed

+187
-27
lines changed

12 files changed

+187
-27
lines changed

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
26602660

26612661
/** Does `tycon` have a field with type `tparam`? Special cased for `scala.*:`
26622662
* as that type is artificially added to tuples. */
2663-
private def typeparamCorrespondsToField(tycon: Type, tparam: TypeParamInfo): Boolean =
2663+
def typeparamCorrespondsToField(tycon: Type, tparam: TypeParamInfo): Boolean =
26642664
productSelectorTypes(tycon, NoSourcePosition).exists {
26652665
case tp: TypeRef =>
26662666
tp.designator.eq(tparam) // Bingo!
@@ -2777,20 +2777,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
27772777
def covariantDisjoint(tp1: Type, tp2: Type, tparam: TypeParamInfo): Boolean =
27782778
provablyDisjoint(tp1, tp2) && typeparamCorrespondsToField(tycon1, tparam)
27792779

2780-
// In the invariant case, we also use a stronger notion of disjointness:
2781-
// we consider fully instantiated types not equal wrt =:= to be disjoint
2782-
// (under any context). This is fine because it matches the runtime
2783-
// semantics of pattern matching. To implement a pattern such as
2784-
// `case Inv[T] => ...`, one needs a type tag for `T` and the compiler
2785-
// is used at runtime to check it the scrutinee's type is =:= to `T`.
2786-
// Note that this is currently a theoretical concern since Dotty
2787-
// doesn't have type tags, meaning that users cannot write patterns
2788-
// that do type tests on higher kinded types.
27892780
def invariantDisjoint(tp1: Type, tp2: Type, tparam: TypeParamInfo): Boolean =
2790-
provablyDisjoint(tp1, tp2) ||
2791-
!isSameType(tp1, tp2) &&
2792-
fullyInstantiated(tp1) && // We can only trust a "no" from `isSameType` when
2793-
fullyInstantiated(tp2) // both `tp1` and `tp2` are fully instantiated.
2781+
provablyDisjoint(tp1, tp2)
27942782

27952783
args1.lazyZip(args2).lazyZip(tycon1.typeParams).exists {
27962784
(arg1, arg2, tparam) =>

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,10 @@ 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+
val classes = owners.filter(sym => sym.isClass && !sym.isAnonymousClass)
859+
if classes.hasNext then
860+
traverse(classes.next.thisType)
857861
case _ =>
858862
traverseChildren(tp)
859863
}

‎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: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,10 @@ class SpaceEngine(using Context) extends SpaceLogic {
663663
// For instance, from i15029, `decompose((X | Y).Field[T]) = [X.Field[T], Y.Field[T]]`.
664664
rec(tycon, Nil).map(typ => Typ(tp.derivedAppliedType(typ.tp, targs)))
665665

666+
case tp @ AppliedType(tycon, _) if tp.classSymbol.children.isEmpty && !canDecompose(tycon) && decomposableArgIdx(tp) >= 0 =>
667+
val (init, targ :: tail) = tp.args.splitAt(decomposableArgIdx(tp)): @unchecked
668+
decompose(targ).map(typ => Typ(tp.derivedAppliedType(tycon, init ::: typ.tp :: tail)))
669+
666670
case tp: NamedType if canDecompose(tp.prefix) =>
667671
rec(tp.prefix, Nil).map(typ => Typ(tp.derivedSelect(typ.tp)))
668672

@@ -708,6 +712,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
708712
def canDecompose(tp: Type): Boolean =
709713
val res = tp.dealias match
710714
case AppliedType(tycon, _) if canDecompose(tycon) => true
715+
case tp: AppliedType if decomposableArgIdx(tp) >= 0 => true
711716
case tp: NamedType if canDecompose(tp.prefix) => true
712717
case _: SingletonType => false
713718
case _: OrType => true
@@ -724,6 +729,21 @@ class SpaceEngine(using Context) extends SpaceLogic {
724729
//debug.println(s"decomposable: ${tp.show} = $res")
725730
res
726731

732+
/** Returns the index of the type argument of `tp` that can be decomposed, if found, or `-1` if not. */
733+
private def decomposableArgIdx(tp: AppliedType)(using Context): Int =
734+
tp.tycon.typeParams.zip(tp.args).indexWhere { (tparam, targ) =>
735+
if tparam.paramVarianceSign >= 0 then
736+
val typeparamCorrespondsToField =
737+
try comparing(_.typeparamCorrespondsToField(tp.tycon, tparam))
738+
catch case _: RecursionOverflow => false // tests/pos/f-bounded-case-class.scala
739+
if typeparamCorrespondsToField then
740+
val cls = targ.classSymbol
741+
targ.baseType(cls) != tp.baseType(cls) // tests/patmat/enum-HList.scala
742+
&& canDecompose(targ)
743+
else false
744+
else false
745+
}
746+
727747
/** Show friendly type name with current scope in mind
728748
*
729749
* E.g. C.this.B --> B if current owner is C
@@ -990,6 +1010,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
9901010
for (pat <- deferred.reverseIterator)
9911011
report.warning(MatchCaseUnreachable(), pat.srcPos)
9921012
if pat != EmptyTree // rethrow case of catch uses EmptyTree
1013+
&& !pat.symbol.isAllOf(SyntheticCase) // from ExpandSAMs collect
9931014
&& isSubspace(covered, prev)
9941015
then {
9951016
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
@@ -1617,9 +1617,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
16171617
}
16181618
else {
16191619
val (protoFormals, _) = decomposeProtoFunction(pt, 1, tree.srcPos)
1620-
val checkMode =
1621-
if (pt.isRef(defn.PartialFunctionClass)) desugar.MatchCheck.None
1622-
else desugar.MatchCheck.Exhaustive
1620+
val checkMode = desugar.MatchCheck.Exhaustive
16231621
typed(desugar.makeCaseLambda(tree.cases, checkMode, protoFormals.length).withSpan(tree.span), pt)
16241622
}
16251623
case _ =>

‎tests/neg/i16451.check‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-- Error: tests/neg/i16451.scala:20:9 ----------------------------------------------------------------------------------
2+
20 | 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.Green.type] => None // error
7+
| ^
8+
|the type test for Wrapper[(Color.Green : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
9+
-- Error: tests/neg/i16451.scala:28:9 ----------------------------------------------------------------------------------
10+
28 | 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 Any
13+
-- Error: tests/neg/i16451.scala:32:9 ----------------------------------------------------------------------------------
14+
32 | 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 Wrapper[Color]
17+
-- Error: tests/neg/i16451.scala:36:9 ----------------------------------------------------------------------------------
18+
36 | case x: Wrapper[Color.Red.type] => Some(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 A1
21+
-- Error: tests/neg/i16451.scala:41:11 ---------------------------------------------------------------------------------
22+
41 | 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]
25+
-- Error: tests/neg/i16451.scala:46:11 ---------------------------------------------------------------------------------
26+
46 | case x: Wrapper[Color.Red.type] => x // error
27+
| ^
28+
|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: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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+
//object Wrapper:
11+
//import scala.reflect.TypeTest
12+
//given TypeTest[Wrapper[Color], Wrapper[Color.Red.type]] with
13+
// def unapply(x: Wrapper[Color]): Option[x.type & Wrapper[Color.Red.type]] =
14+
// if x.value == Color.Red then
15+
// Some(x.asInstanceOf[x.type & Wrapper[Color.Red.type]])
16+
// else None
17+
18+
object Test:
19+
def test_correct(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match
20+
case x: Wrapper[Color.Red.type] => Some(x) // error
21+
case x: Wrapper[Color.Green.type] => None // error
22+
23+
def test_different(x: Wrapper[Color]): Option[Wrapper[Color]] = x match
24+
case x @ Wrapper(_: Color.Red.type) => Some(x)
25+
case x @ Wrapper(_: Color.Green.type) => None
26+
27+
def test_any(x: Any): Option[Wrapper[Color.Red.type]] = x match
28+
case x: Wrapper[Color.Red.type] => Some(x) // error
29+
case _ => None
30+
31+
def test_wrong(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match
32+
case x: Wrapper[Color.Red.type] => Some(x) // error
33+
case _ => None
34+
35+
def t2[A1 <: Wrapper[Color]](x: A1): Option[Wrapper[Color.Red.type]] = x match
36+
case x: Wrapper[Color.Red.type] => Some(x) // error
37+
case _ => None
38+
39+
def test_wrong_seq(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] =
40+
xs.collect {
41+
case x: Wrapper[Color.Red.type] => x // error
42+
}
43+
44+
def test_wrong_seq2(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] =
45+
xs.collect { x => x match
46+
case x: Wrapper[Color.Red.type] => x // error
47+
}
48+
49+
def main(args: Array[String]): Unit =
50+
println(test_wrong_seq(Seq(Wrapper(Color.Red), Wrapper(Color.Green))))
51+
// outputs: List(Wrapper(Red), Wrapper(Green))

‎tests/patmat/t1056.check‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
4: Pattern Match

‎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)