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 23, 2023
1 parent a2da1a6 commit a1a8758
Show file tree
Hide file tree
Showing 15 changed files with 180 additions and 53 deletions.
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
55 changes: 25 additions & 30 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import TypeUtils._
import Contexts._
import Flags._
import ast._
import Decorators._
import Decorators.{ show as *, * }
import Symbols._
import StdNames._
import NameOps._
Expand All @@ -26,6 +26,8 @@ import util.{SrcPos, NoSourcePosition}
import scala.annotation.internal.sharable
import scala.collection.mutable

import SpaceEngine.*

/* Space logic for checking exhaustivity and unreachability of pattern matching
*
* Space can be thought of as a set of possible values. A type or a pattern
Expand Down Expand Up @@ -57,7 +59,6 @@ import scala.collection.mutable

/** space definition */
sealed trait Space:
import SpaceEngine.*

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

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

def canDecompose(using Context): Boolean = decompose != SpaceEngine.ListOfTypNoType
def canDecompose(using Context): Boolean = decompose != ListOfTypNoType

def decompose(using Context): List[Typ] =
val decompose = myDecompose
if decompose == null then
val decompose = tp match
case SpaceEngine.Parts(parts) => parts.map(Typ(_, decomposed = true))
case _ => SpaceEngine.ListOfTypNoType
case Parts(parts) => parts.map(Typ(_, decomposed = true))
case _ => ListOfTypNoType
myDecompose = decompose
decompose
else decompose
Expand Down Expand Up @@ -346,7 +347,7 @@ object SpaceEngine {
}

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

private def project(tp: Type)(using Context): Space = tp match {
case OrType(tp1, tp2) => Or(project(tp1) :: project(tp2) :: Nil)
Expand Down Expand Up @@ -461,16 +462,19 @@ object SpaceEngine {
* If `isValue` is true, then pattern-bound symbols are erased to its upper bound.
* This is needed to avoid spurious unreachable warnings. See tests/patmat/i6197.scala.
*/
private def erase(tp: Type, inArray: Boolean = false, isValue: Boolean = false)(using Context): Type = trace(i"$tp erased to", debug) {

tp match {
private def erase(tp: Type, inArray: Boolean = false, isValue: Boolean = false)(using Context): Type =
trace(i"erase($tp${if inArray then " inArray" else ""}${if isValue then " isValue" else ""})", debug)(tp match {
case tp @ AppliedType(tycon, args) if tycon.typeSymbol.isPatternBound =>
WildcardType

case tp @ AppliedType(tycon, args) =>
val args2 =
if (tycon.isRef(defn.ArrayClass)) args.map(arg => erase(arg, inArray = true, isValue = false))
else args.map(arg => erase(arg, inArray = false, isValue = false))
if tycon.isRef(defn.ArrayClass) then
args.map(arg => erase(arg, inArray = true, isValue = false))
else tycon.typeParams.lazyZip(args).map { (tparam, arg) =>
if isValue && tparam.paramVarianceSign == 0 then WildcardType
else erase(arg, inArray = false, isValue = false)
}
tp.derivedAppliedType(erase(tycon, inArray, isValue = false), args2)

case tp @ OrType(tp1, tp2) =>
Expand All @@ -488,8 +492,7 @@ object SpaceEngine {
else WildcardType

case _ => tp
}
}
})

/** Space of the pattern: unapplySeq(a, b, c: _*)
*/
Expand Down Expand Up @@ -873,16 +876,11 @@ object SpaceEngine {
case _ => tp
})

def checkExhaustivity(_match: Match)(using Context): Unit = {
val Match(sel, cases) = _match
debug.println(i"checking exhaustivity of ${_match}")

if (!exhaustivityCheckable(sel)) return

val selTyp = toUnderlying(sel.tpe).dealias
def checkExhaustivity(m: Match)(using Context): Unit = if exhaustivityCheckable(m.selector) then trace(i"checkExhaustivity($m)", debug) {
val selTyp = toUnderlying(m.selector.tpe).dealias
debug.println(i"selTyp = $selTyp")

val patternSpace = Or(cases.foldLeft(List.empty[Space]) { (acc, x) =>
val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) =>
val space = if (x.guard.isEmpty) project(x.pat) else Empty
debug.println(s"${x.pat.show} ====> ${show(space)}")
space :: acc
Expand All @@ -899,7 +897,7 @@ object SpaceEngine {
if uncovered.nonEmpty then
val hasMore = uncovered.lengthCompare(6) > 0
val deduped = dedup(uncovered.take(6))
report.warning(PatternMatchExhaustivity(showSpaces(deduped), hasMore), sel.srcPos)
report.warning(PatternMatchExhaustivity(showSpaces(deduped), hasMore), m.selector)
}

private def redundancyCheckable(sel: Tree)(using Context): Boolean =
Expand All @@ -912,14 +910,10 @@ object SpaceEngine {
&& !sel.tpe.widen.isRef(defn.QuotedExprClass)
&& !sel.tpe.widen.isRef(defn.QuotedTypeClass)

def checkRedundancy(_match: Match)(using Context): Unit = {
val Match(sel, _) = _match
val cases = _match.cases.toIndexedSeq
debug.println(i"checking redundancy in $_match")

if (!redundancyCheckable(sel)) return
def checkRedundancy(m: Match)(using Context): Unit = if redundancyCheckable(m.selector) then trace(i"checkRedundancy($m)", debug) {
val cases = m.cases.toIndexedSeq

val selTyp = toUnderlying(sel.tpe).dealias
val selTyp = toUnderlying(m.selector.tpe).dealias
debug.println(i"selTyp = $selTyp")

val isNullable = selTyp.classSymbol.isNullableClass
Expand Down Expand Up @@ -953,6 +947,7 @@ object SpaceEngine {
for (pat <- deferred.reverseIterator)
report.warning(MatchCaseUnreachable(), pat.srcPos)
if pat != EmptyTree // rethrow case of catch uses EmptyTree
&& !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
&& 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 @@ -1618,9 +1618,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
1 change: 0 additions & 1 deletion tests/neg-custom-args/fatal-warnings/i15662.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ case class Composite[T](v: T)
def m(composite: Composite[_]): Unit =
composite match {
case Composite[Int](v) => println(v) // error: cannot be checked at runtime
case _ => println("OTHER")
}

def m2(composite: Composite[_]): Unit =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@ object Test {
def err2[A, B](value: Foo[A, B], a: A => Int): B = value match {
case b: Bar[B] => // spurious // error
b.x
case _ => ??? // avoid fatal inexhaustivity warnings suppressing the uncheckable warning
}

def fail[A, B](value: Foo[A, B], a: A => Int): B = value match {
case b: Bar[Int] => // error
b.x
case _ => ??? // avoid fatal inexhaustivity warnings suppressing the uncheckable warning
}
}
4 changes: 1 addition & 3 deletions tests/neg-custom-args/isInstanceOf/enum-approx2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,5 @@ case class Fun[A, B](f: Exp[A => B]) extends Exp[A => B]
class Test {
def eval(e: Fun[Int, Int]) = e match {
case Fun(x: Fun[Int, Double]) => ??? // error
case Fun(x: Exp[Int => String]) => ??? // error
case _ =>
}
}
}
3 changes: 1 addition & 2 deletions tests/neg-custom-args/isInstanceOf/i11178.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ object Test1 {
def test[A](bar: Bar[A]) =
bar match {
case _: Bar[Boolean] => ??? // error
case _ => ???
}
}

Expand All @@ -36,4 +35,4 @@ object Test3 {
case _: Bar[Boolean] => ??? // error
case _ => ???
}
}
}
3 changes: 1 addition & 2 deletions tests/neg-custom-args/isInstanceOf/i8932.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ class Dummy extends Bar[Nothing] with Foo[String]
def bugReport[A](foo: Foo[A]): Foo[A] =
foo match {
case bar: Bar[A] => bar // error
case dummy: Dummy => ???
}

def test = bugReport(new Dummy: Foo[String])
def test = bugReport(new Dummy: Foo[String])
24 changes: 24 additions & 0 deletions tests/neg/i16451.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
-- Error: tests/neg/i16451.scala:13:9 ----------------------------------------------------------------------------------
13 | 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.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:25:9 ----------------------------------------------------------------------------------
25 | 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:29:9 ----------------------------------------------------------------------------------
29 | 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:34:11 ---------------------------------------------------------------------------------
34 | 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:39:11 ---------------------------------------------------------------------------------
39 | 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]
44 changes: 44 additions & 0 deletions tests/neg/i16451.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// 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 Test:
def test_correct(x: Wrapper[Color]): Option[Wrapper[Color.Red.type]] = x match
case x: Wrapper[Color.Red.type] => Some(x) // error
case null => None

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 null => 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 null => 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))
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 })
Loading

0 comments on commit a1a8758

Please sign in to comment.