diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index af02bfed8411..075dad5b3acf 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -165,7 +165,7 @@ trait SpaceLogic { } /** Is `a` a subspace of `b`? Equivalent to `a - b == Empty`, but faster */ - def isSubspace(a: Space, b: Space)(using Context): Boolean = trace(s"${show(a)} < ${show(b)}", debug) { + def isSubspace(a: Space, b: Space)(using Context): Boolean = trace(s"isSubspace(${show(a)}, ${show(b)})", debug) { def tryDecompose1(tp: Type) = canDecompose(tp) && isSubspace(Or(decompose(tp)), b) def tryDecompose2(tp: Type) = canDecompose(tp) && isSubspace(a, Or(decompose(tp))) @@ -212,14 +212,14 @@ trait SpaceLogic { if (isSubType(tp2, tp1)) b else if (canDecompose(tp1)) tryDecompose1(tp1) else if (isSubType(tp1, tp2)) a // problematic corner case: inheriting a case class - else Empty + else intersectUnrelatedAtomicTypes(tp1, tp2) case (Prod(tp1, fun, ss), Typ(tp2, _)) => if (isSubType(tp1, tp2)) a else if (canDecompose(tp2)) tryDecompose2(tp2) else if (isSubType(tp2, tp1)) a // problematic corner case: inheriting a case class - else Empty + else intersectUnrelatedAtomicTypes(tp1, tp2) case (Prod(tp1, fun1, ss1), Prod(tp2, fun2, ss2)) => - if (!isSameUnapply(fun1, fun2)) Empty + if (!isSameUnapply(fun1, fun2)) intersectUnrelatedAtomicTypes(tp1, tp2) else if (ss1.zip(ss2).exists(p => simplify(intersect(p._1, p._2)) == Empty)) Empty else Prod(tp1, fun1, ss1.zip(ss2).map((intersect _).tupled)) } @@ -503,14 +503,34 @@ class SpaceEngine(using Context) extends SpaceLogic { } } + /** Numeric literals, while being constant values of unrelated types (e.g. Char and Int), + * when used in a case may end up matching at runtime, because their equals may returns true. + * Because these are universally available, general purpose types, it would be good to avoid + * returning false positive warnings, such as in `(c: Char) match { case 67 => ... }` emitting a + * reachability warning on the case. So the type `ConstantType(Constant(67, IntTag))` is + * converted to `ConstantType(Constant(67, CharTag))`. #12805 */ + def convertConstantType(tp: Type, pt: Type): Type = tp match + case tp @ ConstantType(const) => + val converted = const.convertTo(pt) + if converted == null then tp else ConstantType(converted) + case _ => tp + + /** Adapt types by performing primitive value boxing. #12805 */ + def maybeBox(tp1: Type, tp2: Type): Type = + if tp1.classSymbol.isPrimitiveValueClass && !tp2.classSymbol.isPrimitiveValueClass then + defn.boxedType(tp1).narrow + else tp1 + /** Is `tp1` a subtype of `tp2`? */ - def isSubType(tp1: Type, tp2: Type): Boolean = { - debug.println(TypeComparer.explained(_.isSubType(tp1, tp2))) + def isSubType(_tp1: Type, tp2: Type): Boolean = { + val tp1 = maybeBox(convertConstantType(_tp1, tp2), tp2) + //debug.println(TypeComparer.explained(_.isSubType(tp1, tp2))) val res = if (ctx.explicitNulls) { tp1 <:< tp2 } else { (tp1 != constantNullType || tp2 == constantNullType) && tp1 <:< tp2 } + debug.println(i"$tp1 <:< $tp2 = $res") res } @@ -650,7 +670,6 @@ class SpaceEngine(using Context) extends SpaceLogic { parts.map(Typ(_, true)) } - /** Abstract sealed types, or-types, Boolean and Java enums can be decomposed */ def canDecompose(tp: Type): Boolean = val res = tp.dealias match @@ -666,7 +685,7 @@ class SpaceEngine(using Context) extends SpaceLogic { || cls.isAllOf(JavaEnumTrait) || tp.isRef(defn.BooleanClass) || tp.isRef(defn.UnitClass) - debug.println(s"decomposable: ${tp.show} = $res") + //debug.println(s"decomposable: ${tp.show} = $res") res /** Show friendly type name with current scope in mind @@ -750,6 +769,7 @@ class SpaceEngine(using Context) extends SpaceLogic { } def show(ss: Seq[Space]): String = ss.map(show).mkString(", ") + /** Display spaces */ def show(s: Space): String = { def params(tp: Type): List[Type] = tp.classSymbol.primaryConstructor.info.firstParamTypes @@ -888,49 +908,36 @@ class SpaceEngine(using Context) extends SpaceLogic { if (!redundancyCheckable(sel)) return - val targetSpace = - if !selTyp.classSymbol.isNullableClass then - project(selTyp) - else - project(OrType(selTyp, constantNullType, soft = false)) - - // in redundancy check, take guard as false in order to soundly approximate - val spaces = cases.map { x => - val res = - if (x.guard.isEmpty) project(x.pat) - else Empty + val isNullable = selTyp.classSymbol.isNullableClass + val targetSpace = if isNullable + then project(OrType(selTyp, constantNullType, soft = false)) + else project(selTyp) + debug.println(s"targetSpace: ${show(targetSpace)}") - debug.println(s"${x.pat.show} ====> ${res}") - res - } - - (1 until cases.length).foreach { i => - val pat = cases(i).pat + cases.iterator.zipWithIndex.foldLeft(Nil: List[Space]) { case (prevs, (CaseDef(pat, guard, _), i)) => + debug.println(i"case pattern: $pat") - if (pat != EmptyTree) { // rethrow case of catch uses EmptyTree - val prevs = Or(spaces.take(i)) - val curr = project(pat) + val curr = project(pat) + debug.println(i"reachable? ${show(curr)}") - debug.println(s"---------------reachable? ${show(curr)}") - debug.println(s"prev: ${show(prevs)}") + val prev = simplify(Or(prevs)) + debug.println(s"prev: ${show(prev)}") - var covered = simplify(intersect(curr, targetSpace)) - debug.println(s"covered: $covered") + val covered = simplify(intersect(curr, targetSpace)) + debug.println(s"covered: ${show(covered)}") - // `covered == Empty` may happen for primitive types with auto-conversion - // see tests/patmat/reader.scala tests/patmat/byte.scala - if (covered == Empty && !isNullLit(pat)) covered = curr - - if (isSubspace(covered, prevs)) { - if i == cases.length - 1 - && isWildcardArg(pat) - && pat.tpe.classSymbol.isNullableClass - then - report.warning(MatchCaseOnlyNullWarning(), pat.srcPos) - else - report.warning(MatchCaseUnreachable(), pat.srcPos) - } + if pat != EmptyTree // rethrow case of catch uses EmptyTree + && prev != Empty // avoid isSubspace(Empty, Empty) - one of the previous cases much be reachable + && isSubspace(covered, prev) + then { + if isNullable && i == cases.length - 1 && isWildcardArg(pat) then + report.warning(MatchCaseOnlyNullWarning(), pat.srcPos) + else + report.warning(MatchCaseUnreachable(), pat.srcPos) } + + // in redundancy check, take guard as false in order to soundly approximate + (if guard.isEmpty then covered else Empty) :: prevs } } } diff --git a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala index 7938a4e5af47..3d877a46558f 100644 --- a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala +++ b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala @@ -45,7 +45,7 @@ class PatmatExhaustivityTest { val baseFilePath = path.toString.stripSuffix(".scala") val checkFilePath = baseFilePath + ".check" - FileDiff.checkAndDump(path.toString, actualLines, checkFilePath) + FileDiff.checkAndDumpOrUpdate(path.toString, actualLines, checkFilePath) } /** A single test with multiple files grouped in a folder */ @@ -57,13 +57,17 @@ class PatmatExhaustivityTest { val actualLines = compile(files) val checkFilePath = s"${path}${File.separator}expected.check" - FileDiff.checkAndDump(path.toString, actualLines, checkFilePath) + FileDiff.checkAndDumpOrUpdate(path.toString, actualLines, checkFilePath) } @Test def patmatExhaustivity: Unit = { val res = Directory(testsDir).list.toList .filter(f => f.extension == "scala" || f.isDirectory) + .filter { f => + val path = if f.isDirectory then f.path + "/" else f.path + path.contains(Properties.testsFilter.getOrElse("")) + } .map(f => if f.isDirectory then compileDir(f.jpath) else compileFile(f.jpath)) val failed = res.filter(!_) diff --git a/compiler/test/dotty/tools/vulpix/FileDiff.scala b/compiler/test/dotty/tools/vulpix/FileDiff.scala index af44dbf2076d..835e8c6a2d6f 100644 --- a/compiler/test/dotty/tools/vulpix/FileDiff.scala +++ b/compiler/test/dotty/tools/vulpix/FileDiff.scala @@ -63,4 +63,21 @@ object FileDiff { } } + def checkAndDumpOrUpdate(sourceTitle: String, actualLines: Seq[String], checkFilePath: String): Boolean = { + val outFilePath = checkFilePath + ".out" + FileDiff.check(sourceTitle, actualLines, checkFilePath) match { + case Some(msg) if dotty.Properties.testsUpdateCheckfile => + FileDiff.dump(checkFilePath, actualLines) + println("Updated checkfile: " + checkFilePath) + false + case Some(msg) => + FileDiff.dump(outFilePath, actualLines) + println(msg) + println(FileDiff.diffMessage(checkFilePath, outFilePath)) + false + case _ => + Files.deleteIfExists(Paths.get(outFilePath)) + true + } + } } diff --git a/tests/patmat/boxing.scala b/tests/patmat/boxing.scala new file mode 100644 index 000000000000..d148e23deca1 --- /dev/null +++ b/tests/patmat/boxing.scala @@ -0,0 +1,18 @@ +class C { + def matchUnboxed(i: Integer) = i match { + case null => 0 + case 1 => 1 + case _ => 9 + } + + def matchBoxed(i: Int) = i match { + case C.ZERO => 0 + case C.ONE => 1 + case _ => 9 + } +} + +object C { + final val ZERO: Integer = 0 + final val ONE: Integer = 1 +} diff --git a/tests/patmat/i12805-fallout.scala b/tests/patmat/i12805-fallout.scala new file mode 100644 index 000000000000..b598b36159ea --- /dev/null +++ b/tests/patmat/i12805-fallout.scala @@ -0,0 +1,30 @@ +import scala.annotation.unchecked.uncheckedVariance + +type Untyped = Null + +class Type + +abstract class Tree[-T >: Untyped] { + type ThisTree[T >: Untyped] <: Tree[T] + + protected var myTpe: T @uncheckedVariance = _ + + def withType(tpe: Type): ThisTree[Type] = { + val tree = this.asInstanceOf[ThisTree[Type]] + tree.myTpe = tpe + tree + } +} + +case class Ident[-T >: Untyped]() extends Tree[T] +case class DefDef[-T >: Untyped]() extends Tree[T] +case class Inlined[-T >: Untyped]() extends Tree[T] +case class CaseDef[-T >: Untyped]() extends Tree[T] + +def test[T >: Untyped](tree: Tree[T], tp: Type) = tree.withType(tp) match { + case Ident() => 1 + case DefDef() => 2 + case _: Inlined[_] => 3 + case CaseDef() => 4 + case _ => 5 +} diff --git a/tests/patmat/i12805.check b/tests/patmat/i12805.check new file mode 100644 index 000000000000..e855c765d801 --- /dev/null +++ b/tests/patmat/i12805.check @@ -0,0 +1,3 @@ +10: Match case Unreachable +16: Match case Unreachable +22: Match case Unreachable diff --git a/tests/patmat/i12805.scala b/tests/patmat/i12805.scala new file mode 100644 index 000000000000..78240c2f9703 --- /dev/null +++ b/tests/patmat/i12805.scala @@ -0,0 +1,22 @@ +import scala.language.implicitConversions + +type Timeframe = "1m" | "2m" | "1H" +type TimeframeN = 1 | 2 | 60 + +def manualConvertToN(tf: Timeframe): TimeframeN = tf match + case "1m" => 1 + case "2m" => 2 + case "1H" => 60 + case "4H" => ??? // was: no reachability warning + +given Conversion[Timeframe, TimeframeN] = + case "1m" => 1 + case "2m" => 2 + case "1H" => 60 + case "4H" => ??? // was: no reachability warning + +given Conversion[TimeframeN, Timeframe] = + case 1 => "1m" + case 2 => "2m" + case 60 => "1H" + case 240 => ??? // was: no reachability warning diff --git a/tests/patmat/i12805b.check b/tests/patmat/i12805b.check new file mode 100644 index 000000000000..62ba8523f65e --- /dev/null +++ b/tests/patmat/i12805b.check @@ -0,0 +1,3 @@ +4: Match case Unreachable +9: Match case Unreachable +14: Match case Unreachable diff --git a/tests/patmat/i12805b.scala b/tests/patmat/i12805b.scala new file mode 100644 index 000000000000..cbff6ae0d2ca --- /dev/null +++ b/tests/patmat/i12805b.scala @@ -0,0 +1,14 @@ +def test1(a: 1 | 2) = a match + case 1 => true + case 2 => false + case 4 => ??? // unreachable case, was: no warning + +def test2(a: 1 | 2) = a match + case 1 => true + case 2 => false + case _ => ??? // unreachable + +def test3(a: 1 | 2) = a match + case 1 => true + case 2 => false + case a if a < 0 => ??? // unreachable