From a1a8758e5fbf189b3737b6dbab0ea2a1be86989e Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 17 Feb 2023 11:46:53 +0000 Subject: [PATCH] Make unchecked cases non-`@unchecked` and non-unreachable --- .../src/dotty/tools/dotc/core/TypeOps.scala | 4 ++ .../tools/dotc/transform/ExpandSAMs.scala | 16 +++--- .../tools/dotc/transform/patmat/Space.scala | 55 +++++++++---------- .../src/dotty/tools/dotc/typer/Typer.scala | 4 +- .../fatal-warnings/i15662.scala | 1 - .../suppressed-type-test-warnings.scala | 2 - .../isInstanceOf/enum-approx2.scala | 4 +- .../neg-custom-args/isInstanceOf/i11178.scala | 3 +- .../neg-custom-args/isInstanceOf/i8932.scala | 3 +- tests/neg/i16451.check | 24 ++++++++ tests/neg/i16451.scala | 44 +++++++++++++++ tests/pos/i16451.CanForward.scala | 34 ++++++++++++ tests/pos/i16451.DiffUtil.scala | 15 +++++ tests/pos/i16451.default.scala | 22 ++++++++ tests/run-macros/i7898.check | 2 +- 15 files changed, 180 insertions(+), 53 deletions(-) create mode 100644 tests/neg/i16451.check create mode 100644 tests/neg/i16451.scala create mode 100644 tests/pos/i16451.CanForward.scala create mode 100644 tests/pos/i16451.DiffUtil.scala create mode 100644 tests/pos/i16451.default.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index c91412988e82..95224a09df31 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -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) } diff --git a/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala b/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala index 0552fe31f8a2..0bfc444e0997 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala @@ -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 } diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index d69371520413..1bd1b9513d2f 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -9,7 +9,7 @@ import TypeUtils._ import Contexts._ import Flags._ import ast._ -import Decorators._ +import Decorators.{ show as *, * } import Symbols._ import StdNames._ import NameOps._ @@ -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 @@ -57,7 +59,6 @@ import scala.collection.mutable /** space definition */ sealed trait Space: - import SpaceEngine.* @sharable private val isSubspaceCache = mutable.HashMap.empty[Space, Boolean] @@ -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 @@ -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) @@ -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) @@ -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) => @@ -488,8 +492,7 @@ object SpaceEngine { else WildcardType case _ => tp - } - } + }) /** Space of the pattern: unapplySeq(a, b, c: _*) */ @@ -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 @@ -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 = @@ -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 @@ -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) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9d8fdcc006c9..b39116f971f8 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -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 _ => diff --git a/tests/neg-custom-args/fatal-warnings/i15662.scala b/tests/neg-custom-args/fatal-warnings/i15662.scala index 1d5ff21eb3ba..afe505922603 100644 --- a/tests/neg-custom-args/fatal-warnings/i15662.scala +++ b/tests/neg-custom-args/fatal-warnings/i15662.scala @@ -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 = diff --git a/tests/neg-custom-args/fatal-warnings/suppressed-type-test-warnings.scala b/tests/neg-custom-args/fatal-warnings/suppressed-type-test-warnings.scala index 92d86b3307e5..175096fc6b21 100644 --- a/tests/neg-custom-args/fatal-warnings/suppressed-type-test-warnings.scala +++ b/tests/neg-custom-args/fatal-warnings/suppressed-type-test-warnings.scala @@ -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 } } diff --git a/tests/neg-custom-args/isInstanceOf/enum-approx2.scala b/tests/neg-custom-args/isInstanceOf/enum-approx2.scala index 516b765ec64b..5e3bdef7553d 100644 --- a/tests/neg-custom-args/isInstanceOf/enum-approx2.scala +++ b/tests/neg-custom-args/isInstanceOf/enum-approx2.scala @@ -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 _ => } -} \ No newline at end of file +} diff --git a/tests/neg-custom-args/isInstanceOf/i11178.scala b/tests/neg-custom-args/isInstanceOf/i11178.scala index 0d6867eba75f..71bc346e5743 100644 --- a/tests/neg-custom-args/isInstanceOf/i11178.scala +++ b/tests/neg-custom-args/isInstanceOf/i11178.scala @@ -12,7 +12,6 @@ object Test1 { def test[A](bar: Bar[A]) = bar match { case _: Bar[Boolean] => ??? // error - case _ => ??? } } @@ -36,4 +35,4 @@ object Test3 { case _: Bar[Boolean] => ??? // error case _ => ??? } -} \ No newline at end of file +} diff --git a/tests/neg-custom-args/isInstanceOf/i8932.scala b/tests/neg-custom-args/isInstanceOf/i8932.scala index f77c28c7b0a7..e070fdae518c 100644 --- a/tests/neg-custom-args/isInstanceOf/i8932.scala +++ b/tests/neg-custom-args/isInstanceOf/i8932.scala @@ -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]) \ No newline at end of file +def test = bugReport(new Dummy: Foo[String]) diff --git a/tests/neg/i16451.check b/tests/neg/i16451.check new file mode 100644 index 000000000000..e53085e8eafa --- /dev/null +++ b/tests/neg/i16451.check @@ -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] diff --git a/tests/neg/i16451.scala b/tests/neg/i16451.scala new file mode 100644 index 000000000000..685b79477bbe --- /dev/null +++ b/tests/neg/i16451.scala @@ -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)) diff --git a/tests/pos/i16451.CanForward.scala b/tests/pos/i16451.CanForward.scala new file mode 100644 index 000000000000..a09a26f22acc --- /dev/null +++ b/tests/pos/i16451.CanForward.scala @@ -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("") diff --git a/tests/pos/i16451.DiffUtil.scala b/tests/pos/i16451.DiffUtil.scala new file mode 100644 index 000000000000..3ade8bb73aa7 --- /dev/null +++ b/tests/pos/i16451.DiffUtil.scala @@ -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 diff --git a/tests/pos/i16451.default.scala b/tests/pos/i16451.default.scala new file mode 100644 index 000000000000..2751f4901b5f --- /dev/null +++ b/tests/pos/i16451.default.scala @@ -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 }) diff --git a/tests/run-macros/i7898.check b/tests/run-macros/i7898.check index 64da4a308f63..044c101dc772 100644 --- a/tests/run-macros/i7898.check +++ b/tests/run-macros/i7898.check @@ -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" }))