From 83093d0952fb03984580b089aaa319f986cd265b 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 --- .../dotty/tools/dotc/core/TypeComparer.scala | 16 +----- .../src/dotty/tools/dotc/core/TypeOps.scala | 4 ++ .../tools/dotc/transform/ExpandSAMs.scala | 16 +++--- .../tools/dotc/transform/patmat/Space.scala | 21 ++++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 4 +- tests/neg/i16451.check | 28 ++++++++++ tests/neg/i16451.scala | 51 +++++++++++++++++++ tests/patmat/t1056.check | 1 + tests/pos/i16451.CanForward.scala | 34 +++++++++++++ tests/pos/i16451.DiffUtil.scala | 15 ++++++ tests/pos/i16451.default.scala | 22 ++++++++ tests/run-macros/i7898.check | 2 +- 12 files changed, 187 insertions(+), 27 deletions(-) create mode 100644 tests/neg/i16451.check create mode 100644 tests/neg/i16451.scala create mode 100644 tests/patmat/t1056.check 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/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 9e8d18765352..563a6c733af0 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -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! @@ -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) => 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 f4c4863d073d..4758c50f2120 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -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))) @@ -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 @@ -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 @@ -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) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 5752409c51c1..f5b68f36c272 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -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 _ => diff --git a/tests/neg/i16451.check b/tests/neg/i16451.check new file mode 100644 index 000000000000..7500eb4d32ab --- /dev/null +++ b/tests/neg/i16451.check @@ -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] diff --git a/tests/neg/i16451.scala b/tests/neg/i16451.scala new file mode 100644 index 000000000000..53cd6f3dc61f --- /dev/null +++ b/tests/neg/i16451.scala @@ -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)) diff --git a/tests/patmat/t1056.check b/tests/patmat/t1056.check new file mode 100644 index 000000000000..4478141a7a95 --- /dev/null +++ b/tests/patmat/t1056.check @@ -0,0 +1 @@ +4: Pattern Match 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" }))