Skip to content

Commit 17cd9aa

Browse files
HarrisL2HarrisL2
committed
Add suggested changes
1 parent 158c1df commit 17cd9aa

File tree

9 files changed

+84
-66
lines changed

9 files changed

+84
-66
lines changed

‎compiler/src/dotty/tools/dotc/transform/patmat/Space.scala‎

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ object SpaceEngine {
116116
def isSubspace(a: Space, b: Space)(using Context): Boolean = a.isSubspace(b)
117117
def canDecompose(typ: Typ)(using Context): Boolean = typ.canDecompose
118118
def decompose(typ: Typ)(using Context): List[Typ] = typ.decompose
119+
def nullSpace(using Context): Space = Typ(ConstantType(Constant(null)), decomposed = false)
119120

120121
/** Simplify space such that a space equal to `Empty` becomes `Empty` */
121122
def computeSimplify(space: Space)(using Context): Space = trace(i"simplify($space)")(space match {
@@ -336,6 +337,13 @@ object SpaceEngine {
336337
case pat: Ident if isBackquoted(pat) =>
337338
Typ(pat.tpe, decomposed = false)
338339

340+
case Ident(nme.WILDCARD) =>
341+
val tp = pat.tpe.stripAnnots.widenSkolem
342+
val isNullable = tp.isInstanceOf[FlexibleType] || tp.classSymbol.isNullableClass
343+
val tpSpace = Typ(erase(tp, isValue = true), decomposed = false)
344+
if isNullable then Or(tpSpace :: nullSpace :: Nil)
345+
else tpSpace
346+
339347
case Ident(_) | Select(_, _) =>
340348
Typ(erase(pat.tpe.stripAnnots.widenSkolem, isValue = true), decomposed = false)
341349

@@ -667,7 +675,7 @@ object SpaceEngine {
667675
case tp => (tp, Nil)
668676
val (tp, typeArgs) = getAppliedClass(tpOriginal)
669677
// This function is needed to get the arguments of the types that will be applied to the class.
670-
// This is necessary because if the arguments of the types contain Nothing,
678+
// This is necessary because if the arguments of the types contain Nothing,
671679
// then this can affect whether the class will be taken into account during the exhaustiveness check
672680
def getTypeArgs(parent: Symbol, child: Symbol, typeArgs: List[Type]): List[Type] =
673681
val superType = child.typeRef.superType
@@ -923,50 +931,48 @@ object SpaceEngine {
923931
&& !sel.tpe.widen.isRef(defn.QuotedExprClass)
924932
&& !sel.tpe.widen.isRef(defn.QuotedTypeClass)
925933

926-
def mayCoverNull(tp: Space)(using Context): Boolean = tp match
927-
case Empty => false
928-
case Prod(_, _, _) => false
929-
case Typ(tp, decomposed) => tp == ConstantType(Constant(null))
930-
case Or(ss) => ss.exists(mayCoverNull)
931-
932934
def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)"):
933935
val selTyp = toUnderlying(m.selector.tpe).dealias
934936
val isNullable = selTyp.isInstanceOf[FlexibleType] || selTyp.classSymbol.isNullableClass
935937
val targetSpace = trace(i"targetSpace($selTyp)"):
936938
if isNullable && !ctx.mode.is(Mode.SafeNulls)
937939
then project(OrType(selTyp, ConstantType(Constant(null)), soft = false))
938940
else project(selTyp)
939-
940-
@tailrec def recur(cases: List[CaseDef], prevs: List[Space], deferred: List[Tree], nullCovered: Boolean): Unit =
941+
var hadNullOnly = false
942+
@tailrec def recur(cases: List[CaseDef], prevs: List[Space], deferred: List[Tree]): Unit =
941943
cases match
942944
case Nil =>
943-
case (c @ CaseDef(pat, guard, _)) :: rest =>
944-
val patNullable = Nullables.matchesNull(c)
945-
val curr = trace(i"project($pat)")(
946-
if patNullable
947-
then Or(List(project(pat), Typ(ConstantType(Constant(null)))))
948-
else project(pat))
945+
case CaseDef(pat, guard, _) :: rest =>
946+
val curr = trace(i"project($pat)")(project(pat))
949947
val covered = trace("covered")(simplify(intersect(curr, targetSpace)))
950948
val prev = trace("prev")(simplify(Or(prevs)))
951949
if prev == Empty && covered == Empty then // defer until a case is reachable
952-
recur(rest, prevs, pat :: deferred, nullCovered)
950+
recur(rest, prevs, pat :: deferred)
953951
else
954952
for pat <- deferred.reverseIterator
955953
do report.warning(MatchCaseUnreachable(), pat.srcPos)
956954

957955
if pat != EmptyTree // rethrow case of catch uses EmptyTree
958956
&& !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
959-
&& isSubspace(covered, Or(List(prev, Typ(ConstantType(Constant(null))))))
960957
then
961-
val nullOnly = isNullable && isWildcardArg(pat) && !nullCovered && !isSubspace(covered, prev) && (!ctx.explicitNulls || selTyp.isInstanceOf[FlexibleType])
962-
if nullOnly then report.warning(MatchCaseOnlyNullWarning() , pat.srcPos)
963-
else if (isSubspace(covered, prev)) then report.warning(MatchCaseUnreachable(), pat.srcPos)
958+
if isSubspace(covered, prev) then
959+
report.warning(MatchCaseUnreachable(), pat.srcPos)
960+
else if isNullable && !hadNullOnly && isWildcardArg(pat)
961+
&& isSubspace(covered, Or(prev :: nullSpace :: Nil)) then
962+
// Issue OnlyNull warning only if:
963+
// 1. The target space is nullable;
964+
// 2. OnlyNull warning has not been issued before;
965+
// 3. The pattern is a wildcard pattern;
966+
// 4. The pattern is not covered by the previous cases,
967+
// but covered by the previous cases with null.
968+
hadNullOnly = true
969+
report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
964970

965971
// in redundancy check, take guard as false in order to soundly approximate
966-
val newPrev = if (guard.isEmpty) then covered :: prevs else prevs
967-
recur(rest, newPrev, Nil, nullCovered || (guard.isEmpty && patNullable))
972+
val newPrev = if guard.isEmpty then covered :: prevs else prevs
973+
recur(rest, newPrev, Nil)
968974

969-
recur(m.cases, Nil, Nil, false)
975+
recur(m.cases, Nil, Nil)
970976
end checkReachability
971977

972978
def checkMatch(m: Match)(using Context): Unit =

‎tests/explicit-nulls/warn/i21577.check‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,29 @@
11
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:5:9 --------------------------------------------
2-
5 | case _ => // warn
2+
5 | case _ => // warn: null only
33
| ^
44
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
55
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:9 -------------------------------------------
6-
12 | case _ => // warn
6+
12 | case _ => // warn: null only
77
| ^
88
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
9+
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:16:7 -------------------------------------------
10+
16 | case _ => // warn: null only
11+
| ^
12+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
913
-- [E030] Match case Unreachable Warning: tests/explicit-nulls/warn/i21577.scala:20:7 ----------------------------------
10-
20 | case _ => // warn
14+
20 | case _ => // warn: unreachable
1115
| ^
1216
| Unreachable case
1317
-- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:29:27 -----------------------------
14-
29 |def f7(s: String | Null) = s match // warn
18+
29 |def f7(s: String | Null) = s match // warn: not exhuastive
1519
| ^
1620
| match may not be exhaustive.
1721
|
1822
| It would fail on pattern case: _: Null
1923
|
2024
| longer explanation available when compiling with `-explain`
2125
-- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:36:33 -----------------------------
22-
36 |def f9(s: String | Int | Null) = s match // warn
26+
36 |def f9(s: String | Int | Null) = s match // warn: not exhuastive
2327
| ^
2428
| match may not be exhaustive.
2529
|

‎tests/explicit-nulls/warn/i21577.scala‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,22 @@ def f(s: String) =
22
val s2 = s.trim()
33
s2 match
44
case s3: String =>
5-
case _ => // warn
5+
case _ => // warn: null only
66

77

88
def f2(s: String | Null) =
99
val s2 = s.nn.trim()
1010
s2 match
1111
case s3: String =>
12-
case _ => // warn
12+
case _ => // warn: null only
1313

1414
def f3(s: String | Null) = s match
1515
case s2: String =>
16-
case _ =>
16+
case _ => // warn: null only
1717

1818
def f5(s: String) = s match
1919
case _: String =>
20-
case _ => // warn
20+
case _ => // warn: unreachable
2121

2222
def f6(s: String) = s.trim() match
2323
case _: String =>
@@ -26,13 +26,13 @@ def f6(s: String) = s.trim() match
2626
def f61(s: String) = s.trim() match
2727
case _: String =>
2828

29-
def f7(s: String | Null) = s match // warn
29+
def f7(s: String | Null) = s match // warn: not exhuastive
3030
case _: String =>
3131

3232
def f8(s: String | Null) = s match
3333
case _: String =>
3434
case null =>
3535

36-
def f9(s: String | Int | Null) = s match // warn
36+
def f9(s: String | Int | Null) = s match // warn: not exhuastive
3737
case _: String =>
3838
case null =>

‎tests/patmat/null.check‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
6: Match case Unreachable
1+
6: Pattern Match
22
13: Pattern Match
33
20: Pattern Match

‎tests/warn/i20121.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ case class CC_B[A](a: A) extends T_A[A, X]
55

66
val v_a: T_A[X, X] = CC_B(null)
77
val v_b = v_a match
8-
case CC_B(_) => 0 // warn: unreachable
9-
case _ => 1
8+
case CC_B(_) => 0
9+
case _ => 1 // warn: null only
1010
// for CC_B[A] to match T_A[X, X]
1111
// A := X
1212
// so require X, aka T_A[Byte, Byte]

‎tests/warn/i20122.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ case class CC_E(a: CC_C[Char, Byte])
77

88
val v_a: T_B[Int, CC_A] = CC_B(CC_E(CC_C(null)))
99
val v_b = v_a match
10-
case CC_B(CC_E(CC_C(_))) => 0 // warn: unreachable
10+
case CC_B(CC_E(CC_C(_))) => 0
1111
case _ => 1
1212
// for CC_B[A, C] to match T_B[C, CC_A]
1313
// C <: Int, ok

‎tests/warn/i20123.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ case class CC_G[A, C](c: C) extends T_A[A, C]
88
val v_a: T_A[Boolean, T_B[Boolean]] = CC_G(null)
99
val v_b = v_a match {
1010
case CC_D() => 0
11-
case CC_G(_) => 1 // warn: unreachable
11+
case CC_G(_) => 1
1212
// for CC_G[A, C] to match T_A[Boolean, T_B[Boolean]]
1313
// A := Boolean, which is ok
1414
// C := T_B[Boolean],

‎tests/warn/redundant-null.check‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,56 @@
11
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:10:7 -----------------------------------------
2-
10 | case _: n.type => // warn
2+
10 | case _: n.type => // warn: unreachable
33
| ^^^^^^^^^
44
| Unreachable case
55
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:12:7 -----------------------------------------
6-
12 | case _ => // warn
6+
12 | case _ => // warn: unreachable
77
| ^
88
| Unreachable case
99
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:13:7 -----------------------------------------
10-
13 | case _ => // warn
10+
13 | case _ => // warn: unreachable
1111
| ^
1212
| Unreachable case
1313
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:18:7 -----------------------------------------
14-
18 | case _ => 3 // warn
14+
18 | case _ => 3 // warn: unreachable
1515
| ^
1616
| Unreachable case
1717
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:23:7 -----------------------------------------
18-
23 | case _: B => // warn
18+
23 | case _: B => // warn: unreachable
1919
| ^^^^
2020
| Unreachable case
2121
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:24:7 -----------------------------------------
22-
24 | case _ => // warn
22+
24 | case _ => // warn: unreachable
2323
| ^
2424
| Unreachable case
2525
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:25:7 -----------------------------------------
26-
25 | case null => // warn
26+
25 | case null => // warn: unreachable
2727
| ^^^^
2828
| Unreachable case
2929
-- [E121] Pattern Match Warning: tests/warn/redundant-null.scala:30:7 --------------------------------------------------
30-
30 | case _ => // warn
30+
30 | case _ => // warn: null only
3131
| ^
3232
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
3333
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:31:7 -----------------------------------------
34-
31 | case null => // warn
34+
31 | case null => // warn: unreachable
3535
| ^^^^
3636
| Unreachable case
3737
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:32:7 -----------------------------------------
38-
32 | case _ => // warn
38+
32 | case _ => // warn: unreachable
3939
| ^
4040
| Unreachable case
4141
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:33:7 -----------------------------------------
42-
33 | case _ => // warn
42+
33 | case _ => // warn: unreachable
4343
| ^
4444
| Unreachable case
4545
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:37:7 -----------------------------------------
46-
37 | case _ => println("unreachable") // warn
46+
37 | case _ => // warn: unreachable
4747
| ^
4848
| Unreachable case
4949
-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:41:7 -----------------------------------------
50-
41 | case _ => // warn
50+
41 | case _ => // warn: unreachable
5151
| ^
5252
| Unreachable case
53+
-- [E121] Pattern Match Warning: tests/warn/redundant-null.scala:45:7 --------------------------------------------------
54+
45 | case _ => // warn: null only
55+
| ^
56+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).

‎tests/warn/redundant-null.scala‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,35 +7,39 @@ val n = null
77
def f(s: A) = s match
88
case _: n.type =>
99
case _: A =>
10-
case _: n.type => // warn
10+
case _: n.type => // warn: unreachable
1111
case null =>
12-
case _ => // warn
13-
case _ => // warn
12+
case _ => // warn: unreachable
13+
case _ => // warn: unreachable
1414

1515
def f2(s: A | B | C) = s match
1616
case _: A => 0
1717
case _: C | null | _: B => 1
18-
case _ => 3 // warn
18+
case _ => 3 // warn: unreachable
1919

2020
def f3(s: A | B) = s match
2121
case _: A =>
2222
case _ =>
23-
case _: B => // warn
24-
case _ => // warn
25-
case null => // warn
23+
case _: B => // warn: unreachable
24+
case _ => // warn: unreachable
25+
case null => // warn: unreachable
2626

2727
def f4(s: String | Int) = s match
2828
case _: Int =>
2929
case _: String =>
30-
case _ => // warn
31-
case null => // warn
32-
case _ => // warn
33-
case _ => // warn
30+
case _ => // warn: null only
31+
case null => // warn: unreachable
32+
case _ => // warn: unreachable
33+
case _ => // warn: unreachable
3434

3535
def f5(x: String) = x match
36-
case x => println("catch all")
37-
case _ => println("unreachable") // warn
36+
case x =>
37+
case _ => // warn: unreachable
3838

3939
def test(s: String | Null) = s match
4040
case ss =>
41-
case _ => // warn
41+
case _ => // warn: unreachable
42+
43+
def test2(s: String | Null) = s match
44+
case ss: String =>
45+
case _ => // warn: null only

0 commit comments

Comments
 (0)