Skip to content

Commit 6946371

Browse files
abelnabeln
committed
Adapt pattern matching checks to explicit nulls
In the pattern matching checks, we used to assume that types are implicitly nullable. This showed up when checking whether a pattern is redundant/exhaustive. Change the logic so that types are explicitly nullable.
1 parent 53c759d commit 6946371

File tree

5 files changed

+133
-22
lines changed

5 files changed

+133
-22
lines changed

‎compiler/src/dotty/tools/dotc/transform/SyntheticMethods.scala

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,13 @@ class SyntheticMethods(thisPhase: DenotTransformer) {
120120
val ioob = defn.IndexOutOfBoundsException.typeRef
121121
// Second constructor of ioob that takes a String argument
122122
def filterStringConstructor(s: Symbol): Boolean = s.info match {
123-
case m: MethodType if s.isConstructor => m.paramInfos == List(defn.StringType)
123+
case m: MethodType if s.isConstructor && m.paramInfos.size == 1 =>
124+
val pinfo = if (ctx.settings.YexplicitNulls.value) {
125+
m.paramInfos.head.stripJavaNull
126+
} else {
127+
m.paramInfos.head
128+
}
129+
pinfo == defn.StringType
124130
case _ => false
125131
}
126132
val constructor = ioob.typeSymbol.info.decls.find(filterStringConstructor _).asTerm

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

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -290,12 +290,27 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
290290
private val scalaNilType = ctx.requiredModuleRef("scala.collection.immutable.Nil")
291291
private val scalaConsType = ctx.requiredClassRef("scala.collection.immutable.::")
292292

293-
private val nullType = ConstantType(Constant(null))
294-
private val nullSpace = Typ(nullType)
293+
private val constantNullType = ConstantType(Constant(null))
294+
private val constantNullSpace = Typ(constantNullType)
295+
296+
/** Does the given tree stand for the literal `null`? */
297+
def isNullLit(tree: Tree): Boolean = tree match {
298+
case Literal(Constant(null)) => true
299+
case _ => false
300+
}
301+
302+
/** Does the given space contain just the value `null`? */
303+
def isNullSpace(space: Space): Boolean = space match {
304+
case Typ(tpe, _) => tpe =:= constantNullType || tpe.isNullType
305+
case Or(spaces) => spaces.forall(isNullSpace)
306+
case _ => false
307+
}
295308

296309
override def intersectUnrelatedAtomicTypes(tp1: Type, tp2: Type): Space = {
297-
// Precondition: !isSubType(tp1, tp2) && !isSubType(tp2, tp1)
298-
if (tp1 == nullType || tp2 == nullType) {
310+
// Precondition: !isSubType(tp1, tp2) && !isSubType(tp2, tp1).
311+
if (ctx.settings.YexplicitNulls.value) {
312+
// No additional checks required: since subtyping already reflects nullability.
313+
} else if (tp1.isNullType || tp2.isNullType) {
299314
// Since projections of types don't include null, intersection with null is empty.
300315
return Empty
301316
}
@@ -327,7 +342,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
327342
Typ(ConstantType(c), false)
328343
case _: BackquotedIdent => Typ(pat.tpe, false)
329344
case Ident(nme.WILDCARD) =>
330-
Or(Typ(pat.tpe.stripAnnots, false) :: nullSpace :: Nil)
345+
Or(Typ(pat.tpe.stripAnnots, false) :: constantNullSpace :: Nil)
331346
case Ident(_) | Select(_, _) =>
332347
Typ(pat.tpe.stripAnnots, false)
333348
case Alternative(trees) => Or(trees.map(project(_)))
@@ -406,7 +421,11 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
406421
/** Is `tp1` a subtype of `tp2`? */
407422
def isSubType(tp1: Type, tp2: Type): Boolean = {
408423
debug.println(TypeComparer.explained(implicit ctx => tp1 <:< tp2))
409-
val res = (tp1 != nullType || tp2 == nullType) && tp1 <:< tp2
424+
val res = if (ctx.settings.YexplicitNulls.value) {
425+
tp1 <:< tp2
426+
} else {
427+
(tp1 != constantNullType || tp2 == constantNullType) && tp1 <:< tp2
428+
}
410429
res
411430
}
412431

@@ -630,7 +649,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
630649

631650
def doShow(s: Space, mergeList: Boolean = false): String = s match {
632651
case Empty => ""
633-
case Typ(c: ConstantType, _) => c.value.value.toString
652+
case Typ(c: ConstantType, _) =>
653+
val v = c.value.value
654+
if (v == null) "null" else v.toString
634655
case Typ(tp: TermRef, _) => tp.symbol.showName
635656
case Typ(tp, decomposed) =>
636657
val sym = tp.widen.classSymbol
@@ -731,11 +752,14 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
731752

732753
if (!redundancyCheckable(sel)) return
733754

734-
val targetSpace =
755+
val targetSpace = if (ctx.settings.YexplicitNulls.value) {
756+
Typ(selTyp, true)
757+
} else {
735758
if (selTyp.classSymbol.isPrimitiveValueClass)
736759
Typ(selTyp, true)
737760
else
738-
Or(Typ(selTyp, true) :: nullSpace :: Nil)
761+
Or(Typ(selTyp, true) :: constantNullSpace :: Nil)
762+
}
739763

740764
// in redundancy check, take guard as false in order to soundly approximate
741765
def projectPrevCases(cases: List[CaseDef]): Space =
@@ -744,11 +768,6 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
744768
else Empty
745769
}.reduce((a, b) => Or(List(a, b)))
746770

747-
def isNull(tree: Tree): Boolean = tree match {
748-
case Literal(Constant(null)) => true
749-
case _ => false
750-
}
751-
752771
(1 until cases.length).foreach { i =>
753772
val prevs = projectPrevCases(cases.take(i))
754773

@@ -765,18 +784,27 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
765784

766785
// `covered == Empty` may happen for primitive types with auto-conversion
767786
// see tests/patmat/reader.scala tests/patmat/byte.scala
768-
if (covered == Empty) covered = curr
787+
if (covered == Empty && !isNullLit(pat)) covered = curr
769788

770789
if (isSubspace(covered, prevs)) {
771790
ctx.warning(MatchCaseUnreachable(), pat.sourcePos)
772791
}
773792

774793
// if last case is `_` and only matches `null`, produce a warning
775-
if (i == cases.length - 1 && !isNull(pat) ) {
776-
simplify(minus(covered, prevs)) match {
777-
case Typ(`nullType`, _) =>
778-
ctx.warning(MatchCaseOnlyNullWarning(), pat.sourcePos)
779-
case _ =>
794+
if (i == cases.length - 1 && !isNullLit(pat) ) {
795+
val simpl = simplify(minus(covered, prevs))
796+
if (ctx.settings.YexplicitNulls.value) {
797+
simpl match {
798+
case space if isNullSpace(space) =>
799+
ctx.warning(MatchCaseOnlyNullWarning(), pat.sourcePos)
800+
case _ =>
801+
}
802+
} else {
803+
simpl match {
804+
case Typ(`constantNullType`, _) =>
805+
ctx.warning(MatchCaseOnlyNullWarning(), pat.sourcePos)
806+
case _ =>
807+
}
780808
}
781809
}
782810
}

‎compiler/test/dotty/tools/dotc/CompilationTests.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,8 @@ class CompilationTests extends ParallelTesting {
277277
// Explicit nulls tests
278278
@Test def explicitNullsNeg: Unit = {
279279
implicit val testGroup: TestGroup = TestGroup("explicitNullsNeg")
280-
compileFilesInDir("tests/explicit-nulls/neg", explicitNullsOptions)
280+
compileFilesInDir("tests/explicit-nulls/neg", explicitNullsOptions) +
281+
compileFilesInDir("tests/explicit-nulls/neg-patmat", explicitNullsOptions and "-Xfatal-warnings")
281282
}.checkExpectedErrors()
282283

283284
@Test def explicitNullsPos: Unit = {

‎tests/explicit-nulls/neg-patmat/patmat1.scala

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
2+
class Foo {
3+
val s: String = ???
4+
s match {
5+
case s: String => 100 // warning: type test will always succeed
6+
case _ => 200 // error: unreachable
7+
}
8+
9+
s match {
10+
case s: String => 100 // warning: type test will always succeed
11+
case _ => 200 // error: unreachable
12+
}
13+
14+
sealed trait Animal
15+
case class Dog(name: String) extends Animal
16+
case object Cat extends Animal
17+
18+
val a: Animal = ???
19+
a match {
20+
case Dog(name) => 100
21+
case Cat => 200
22+
case _ => 300 // error: unreachable
23+
}
24+
25+
val a2: Animal|Null = ???
26+
a2 match {
27+
case Dog(_) => 100
28+
case Cat => 200
29+
case _ => 300 // error: only matches null
30+
}
31+
32+
val a3: Animal|Null = ???
33+
a3 match {
34+
case Dog(_) => 100
35+
case Cat => 200
36+
case null => 300 // ok
37+
}
38+
}

‎tests/explicit-nulls/pos/pattern-matching.scala

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
2+
class Foo {
3+
val s: String = ???
4+
s match {
5+
case s: String => 100 // warning: type test will always succeed
6+
case _ => 200 // warning: unreachable
7+
}
8+
9+
s match {
10+
case s: String => 100 // warning: type test will always succeed
11+
case _ => 200 // warning: unreachable
12+
}
13+
14+
sealed trait Animal
15+
case class Dog(name: String) extends Animal
16+
case object Cat extends Animal
17+
18+
val a: Animal = ???
19+
a match {
20+
case Dog(name) => 100
21+
case Cat => 200
22+
case _ => 300 // warning: unreachable
23+
}
24+
25+
val a2: Animal|Null = ???
26+
a2 match {
27+
case Dog(_) => 100
28+
case Cat => 200
29+
case _ => 300 // warning: only matches null
30+
}
31+
32+
val a3: Animal|Null = ???
33+
a3 match {
34+
case Dog(_) => 100
35+
case Cat => 200
36+
case null => 300 // ok
37+
}
38+
}

0 commit comments

Comments
 (0)