Skip to content

Commit

Permalink
Make unchecked cases non-@unchecked and non-unreachable
Browse files Browse the repository at this point in the history
  • Loading branch information
dwijnand committed Feb 20, 2023
1 parent 0416992 commit 83093d0
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 27 deletions.
16 changes: 2 additions & 14 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2660,7 +2660,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling

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

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

args1.lazyZip(args2).lazyZip(tycon1.typeParams).exists {
(arg1, arg2, tparam) =>
Expand Down
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,10 @@ object TypeOps:
case tp: TypeRef if tp.symbol.isAbstractOrParamType =>
gadtSyms += tp.symbol
traverseChildren(tp)
val owners = Iterator.iterate(tp.symbol)(_.maybeOwner).takeWhile(_.exists)
val classes = owners.filter(sym => sym.isClass && !sym.isAnonymousClass)
if classes.hasNext then
traverse(classes.next.thisType)
case _ =>
traverseChildren(tp)
}
Expand Down
16 changes: 7 additions & 9 deletions compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,13 @@ class ExpandSAMs extends MiniPhase:

def translateMatch(tree: Match, pfParam: Symbol, cases: List[CaseDef], defaultValue: Tree)(using Context) = {
val selector = tree.selector
val selectorTpe = selector.tpe.widen
val defaultSym = newSymbol(pfParam.owner, nme.WILDCARD, SyntheticCase, selectorTpe)
val defaultCase =
CaseDef(
Bind(defaultSym, Underscore(selectorTpe)),
EmptyTree,
defaultValue)
val unchecked = selector.annotated(New(ref(defn.UncheckedAnnot.typeRef)))
cpy.Match(tree)(unchecked, cases :+ defaultCase)
val cases1 = if cases.exists(isDefaultCase) then cases
else
val selectorTpe = selector.tpe.widen
val defaultSym = newSymbol(pfParam.owner, nme.WILDCARD, SyntheticCase, selectorTpe)
val defaultCase = CaseDef(Bind(defaultSym, Underscore(selectorTpe)), EmptyTree, defaultValue)
cases :+ defaultCase
cpy.Match(tree)(selector, cases1)
.subst(param.symbol :: Nil, pfParam :: Nil)
// Needed because a partial function can be written as:
// param => param match { case "foo" if foo(param) => param }
Expand Down
21 changes: 21 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,10 @@ class SpaceEngine(using Context) extends SpaceLogic {
// For instance, from i15029, `decompose((X | Y).Field[T]) = [X.Field[T], Y.Field[T]]`.
rec(tycon, Nil).map(typ => Typ(tp.derivedAppliedType(typ.tp, targs)))

case tp @ AppliedType(tycon, _) if tp.classSymbol.children.isEmpty && !canDecompose(tycon) && decomposableArgIdx(tp) >= 0 =>
val (init, targ :: tail) = tp.args.splitAt(decomposableArgIdx(tp)): @unchecked
decompose(targ).map(typ => Typ(tp.derivedAppliedType(tycon, init ::: typ.tp :: tail)))

case tp: NamedType if canDecompose(tp.prefix) =>
rec(tp.prefix, Nil).map(typ => Typ(tp.derivedSelect(typ.tp)))

Expand Down Expand Up @@ -708,6 +712,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
def canDecompose(tp: Type): Boolean =
val res = tp.dealias match
case AppliedType(tycon, _) if canDecompose(tycon) => true
case tp: AppliedType if decomposableArgIdx(tp) >= 0 => true
case tp: NamedType if canDecompose(tp.prefix) => true
case _: SingletonType => false
case _: OrType => true
Expand All @@ -724,6 +729,21 @@ class SpaceEngine(using Context) extends SpaceLogic {
//debug.println(s"decomposable: ${tp.show} = $res")
res

/** Returns the index of the type argument of `tp` that can be decomposed, if found, or `-1` if not. */
private def decomposableArgIdx(tp: AppliedType)(using Context): Int =
tp.tycon.typeParams.zip(tp.args).indexWhere { (tparam, targ) =>
if tparam.paramVarianceSign >= 0 then
val typeparamCorrespondsToField =
try comparing(_.typeparamCorrespondsToField(tp.tycon, tparam))
catch case _: RecursionOverflow => false // tests/pos/f-bounded-case-class.scala
if typeparamCorrespondsToField then
val cls = targ.classSymbol
targ.baseType(cls) != tp.baseType(cls) // tests/patmat/enum-HList.scala
&& canDecompose(targ)
else false
else false
}

/** Show friendly type name with current scope in mind
*
* E.g. C.this.B --> B if current owner is C
Expand Down Expand Up @@ -990,6 +1010,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
for (pat <- deferred.reverseIterator)
report.warning(MatchCaseUnreachable(), pat.srcPos)
if pat != EmptyTree // rethrow case of catch uses EmptyTree
&& !pat.symbol.isAllOf(SyntheticCase) // from ExpandSAMs collect
&& isSubspace(covered, prev)
then {
val nullOnly = isNullable && i == len - 1 && isWildcardArg(pat)
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1617,9 +1617,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
}
else {
val (protoFormals, _) = decomposeProtoFunction(pt, 1, tree.srcPos)
val checkMode =
if (pt.isRef(defn.PartialFunctionClass)) desugar.MatchCheck.None
else desugar.MatchCheck.Exhaustive
val checkMode = desugar.MatchCheck.Exhaustive
typed(desugar.makeCaseLambda(tree.cases, checkMode, protoFormals.length).withSpan(tree.span), pt)
}
case _ =>
Expand Down
28 changes: 28 additions & 0 deletions tests/neg/i16451.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-- Error: tests/neg/i16451.scala:20:9 ----------------------------------------------------------------------------------
20 | case x: Wrapper[Color.Red.type] => Some(x) // error
| ^
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
-- Error: tests/neg/i16451.scala:21:9 ----------------------------------------------------------------------------------
21 | case x: Wrapper[Color.Green.type] => None // error
| ^
|the type test for Wrapper[(Color.Green : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
-- Error: tests/neg/i16451.scala:28:9 ----------------------------------------------------------------------------------
28 | case x: Wrapper[Color.Red.type] => Some(x) // error
| ^
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Any
-- Error: tests/neg/i16451.scala:32:9 ----------------------------------------------------------------------------------
32 | case x: Wrapper[Color.Red.type] => Some(x) // error
| ^
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
-- Error: tests/neg/i16451.scala:36:9 ----------------------------------------------------------------------------------
36 | case x: Wrapper[Color.Red.type] => Some(x) // error
| ^
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from A1
-- Error: tests/neg/i16451.scala:41:11 ---------------------------------------------------------------------------------
41 | case x: Wrapper[Color.Red.type] => x // error
| ^
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
-- Error: tests/neg/i16451.scala:46:11 ---------------------------------------------------------------------------------
46 | case x: Wrapper[Color.Red.type] => x // error
| ^
|the type test for Wrapper[(Color.Red : Color)] cannot be checked at runtime because its type arguments can't be determined from Wrapper[Color]
51 changes: 51 additions & 0 deletions tests/neg/i16451.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// scalac: -Werror
enum Color:
case Red, Green
//sealed trait Color
//object Color:
// case object Red extends Color
// case object Green extends Color

case class Wrapper[A](value: A)
//object Wrapper:
//import scala.reflect.TypeTest
//given TypeTest[Wrapper[Color], Wrapper[Color.Red.type]] with
// def unapply(x: Wrapper[Color]): Option[x.type & Wrapper[Color.Red.type]] =
// if x.value == Color.Red then
// Some(x.asInstanceOf[x.type & Wrapper[Color.Red.type]])
// else None

object Test:
def test_correct(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match
case x: Wrapper[Color.Red.type] => Some(x) // error
case x: Wrapper[Color.Green.type] => None // error

def test_different(x: Wrapper[Color]): Option[Wrapper[Color]] = x match
case x @ Wrapper(_: Color.Red.type) => Some(x)
case x @ Wrapper(_: Color.Green.type) => None

def test_any(x: Any): Option[Wrapper[Color.Red.type]] = x match
case x: Wrapper[Color.Red.type] => Some(x) // error
case _ => None

def test_wrong(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match
case x: Wrapper[Color.Red.type] => Some(x) // error
case _ => None

def t2[A1 <: Wrapper[Color]](x: A1): Option[Wrapper[Color.Red.type]] = x match
case x: Wrapper[Color.Red.type] => Some(x) // error
case _ => None

def test_wrong_seq(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] =
xs.collect {
case x: Wrapper[Color.Red.type] => x // error
}

def test_wrong_seq2(xs: Seq[Wrapper[Color]]): Seq[Wrapper[Color.Red.type]] =
xs.collect { x => x match
case x: Wrapper[Color.Red.type] => x // error
}

def main(args: Array[String]): Unit =
println(test_wrong_seq(Seq(Wrapper(Color.Red), Wrapper(Color.Green))))
// outputs: List(Wrapper(Red), Wrapper(Green))
1 change: 1 addition & 0 deletions tests/patmat/t1056.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4: Pattern Match
34 changes: 34 additions & 0 deletions tests/pos/i16451.CanForward.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// scalac: -Werror
abstract class Namer:
private enum CanForward:
case Yes
case No(whyNot: String)
case Skip // for members that have never forwarders

class Mbr
private def canForward(mbr: Mbr): CanForward = CanForward.Yes

private def applyOrElse[A1 <: CanForward, B1 >: String](x: A1, default: A1 => B1): B1 = x match
case CanForward.No(whyNot @ _) => whyNot
case _ => ""

def addForwardersNamed(mbrs: List[Mbr]) =
val reason = mbrs.map(canForward).collect {
case CanForward.No(whyNot) => whyNot
}.headOption.getOrElse("")

class ClassCompleter:
def addForwardersNamed(mbrs: List[Mbr]) =
val reason = mbrs.map(canForward).collect {
case CanForward.No(whyNot) => whyNot
}.headOption.getOrElse("")

private def exportForwarders =
def addForwardersNamed(mbrs: List[Mbr]) =
val reason = mbrs.map(canForward).collect {
case CanForward.No(whyNot) => whyNot
}.headOption.getOrElse("")
if mbrs.size == 4 then
val reason = mbrs.map(canForward).collect {
case CanForward.No(whyNot) => whyNot
}.headOption.getOrElse("")
15 changes: 15 additions & 0 deletions tests/pos/i16451.DiffUtil.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// scalac: -Werror
object DiffUtil:
private sealed trait Patch
private final case class Unmodified(str: String) extends Patch
private final case class Modified(original: String, str: String) extends Patch
private final case class Deleted(str: String) extends Patch
private final case class Inserted(str: String) extends Patch

private def test(diff: Array[Patch]) =
diff.collect {
case Unmodified(str) => str
case Inserted(str) => s"+$str"
case Modified(orig, str) => s"{$orig,$str}"
case Deleted(str) => s"-$str"
}.mkString
22 changes: 22 additions & 0 deletions tests/pos/i16451.default.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// scalac: -Werror

import java.lang.reflect.*
import scala.annotation.tailrec

class Test:
@tailrec private def unwrapThrowable(x: Throwable): Throwable = x match {
case _: InvocationTargetException | // thrown by reflectively invoked method or constructor
_: ExceptionInInitializerError | // thrown when running a static initializer (e.g. a scala module constructor)
_: UndeclaredThrowableException | // invocation on a proxy instance if its invocation handler's `invoke` throws an exception
_: ClassNotFoundException | // no definition for a class instantiated by name
_: NoClassDefFoundError // the definition existed when the executing class was compiled, but can no longer be found
if x.getCause != null =>
unwrapThrowable(x.getCause)
case _ => x
}

private def unwrapHandler[T](pf: PartialFunction[Throwable, T]): PartialFunction[Throwable, T] =
pf.compose({ case ex => unwrapThrowable(ex) })

def test =
unwrapHandler({ case ex => throw ex })
2 changes: 1 addition & 1 deletion tests/run-macros/i7898.check
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
scala.List.apply[scala.PartialFunction[scala.Int, scala.Predef.String]](((x$1: scala.Int) => (x$1: @scala.unchecked) match {
scala.List.apply[scala.PartialFunction[scala.Int, scala.Predef.String]](((x$1: scala.Int) => x$1 match {
case 1 =>
"x"
}))

0 comments on commit 83093d0

Please sign in to comment.