diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index 50d7a8c643a3..f3fba2e83fa7 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -627,8 +627,6 @@ object Trees { case class CaseDef[+T <: Untyped] private[ast] (pat: Tree[T], guard: Tree[T], body: Tree[T])(implicit @constructorOnly src: SourceFile) extends Tree[T] { type ThisTree[+T <: Untyped] = CaseDef[T] - /** Should this case be considered partial for exhaustivity and unreachability checking */ - def maybePartial(using Context): Boolean = !guard.isEmpty || body.isInstanceOf[SubMatch[T]] } /** label[tpt]: { expr } */ diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 60ddeac5d3cd..44b32b6bd8cf 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -930,12 +930,117 @@ object SpaceEngine { case _ => tp }) + /** Check if the SubMatch selector references the variable bound by the outer pattern. + * + * case x @ _ if x match + * ^ pat ^ selector + * + */ + private object SelectorBoundVar: + def unapply(args: (Tree, Tree))(using Context): Boolean = + val (selector, pat) = args + pat match + case b: Bind => selector.symbol == b.symbol + case _ => false + + /** Find the index of the parameter in an outer UnApply pattern that directly binds the selector symbol. + * + * case Wrapper(c) if c match + * ^ returns Some(0) + * + */ + private object SelectorParamIndex: + def unapply(args: (Tree, Tree))(using Context): Option[Int] = + val (selector, pat) = args + unbind(pat) match + case UnApply(_, _, pats) => + val idx = pats.indexWhere { + case b: Bind => b.symbol == selector.symbol + case _ => false + } + Option.when(idx >= 0)(idx) + case _ => None + + /** Find the constructor parameter index corresponding to a field access on the outer pattern's bound var. + * + * case x if x.version match -- returns Some(1) for Document(title, version) + * ^^^^^^^^^ selector + * + */ + private object SelectorFieldIndex: + def unapply(args: (Tree, Tree))(using Context): Option[Int] = + args match + case (Select(qual, fieldName), b: Bind) if b.symbol == qual.symbol => + val cls = toUnderlying(qual.tpe).classSymbol + if cls.is(CaseClass) && !cls.isOneOf(AbstractOrTrait) then + val idx = cls.caseAccessors.indexWhere(_.name == fieldName) + Option.when(idx >= 0)(idx) + else None + case _ => None + + private def narrowProdParam(patSpace: Space, idx: Int, subSpace: Space)(using Context): Option[Space] = + def narrow(prod: Prod): Option[Space] = + val Prod(tp, unappTp, params) = prod + if idx >= params.length then None + else + val narrowedParam = simplify(intersect(params(idx), subSpace)) + Some(simplify(Prod(tp, unappTp, params.updated(idx, narrowedParam)))) + patSpace match + case prod @ Prod(tp, unappTp1, _) => + expandCaseClass(tp) match + case null => None + case Prod(_, unappTp2, _) if isSameUnapply(unappTp1, unappTp2) => narrow(prod) + case Typ(tp, _) => + expandCaseClass(tp) match + case null => None + case prod => narrow(prod) + case _ => None + + private def projectSubMatch(pat: Tree, sm: SubMatch)(using Context): Option[Space] = + val Match(selector, cases) = sm + + val subSpace = Or(cases.map(projectCaseDef)) + if simplify(subSpace) == Empty then return None // all sub-cases are guarded or empty; treat outer case as partial + def selTyp = toUnderlying(selector.tpe) + def patSpace = project(pat) + + (selector, pat) match + case SelectorBoundVar() => + Some(simplify(intersect(patSpace, subSpace))) + case SelectorParamIndex(idx) => + narrowProdParam(patSpace, idx, subSpace) + case SelectorFieldIndex(idx) => + narrowProdParam(patSpace, idx, subSpace) + case _ if simplify(minus(project(selTyp), subSpace)) == Empty => + Some(patSpace) + case _ => None + + /** Resolve the space covered by a case and whether it may be partial. + * @return (space, maybePartial) where maybePartial is true when the case + * may not fully cover its pattern space (due to a guard or unresolvable SubMatch). + */ + private def resolveCaseDef(c: CaseDef, projectPat: Tree => Space)(using Context): (Space, Boolean) = + def patSpace = projectPat(c.pat) + + if !c.guard.isEmpty then (patSpace, true) + else c.body match + case sm: SubMatch => + projectSubMatch(c.pat, sm) match + case Some(space) => (space, false) + case None => (patSpace, true) + case _ => (patSpace, false) + + /** Project a single CaseDef to the space it definitely covers */ + private def projectCaseDef(c: CaseDef)(using Context): Space = + val (space, maybePartial) = resolveCaseDef(c, project) + if maybePartial then Empty else space + def checkExhaustivity(m: Match)(using Context): Unit = trace(i"checkExhaustivity($m)") { val selTyp = toUnderlying(m.selector.tpe.stripUnsafeNulls()).dealias val targetSpace = trace(i"targetSpace($selTyp)")(project(selTyp)) val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) => - val space = if x.maybePartial then Empty else trace(i"project(${x.pat})")(project(x.pat)) + val space = trace(i"projectCaseDef(${x.pat})")(projectCaseDef(x)) space :: acc }) @@ -978,7 +1083,7 @@ object SpaceEngine { cases match case Nil => case (c @ CaseDef(pat, _, _)) :: rest => - val curr = trace(i"project($pat)")(projectPat(pat)) + val (curr, maybePartial) = resolveCaseDef(c, projectPat) val covered = trace("covered")(simplify(intersect(curr, targetSpace))) val prev = trace("prev")(simplify(Or(prevs))) if prev == Empty && covered == Empty then // defer until a case is reachable @@ -1003,8 +1108,8 @@ object SpaceEngine { hadNullOnly = true report.warning(MatchCaseOnlyNullWarning(), pat.srcPos) - // in redundancy check, take guard as false (or potential sub cases as partial) for a sound approximation - val newPrev = if c.maybePartial then prevs else covered :: prevs + // in redundancy check, take guard as false for a sound approximation + val newPrev = if maybePartial then prevs else covered :: prevs recur(rest, newPrev, Nil) recur(m.cases, Nil, Nil) diff --git a/compiler/test/dotc/scoverage-ignore.excludelist b/compiler/test/dotc/scoverage-ignore.excludelist index 22cc1a29adb4..cb844ee8c5b8 100644 --- a/compiler/test/dotc/scoverage-ignore.excludelist +++ b/compiler/test/dotc/scoverage-ignore.excludelist @@ -80,6 +80,7 @@ quote-function-applied-to.scala skolems2.scala spurious-overload.scala sub-cases-exhaustivity.scala +sub-cases-reachability.scala tailrec.scala traitParams.scala i25460.scala diff --git a/tests/pos/sub-cases-exhaustivity.scala b/tests/pos/sub-cases-exhaustivity.scala new file mode 100644 index 000000000000..2081dacee2f3 --- /dev/null +++ b/tests/pos/sub-cases-exhaustivity.scala @@ -0,0 +1,117 @@ +//> using options -Werror +import scala.language.experimental.subCases + +case class NotFound(id: String) +enum AcceptError: + case IsCancelled(id: String) + case Denial +case object CatastrophicError +type Error = NotFound | AcceptError | CatastrophicError.type + +import AcceptError.* + +def errorToString: Error => String = + case NotFound(id) => s"NotFound: $id" + case CatastrophicError => s"It is all doom" + case ae if ae match + case Denial => s"In Denial" + case IsCancelled(id) => s"IsCancelled: $id" + +def errorToString2: Error => String = + case NotFound(id) => s"NotFound: $id" + case CatastrophicError => s"It is all doom" + case ae if ae match + case Denial => s"In Denial" + case ea if ea match + case IsCancelled(id) => s"IsCancelled: $id" + +enum Color: + case Red, Green, Blue + +def colorName(c: Color): String = c match + case c1 if c1 match + case Color.Red => "red" + case Color.Green => "green" + case Color.Blue => "blue" + +case class Wrapper(c: Color) + +def wrappedColorName(w: Wrapper): String = w match + case Wrapper(c) if c match + case Color.Red => "red" + case Color.Green => "green" + case Color.Blue => "blue" + +def wrappedColorName2(w: Wrapper): String = w match + case Wrapper(c) if c match + case Color.Red => "red" + case Color.Green => "green" + case Wrapper(c) if c match + case Color.Blue => "blue" + +def colorNameAlt(c: Color): String = c match + case c1 if c1 match + case Color.Red | Color.Green => "warm" + case Color.Blue => "cool" + +def wrappedColorBind(w: Wrapper): String = w match + case x @ Wrapper(c) if c match + case Color.Red => "red" + case Color.Green => "green" + case Color.Blue => "blue" + +def nestedSubcases(c: Color): String = c match + case c1 if c1 match + case Color.Red => "red" + case c2 if c2 match + case Color.Green => "green" + case Color.Blue => "blue" + +enum Version: + case Legacy + case Stable(major: Int, minor: Int) + +case class Document(title: String, version: Version) + +def getVersion(d: Option[Document]): String = d match + case Some(x) if x.version match + case Version.Stable(m, n) => s"$m.$n" + case Version.Legacy => "legacy" + case None => "none" + +sealed trait Shape +case class Circle(r: Double) extends Shape +case class Rectangle(w: Double, h: Double) extends Shape + +def tupleFirstExhaustive(pair: (Color, Color)): String = pair match + case (a, b) if a match + case Color.Red => "red first" + case Color.Green => "green first" + case Color.Blue => "blue first" + +def tupleSecondExhaustive(pair: (Color, Color)): String = pair match + case (a, b) if b match + case Color.Red => "red second" + case Color.Green => "green second" + case Color.Blue => "blue second" + +def typedBoundExhaustive(s: Shape): String = s match + case x: Circle if x match + case Circle(r) => s"circle r=$r" + case _: Rectangle => "rectangle" + +def typedGuardedFallback(s: Shape): String = s match + case x: Circle if x match + case Circle(r) if r > 0 => "positive circle" + case _: Circle => "other circle" + case _: Rectangle => "rectangle" + +def fieldAccessExhaustive(d: Document): String = d match + case x if x.version match + case Version.Stable(m, n) => s"$m.$n" + case Version.Legacy => "legacy" + +def fieldAccessTypedBound(d: Document): String = d match + case x: Document if x.version match + case Version.Stable(m, n) => s"$m.$n" + case Version.Legacy => "legacy" diff --git a/tests/pos/sub-cases-reachability.scala b/tests/pos/sub-cases-reachability.scala new file mode 100644 index 000000000000..681c748acee2 --- /dev/null +++ b/tests/pos/sub-cases-reachability.scala @@ -0,0 +1,29 @@ +//> using options -Werror +import scala.language.experimental.subCases + +enum Color: + case Red, Green, Blue + +def guardMakesPartial(c: Color, flag: Boolean): String = c match + case c1 if flag if c1 match + case Color.Red => "red" + case Color.Green => "green" + case Color.Blue => "blue" + case _ => "fallback" + +def literalFallback(n: Int): String = n match + case x if x match + case 1 => "one" + case 2 => "two" + case _ => "other" + +enum Version: + case Legacy + case Stable + +case class Document(version: Version) + +def partialFieldAccess(d: Document): String = d match + case x if x.version match + case Version.Legacy => "legacy" + case _ => "other" diff --git a/tests/warn/sub-cases-exhaustivity.check b/tests/warn/sub-cases-exhaustivity.check index ef59bd28d8fb..44a183a5d0d5 100644 --- a/tests/warn/sub-cases-exhaustivity.check +++ b/tests/warn/sub-cases-exhaustivity.check @@ -14,7 +14,59 @@ | It would fail on pattern case: E.B(_) | | longer explanation available when compiling with `-explain` --- [E030] Match case Unreachable Warning: tests/warn/sub-cases-exhaustivity.scala:32:9 --------------------------------- -32 | case A(_) => 3 // warn: unreacheable - | ^^^^ - | Unreachable case +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:40:2 ----------------------------- +40 | w match // warn: match may not be exhaustive: It would fail on pattern case: Wrapper(Red) + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: Wrapper(Red) + | + | longer explanation available when compiling with `-explain` +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:46:2 ----------------------------- +46 | c match // warn: match may not be exhaustive: It would fail on pattern case: Color.Blue + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: Blue + | + | longer explanation available when compiling with `-explain` +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:53:2 ----------------------------- +53 | c match // warn: match may not be exhaustive: It would fail on pattern case: Color.Blue + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: Blue + | + | longer explanation available when compiling with `-explain` +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:61:54 ---------------------------- +61 |def tupleFirstMissing(pair: (Color, Color)): String = pair match // warn + | ^^^^ + | match may not be exhaustive. + | + | It would fail on pattern case: (Blue, _) + | + | longer explanation available when compiling with `-explain` +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:66:55 ---------------------------- +66 |def tupleSecondMissing(pair: (Color, Color)): String = pair match // warn + | ^^^^ + | match may not be exhaustive. + | + | It would fail on pattern case: (_, Blue) + | + | longer explanation available when compiling with `-explain` +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:71:45 ---------------------------- +71 |def typedGuardedSubcases(s: Shape): String = s match // warn + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: Circle(_) + | + | longer explanation available when compiling with `-explain` +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:82:46 ---------------------------- +82 |def fieldAccessMissing(d: Document): String = d match // warn + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: Document(_, Legacy) + | + | longer explanation available when compiling with `-explain` diff --git a/tests/warn/sub-cases-exhaustivity.scala b/tests/warn/sub-cases-exhaustivity.scala index 73cea9c14fa9..218d1c7d8fe9 100644 --- a/tests/warn/sub-cases-exhaustivity.scala +++ b/tests/warn/sub-cases-exhaustivity.scala @@ -29,5 +29,56 @@ object Test: case C => 21 case A(_) => 22 case A(_) => 3 // nowarn: should not be reported as unreachable - case A(_) => 3 // warn: unreacheable case C => 4 + +enum Color: + case Red, Green, Blue + +case class Wrapper(c: Color) + +def wrappedColorName(w: Wrapper): String = + w match // warn: match may not be exhaustive: It would fail on pattern case: Wrapper(Red) + case Wrapper(c) if c match + case Color.Green => "green" + case Color.Blue => "blue" + +def nestedSubcasesMissing(c: Color): String = + c match // warn: match may not be exhaustive: It would fail on pattern case: Color.Blue + case c1 if c1 match + case Color.Red => "red" + case c2 if c2 match + case Color.Green => "green" + +def testAlternativeMissing(c: Color): String = + c match // warn: match may not be exhaustive: It would fail on pattern case: Color.Blue + case c1 if c1 match + case Color.Red | Color.Green => "warm" + +sealed trait Shape +case class Circle(r: Double) extends Shape +case class Rectangle(w: Double, h: Double) extends Shape + +def tupleFirstMissing(pair: (Color, Color)): String = pair match // warn + case (a, b) if a match + case Color.Red => "red first" + case Color.Green => "green first" + +def tupleSecondMissing(pair: (Color, Color)): String = pair match // warn + case (a, b) if b match + case Color.Red => "red second" + case Color.Green => "green second" + +def typedGuardedSubcases(s: Shape): String = s match // warn + case x: Circle if x match + case Circle(r) if r > 0 => "positive circle" + case _: Rectangle => "rectangle" + +enum Version: + case Legacy + case Stable(major: Int, minor: Int) + +case class Document(title: String, version: Version) + +def fieldAccessMissing(d: Document): String = d match // warn + case x if x.version match + case Version.Stable(m, n) => s"$m.$n" diff --git a/tests/warn/sub-cases-reachability.check b/tests/warn/sub-cases-reachability.check new file mode 100644 index 000000000000..72a07fe34ce0 --- /dev/null +++ b/tests/warn/sub-cases-reachability.check @@ -0,0 +1,32 @@ +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:15:10 -------------------------------- +15 | case AB.A => "unreachable" // warn + | ^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:21:14 -------------------------------- +21 | case Wrapper(Color.Red) => "unreachable" // warn + | ^^^^^^^^^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:30:14 -------------------------------- +30 | case Wrapper(_) => "unreachable" // warn + | ^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:37:14 -------------------------------- +37 | case Wrapper(_) => "unreachable" // warn + | ^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:45:13 -------------------------------- +45 | case Color.Blue => "unreachable" // warn + | ^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:50:13 -------------------------------- +50 | case Color.Red => "unreachable" // warn + | ^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:63:7 --------------------------------- +63 | case (Color.Red, _) => "unreachable" // warn + | ^^^^^^^^^^^^^^ + | Unreachable case +-- [E030] Match case Unreachable Warning: tests/warn/sub-cases-reachability.scala:69:15 -------------------------------- +69 | case Document(_, Version.Legacy) => "unreachable" // warn + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Unreachable case diff --git a/tests/warn/sub-cases-reachability.scala b/tests/warn/sub-cases-reachability.scala new file mode 100644 index 000000000000..6621297b2220 --- /dev/null +++ b/tests/warn/sub-cases-reachability.scala @@ -0,0 +1,69 @@ +import scala.language.experimental.subCases + +enum Color: + case Red, Green, Blue + +case class Wrapper(c: Color) + +enum AB: + case A, B + +def testBoundVarReachability(ab: AB) = ab match + case x if x match + case AB.A => "a" + case AB.B => "b" + case AB.A => "unreachable" // warn + +def testParamIndexReachability(w: Wrapper) = w match + case Wrapper(c) if c match + case Color.Red => "red" + case Color.Green => "green" + case Wrapper(Color.Red) => "unreachable" // warn + case Wrapper(Color.Blue) => "blue" // not unreachable + +def testCombinedReachability(w: Wrapper) = w match + case Wrapper(c) if c match + case Color.Red => "red" + case Color.Green => "green" + case Wrapper(c) if c match + case Color.Blue => "blue" + case Wrapper(_) => "unreachable" // warn + +def testBindWrappingUnApply(w: Wrapper) = w match + case x @ Wrapper(c) if c match + case Color.Red => "red" + case Color.Green => "green" + case Color.Blue => "blue" + case Wrapper(_) => "unreachable" // warn + +def testNestedSubcases(c: Color): String = c match + case c1 if c1 match + case Color.Red => "red" + case c2 if c2 match + case Color.Green => "green" + case Color.Blue => "blue" + case Color.Blue => "unreachable" // warn + +def testAlternativeReachability(c: Color): String = c match + case c1 if c1 match + case Color.Red | Color.Green | Color.Blue => "all" + case Color.Red => "unreachable" // warn + +enum Version: + case Legacy + case Stable(major: Int, minor: Int) + +case class Document(title: String, version: Version) + +def tupleFirstReachability(pair: (Color, Color)): String = pair match + case (a, b) if a match + case Color.Red => "red first" + case Color.Green => "green first" + case Color.Blue => "blue first" + case (Color.Red, _) => "unreachable" // warn + +def fieldAccessReachability(d: Document): String = d match + case x if x.version match + case Version.Stable(m, n) => s"$m.$n" + case Version.Legacy => "legacy" + case Document(_, Version.Legacy) => "unreachable" // warn