Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make unchecked cases non-@unchecked and non-unreachable #16958

Merged
merged 1 commit into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ object TypeOps:
*
* Otherwise, return NoType.
*/
private def instantiateToSubType(tp1: NamedType, tp2: Type, mixins: List[Type])(using Context): Type = {
private def instantiateToSubType(tp1: NamedType, tp2: Type, mixins: List[Type])(using Context): Type = trace(i"instantiateToSubType($tp1, $tp2, $mixins)", typr) {
// In order for a child type S to qualify as a valid subtype of the parent
// T, we need to test whether it is possible S <: T.
//
Expand Down Expand Up @@ -854,6 +854,12 @@ object TypeOps:
case tp: TypeRef if tp.symbol.isAbstractOrParamType =>
gadtSyms += tp.symbol
traverseChildren(tp)
val owners = Iterator.iterate(tp.symbol)(_.maybeOwner).takeWhile(_.exists)
for sym <- owners do
// add ThisType's for the classes symbols in the ownership of `tp`
// for example, i16451.CanForward.scala, add `Namer.this`, as one of the owners of the type parameter `A1`
if sym.isClass && !sym.isAnonymousClass && !sym.isStaticOwner then
traverse(sym.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)
dwijnand marked this conversation as resolved.
Show resolved Hide resolved
.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
59 changes: 29 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 => _, * }
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,23 @@ 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
// when matching against a value,
// any type argument for an invariant type parameter will be unchecked,
// meaning it won't fail to match against anything; thus the wildcard replacement
WildcardType
else erase(arg, inArray = false, isValue = false)
dwijnand marked this conversation as resolved.
Show resolved Hide resolved
}
tp.derivedAppliedType(erase(tycon, inArray, isValue = false), args2)

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

case _ => tp
}
}
})

/** Space of the pattern: unapplySeq(a, b, c: _*)
*/
Expand Down Expand Up @@ -873,16 +880,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 +901,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 +914,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 +951,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
Loading