Skip to content

Commit 1199f00

Browse files
HarrisL2KacperFKorban
HarrisL2
authored and
KacperFKorban
committed
Implement change and add test cases
1 parent e8576e9 commit 1199f00

File tree

7 files changed

+114
-76
lines changed

7 files changed

+114
-76
lines changed

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

Lines changed: 33 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import typer.*, Applications.*, Inferencing.*, ProtoTypes.*
1414
import util.*
1515

1616
import scala.annotation.internal.sharable
17+
import scala.annotation.tailrec
1718
import scala.collection.mutable
1819

1920
import SpaceEngine.*
@@ -696,7 +697,7 @@ object SpaceEngine {
696697
else NoType
697698
}.filter(_.exists)
698699
parts
699-
700+
case tp: FlexibleType => List(tp.underlying, ConstantType(Constant(null)))
700701
case _ => ListOfNoType
701702
end rec
702703

@@ -876,6 +877,7 @@ object SpaceEngine {
876877
case tp: SingletonType => toUnderlying(tp.underlying)
877878
case tp: ExprType => toUnderlying(tp.resultType)
878879
case AnnotatedType(tp, annot) => AnnotatedType(toUnderlying(tp), annot)
880+
case tp: FlexibleType => tp.derivedFlexibleType(toUnderlying(tp.underlying))
879881
case _ => tp
880882
})
881883

@@ -910,58 +912,40 @@ object SpaceEngine {
910912
&& !sel.tpe.widen.isRef(defn.QuotedExprClass)
911913
&& !sel.tpe.widen.isRef(defn.QuotedTypeClass)
912914

913-
def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)") {
914-
val cases = m.cases.toIndexedSeq
915-
915+
def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)"):
916916
val selTyp = toUnderlying(m.selector.tpe).dealias
917-
918-
val isNullable = selTyp.classSymbol.isNullableClass
919-
val targetSpace = trace(i"targetSpace($selTyp)")(if isNullable
917+
val isNullable = selTyp.isInstanceOf[FlexibleType] || selTyp.classSymbol.isNullableClass
918+
val targetSpace = trace(i"targetSpace($selTyp)"):
919+
if isNullable && !ctx.mode.is(Mode.SafeNulls)
920920
then project(OrType(selTyp, ConstantType(Constant(null)), soft = false))
921921
else project(selTyp)
922-
)
923-
924-
var i = 0
925-
val len = cases.length
926-
var prevs = List.empty[Space]
927-
var deferred = List.empty[Tree]
928-
929-
while (i < len) {
930-
val CaseDef(pat, guard, _) = cases(i)
931922

932-
val curr = trace(i"project($pat)")(project(pat))
933-
934-
val covered = trace("covered")(simplify(intersect(curr, targetSpace)))
935-
936-
val prev = trace("prev")(simplify(Or(prevs)))
937-
938-
if prev == Empty && covered == Empty then // defer until a case is reachable
939-
deferred ::= pat
940-
else {
941-
for (pat <- deferred.reverseIterator)
942-
report.warning(MatchCaseUnreachable(), pat.srcPos)
943-
if pat != EmptyTree // rethrow case of catch uses EmptyTree
944-
&& !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
945-
&& isSubspace(covered, Or(List(prev, Typ(defn.NullType)))) // for when Null is not subtype of AnyRef under explicit nulls
946-
then {
947-
val nullOnly =
948-
(isNullable || (defn.NullType <:< selTyp))
949-
&& i == len - 1
950-
&& isWildcardArg(pat)
951-
if nullOnly then {
952-
report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
953-
} else if isSubspace(covered, prev) then {
954-
report.warning(MatchCaseUnreachable(), pat.srcPos)
955-
}
956-
}
957-
deferred = Nil
958-
}
959-
960-
// in redundancy check, take guard as false in order to soundly approximate
961-
prevs ::= (if guard.isEmpty then covered else Empty)
962-
i += 1
963-
}
964-
}
923+
@tailrec def recur(cases: List[CaseDef], prevs: List[Space], deferred: List[Tree]): Unit =
924+
cases match
925+
case Nil =>
926+
case CaseDef(pat, guard, _) :: rest =>
927+
val curr = trace(i"project($pat)")(project(pat))
928+
val covered = trace("covered")(simplify(intersect(curr, targetSpace)))
929+
val prev = trace("prev")(simplify(Or(prevs)))
930+
if prev == Empty && covered == Empty then // defer until a case is reachable
931+
recur(rest, prevs, pat :: deferred)
932+
else
933+
for pat <- deferred.reverseIterator
934+
do report.warning(MatchCaseUnreachable(), pat.srcPos)
935+
936+
if pat != EmptyTree // rethrow case of catch uses EmptyTree
937+
&& !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
938+
&& isSubspace(covered, prev)
939+
then
940+
val nullOnly = isNullable && rest.isEmpty && isWildcardArg(pat)
941+
val msg = if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
942+
report.warning(msg, pat.srcPos)
943+
944+
val newPrev = if guard.isEmpty then covered :: prevs else prevs
945+
recur(rest, newPrev, Nil)
946+
947+
recur(m.cases, Nil, Nil)
948+
end checkReachability
965949

966950
def checkMatch(m: Match)(using Context): Unit =
967951
if exhaustivityCheckable(m.selector) then checkExhaustivity(m)

‎tests/explicit-nulls/pos/interop-constructor.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Test that constructors have a non-nullab.e return type.
1+
// Test that constructors have a non-nullable return type.
22

33
class Foo {
44
val x: java.lang.String = new java.lang.String()

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,28 @@
11
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:5:9 --------------------------------------------
2-
5 | case _ => println(2) // warn
2+
5 | case _ => // warn
33
| ^
44
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
5-
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:11 ------------------------------------------
6-
12 | case _ => println(2) // warn
7-
| ^
8-
| 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:18:11 ------------------------------------------
10-
18 | case _ => println(2) // warn
11-
| ^
12-
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
5+
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:9 -------------------------------------------
6+
12 | case _ => // warn
7+
| ^
8+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
9+
-- [E030] Match case Unreachable Warning: tests/explicit-nulls/warn/i21577.scala:20:7 ----------------------------------
10+
20 | case _ => // warn
11+
| ^
12+
| Unreachable case
13+
-- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:29:27 -----------------------------
14+
29 |def f7(s: String | Null) = s match // warn
15+
| ^
16+
| match may not be exhaustive.
17+
|
18+
| It would fail on pattern case: _: Null
19+
|
20+
| longer explanation available when compiling with `-explain`
21+
-- [E029] Pattern Match Exhaustivity Warning: tests/explicit-nulls/warn/i21577.scala:36:33 -----------------------------
22+
36 |def f9(s: String | Int | Null) = s match // warn
23+
| ^
24+
| match may not be exhaustive.
25+
|
26+
| It would fail on pattern case: _: Int
27+
|
28+
| longer explanation available when compiling with `-explain`

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

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,38 @@
11
def f(s: String) =
22
val s2 = s.trim()
33
s2 match
4-
case s3: String => println(1)
5-
case _ => println(2) // warn
4+
case s3: String =>
5+
case _ => // warn
66

77

88
def f2(s: String | Null) =
99
val s2 = s.nn.trim()
10-
s2 match
11-
case s3: String => println(1)
12-
case _ => println(2) // warn
13-
14-
def f3(s: String | Null) =
15-
val s2 = s
16-
s2 match
17-
case s3: String => println(1)
18-
case _ => println(2) // warn
19-
20-
def f4(s: String | Int) =
21-
val s2 = s
22-
s2 match
23-
case s3: String => println(1)
24-
case _ => println(2)
10+
s2 match
11+
case s3: String =>
12+
case _ => // warn
13+
14+
def f3(s: String | Null) = s match
15+
case s2: String =>
16+
case _ =>
17+
18+
def f5(s: String) = s match
19+
case _: String =>
20+
case _ => // warn
21+
22+
def f6(s: String) = s.trim() match
23+
case _: String =>
24+
case null =>
25+
26+
def f61(s: String) = s.trim() match
27+
case _: String =>
28+
29+
def f7(s: String | Null) = s match // warn
30+
case _: String =>
31+
32+
def f8(s: String | Null) = s match
33+
case _: String =>
34+
case null =>
35+
36+
def f9(s: String | Int | Null) = s match // warn
37+
case _: String =>
38+
case null =>

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/interop/S.scala:8:11 ----------------------------------------
2+
8 | case _ => // warn
3+
| ^
4+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
5+
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/interop/S.scala:9:9 -----------------------------------------
6+
9 | case _ => println(2) // warn
7+
| ^
8+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).

‎tests/explicit-nulls/warn/interop/J.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import java.util.ArrayList;
2+
3+
class J {
4+
ArrayList<ArrayList<String>> foo(String x) { return null; }
5+
static String fooStatic(String x) { return null; }
6+
}

‎tests/explicit-nulls/warn/interop/S.scala‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import java.util.ArrayList
2+
def f() =
3+
val j = new J()
4+
val s2 = j.foo(null)
5+
s2 match
6+
case s3: ArrayList[ArrayList[String]] => s3.get(0) match
7+
case _: ArrayList[_] =>
8+
case _ => // warn
9+
case _ => println(2) // warn
10+

0 commit comments

Comments
 (0)