From 17cd9aa644beaf8fb99163c5a06e9772bc1abf62 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Mon, 4 Nov 2024 11:59:14 -0500 Subject: [PATCH] Add suggested changes --- .../tools/dotc/transform/patmat/Space.scala | 52 +++++++++++-------- tests/explicit-nulls/warn/i21577.check | 14 +++-- tests/explicit-nulls/warn/i21577.scala | 12 ++--- tests/patmat/null.check | 2 +- tests/warn/i20121.scala | 4 +- tests/warn/i20122.scala | 2 +- tests/warn/i20123.scala | 2 +- tests/warn/redundant-null.check | 30 ++++++----- tests/warn/redundant-null.scala | 32 +++++++----- 9 files changed, 84 insertions(+), 66 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 463bf767a442..7410d617c4a0 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -116,6 +116,7 @@ object SpaceEngine { def isSubspace(a: Space, b: Space)(using Context): Boolean = a.isSubspace(b) def canDecompose(typ: Typ)(using Context): Boolean = typ.canDecompose def decompose(typ: Typ)(using Context): List[Typ] = typ.decompose + def nullSpace(using Context): Space = Typ(ConstantType(Constant(null)), decomposed = false) /** Simplify space such that a space equal to `Empty` becomes `Empty` */ def computeSimplify(space: Space)(using Context): Space = trace(i"simplify($space)")(space match { @@ -336,6 +337,13 @@ object SpaceEngine { case pat: Ident if isBackquoted(pat) => Typ(pat.tpe, decomposed = false) + case Ident(nme.WILDCARD) => + val tp = pat.tpe.stripAnnots.widenSkolem + val isNullable = tp.isInstanceOf[FlexibleType] || tp.classSymbol.isNullableClass + val tpSpace = Typ(erase(tp, isValue = true), decomposed = false) + if isNullable then Or(tpSpace :: nullSpace :: Nil) + else tpSpace + case Ident(_) | Select(_, _) => Typ(erase(pat.tpe.stripAnnots.widenSkolem, isValue = true), decomposed = false) @@ -667,7 +675,7 @@ object SpaceEngine { case tp => (tp, Nil) val (tp, typeArgs) = getAppliedClass(tpOriginal) // This function is needed to get the arguments of the types that will be applied to the class. - // This is necessary because if the arguments of the types contain Nothing, + // This is necessary because if the arguments of the types contain Nothing, // then this can affect whether the class will be taken into account during the exhaustiveness check def getTypeArgs(parent: Symbol, child: Symbol, typeArgs: List[Type]): List[Type] = val superType = child.typeRef.superType @@ -923,12 +931,6 @@ object SpaceEngine { && !sel.tpe.widen.isRef(defn.QuotedExprClass) && !sel.tpe.widen.isRef(defn.QuotedTypeClass) - def mayCoverNull(tp: Space)(using Context): Boolean = tp match - case Empty => false - case Prod(_, _, _) => false - case Typ(tp, decomposed) => tp == ConstantType(Constant(null)) - case Or(ss) => ss.exists(mayCoverNull) - def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)"): val selTyp = toUnderlying(m.selector.tpe).dealias val isNullable = selTyp.isInstanceOf[FlexibleType] || selTyp.classSymbol.isNullableClass @@ -936,37 +938,41 @@ object SpaceEngine { if isNullable && !ctx.mode.is(Mode.SafeNulls) then project(OrType(selTyp, ConstantType(Constant(null)), soft = false)) else project(selTyp) - - @tailrec def recur(cases: List[CaseDef], prevs: List[Space], deferred: List[Tree], nullCovered: Boolean): Unit = + var hadNullOnly = false + @tailrec def recur(cases: List[CaseDef], prevs: List[Space], deferred: List[Tree]): Unit = cases match case Nil => - case (c @ CaseDef(pat, guard, _)) :: rest => - val patNullable = Nullables.matchesNull(c) - val curr = trace(i"project($pat)")( - if patNullable - then Or(List(project(pat), Typ(ConstantType(Constant(null))))) - else project(pat)) + case CaseDef(pat, guard, _) :: rest => + val curr = trace(i"project($pat)")(project(pat)) 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 - recur(rest, prevs, pat :: deferred, nullCovered) + recur(rest, prevs, pat :: deferred) else for pat <- deferred.reverseIterator do 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, Or(List(prev, Typ(ConstantType(Constant(null)))))) then - val nullOnly = isNullable && isWildcardArg(pat) && !nullCovered && !isSubspace(covered, prev) && (!ctx.explicitNulls || selTyp.isInstanceOf[FlexibleType]) - if nullOnly then report.warning(MatchCaseOnlyNullWarning() , pat.srcPos) - else if (isSubspace(covered, prev)) then report.warning(MatchCaseUnreachable(), pat.srcPos) + if isSubspace(covered, prev) then + report.warning(MatchCaseUnreachable(), pat.srcPos) + else if isNullable && !hadNullOnly && isWildcardArg(pat) + && isSubspace(covered, Or(prev :: nullSpace :: Nil)) then + // Issue OnlyNull warning only if: + // 1. The target space is nullable; + // 2. OnlyNull warning has not been issued before; + // 3. The pattern is a wildcard pattern; + // 4. The pattern is not covered by the previous cases, + // but covered by the previous cases with null. + hadNullOnly = true + report.warning(MatchCaseOnlyNullWarning(), pat.srcPos) // in redundancy check, take guard as false in order to soundly approximate - val newPrev = if (guard.isEmpty) then covered :: prevs else prevs - recur(rest, newPrev, Nil, nullCovered || (guard.isEmpty && patNullable)) + val newPrev = if guard.isEmpty then covered :: prevs else prevs + recur(rest, newPrev, Nil) - recur(m.cases, Nil, Nil, false) + recur(m.cases, Nil, Nil) end checkReachability def checkMatch(m: Match)(using Context): Unit = diff --git a/tests/explicit-nulls/warn/i21577.check b/tests/explicit-nulls/warn/i21577.check index acedd7a9c713..b548a5bedc30 100644 --- a/tests/explicit-nulls/warn/i21577.check +++ b/tests/explicit-nulls/warn/i21577.check @@ -1,17 +1,21 @@ -- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:5:9 -------------------------------------------- -5 | case _ => // warn +5 | case _ => // warn: null only | ^ | Unreachable case except for null (if this is intentional, consider writing case null => instead). -- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:9 ------------------------------------------- -12 | case _ => // warn +12 | case _ => // warn: null only | ^ | Unreachable case except for null (if this is intentional, consider writing case null => instead). +-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:16:7 ------------------------------------------- +16 | case _ => // warn: null only + | ^ + | Unreachable case except for null (if this is intentional, consider writing case null => instead). -- [E030] Match case Unreachable Warning: tests/explicit-nulls/warn/i21577.scala:20:7 ---------------------------------- -20 | case _ => // warn +20 | case _ => // warn: unreachable | ^ | Unreachable case -- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:29:27 ----------------------------- -29 |def f7(s: String | Null) = s match // warn +29 |def f7(s: String | Null) = s match // warn: not exhuastive | ^ | match may not be exhaustive. | @@ -19,7 +23,7 @@ | | longer explanation available when compiling with `-explain` -- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:36:33 ----------------------------- -36 |def f9(s: String | Int | Null) = s match // warn +36 |def f9(s: String | Int | Null) = s match // warn: not exhuastive | ^ | match may not be exhaustive. | diff --git a/tests/explicit-nulls/warn/i21577.scala b/tests/explicit-nulls/warn/i21577.scala index 67da6068f22c..1bba8f4da01f 100644 --- a/tests/explicit-nulls/warn/i21577.scala +++ b/tests/explicit-nulls/warn/i21577.scala @@ -2,22 +2,22 @@ def f(s: String) = val s2 = s.trim() s2 match case s3: String => - case _ => // warn + case _ => // warn: null only def f2(s: String | Null) = val s2 = s.nn.trim() s2 match case s3: String => - case _ => // warn + case _ => // warn: null only def f3(s: String | Null) = s match case s2: String => - case _ => + case _ => // warn: null only def f5(s: String) = s match case _: String => - case _ => // warn + case _ => // warn: unreachable def f6(s: String) = s.trim() match case _: String => @@ -26,13 +26,13 @@ def f6(s: String) = s.trim() match def f61(s: String) = s.trim() match case _: String => -def f7(s: String | Null) = s match // warn +def f7(s: String | Null) = s match // warn: not exhuastive case _: String => def f8(s: String | Null) = s match case _: String => case null => -def f9(s: String | Int | Null) = s match // warn +def f9(s: String | Int | Null) = s match // warn: not exhuastive case _: String => case null => \ No newline at end of file diff --git a/tests/patmat/null.check b/tests/patmat/null.check index f539a921e814..d9c265adf377 100644 --- a/tests/patmat/null.check +++ b/tests/patmat/null.check @@ -1,3 +1,3 @@ -6: Match case Unreachable +6: Pattern Match 13: Pattern Match 20: Pattern Match diff --git a/tests/warn/i20121.scala b/tests/warn/i20121.scala index ce8e3e4d74f6..b8402fa808ac 100644 --- a/tests/warn/i20121.scala +++ b/tests/warn/i20121.scala @@ -5,8 +5,8 @@ case class CC_B[A](a: A) extends T_A[A, X] val v_a: T_A[X, X] = CC_B(null) val v_b = v_a match - case CC_B(_) => 0 // warn: unreachable - case _ => 1 + case CC_B(_) => 0 + case _ => 1 // warn: null only // for CC_B[A] to match T_A[X, X] // A := X // so require X, aka T_A[Byte, Byte] diff --git a/tests/warn/i20122.scala b/tests/warn/i20122.scala index 50da42a5926c..d035a18d3b09 100644 --- a/tests/warn/i20122.scala +++ b/tests/warn/i20122.scala @@ -7,7 +7,7 @@ case class CC_E(a: CC_C[Char, Byte]) val v_a: T_B[Int, CC_A] = CC_B(CC_E(CC_C(null))) val v_b = v_a match - case CC_B(CC_E(CC_C(_))) => 0 // warn: unreachable + case CC_B(CC_E(CC_C(_))) => 0 case _ => 1 // for CC_B[A, C] to match T_B[C, CC_A] // C <: Int, ok diff --git a/tests/warn/i20123.scala b/tests/warn/i20123.scala index 32de903210b2..0af7aba5a3a5 100644 --- a/tests/warn/i20123.scala +++ b/tests/warn/i20123.scala @@ -8,7 +8,7 @@ case class CC_G[A, C](c: C) extends T_A[A, C] val v_a: T_A[Boolean, T_B[Boolean]] = CC_G(null) val v_b = v_a match { case CC_D() => 0 - case CC_G(_) => 1 // warn: unreachable + case CC_G(_) => 1 // for CC_G[A, C] to match T_A[Boolean, T_B[Boolean]] // A := Boolean, which is ok // C := T_B[Boolean], diff --git a/tests/warn/redundant-null.check b/tests/warn/redundant-null.check index 7fdfbc1718fc..9d710e18961d 100644 --- a/tests/warn/redundant-null.check +++ b/tests/warn/redundant-null.check @@ -1,52 +1,56 @@ -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:10:7 ----------------------------------------- -10 | case _: n.type => // warn +10 | case _: n.type => // warn: unreachable | ^^^^^^^^^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:12:7 ----------------------------------------- -12 | case _ => // warn +12 | case _ => // warn: unreachable | ^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:13:7 ----------------------------------------- -13 | case _ => // warn +13 | case _ => // warn: unreachable | ^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:18:7 ----------------------------------------- -18 | case _ => 3 // warn +18 | case _ => 3 // warn: unreachable | ^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:23:7 ----------------------------------------- -23 | case _: B => // warn +23 | case _: B => // warn: unreachable | ^^^^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:24:7 ----------------------------------------- -24 | case _ => // warn +24 | case _ => // warn: unreachable | ^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:25:7 ----------------------------------------- -25 | case null => // warn +25 | case null => // warn: unreachable | ^^^^ | Unreachable case -- [E121] Pattern Match Warning: tests/warn/redundant-null.scala:30:7 -------------------------------------------------- -30 | case _ => // warn +30 | case _ => // warn: null only | ^ | Unreachable case except for null (if this is intentional, consider writing case null => instead). -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:31:7 ----------------------------------------- -31 | case null => // warn +31 | case null => // warn: unreachable | ^^^^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:32:7 ----------------------------------------- -32 | case _ => // warn +32 | case _ => // warn: unreachable | ^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:33:7 ----------------------------------------- -33 | case _ => // warn +33 | case _ => // warn: unreachable | ^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:37:7 ----------------------------------------- -37 | case _ => println("unreachable") // warn +37 | case _ => // warn: unreachable | ^ | Unreachable case -- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:41:7 ----------------------------------------- -41 | case _ => // warn +41 | case _ => // warn: unreachable | ^ | Unreachable case +-- [E121] Pattern Match Warning: tests/warn/redundant-null.scala:45:7 -------------------------------------------------- +45 | case _ => // warn: null only + | ^ + | Unreachable case except for null (if this is intentional, consider writing case null => instead). diff --git a/tests/warn/redundant-null.scala b/tests/warn/redundant-null.scala index 17da3e88350e..ec0096d78daf 100644 --- a/tests/warn/redundant-null.scala +++ b/tests/warn/redundant-null.scala @@ -7,35 +7,39 @@ val n = null def f(s: A) = s match case _: n.type => case _: A => - case _: n.type => // warn + case _: n.type => // warn: unreachable case null => - case _ => // warn - case _ => // warn + case _ => // warn: unreachable + case _ => // warn: unreachable def f2(s: A | B | C) = s match case _: A => 0 case _: C | null | _: B => 1 - case _ => 3 // warn + case _ => 3 // warn: unreachable def f3(s: A | B) = s match case _: A => case _ => - case _: B => // warn - case _ => // warn - case null => // warn + case _: B => // warn: unreachable + case _ => // warn: unreachable + case null => // warn: unreachable def f4(s: String | Int) = s match case _: Int => case _: String => - case _ => // warn - case null => // warn - case _ => // warn - case _ => // warn + case _ => // warn: null only + case null => // warn: unreachable + case _ => // warn: unreachable + case _ => // warn: unreachable def f5(x: String) = x match - case x => println("catch all") - case _ => println("unreachable") // warn + case x => + case _ => // warn: unreachable def test(s: String | Null) = s match case ss => - case _ => // warn \ No newline at end of file + case _ => // warn: unreachable + +def test2(s: String | Null) = s match + case ss: String => + case _ => // warn: null only \ No newline at end of file