Skip to content

Commit 6b26186

Browse files
committed
Fix #5067: Ycheck failure in pattern matching against a value of type Nothing
When desugaring pattern matching code for expressions where the matched value has type `Null` or `Nothing`, we used to generate code that's type-incorrect. Example: ``` val Some(x) = null ``` got desugared into ``` val x: Nothing = matchResult1[Nothing]: { case val x1: Null @unchecked = null: Null @unchecked if x1.ne(null) then { case val x: Nothing = x1.value.asInstanceOf[Nothing] return[matchResult1] x: Nothing } else () return[matchResult1] throw new MatchError(x1) } ``` There were two problems here: 1) `x1.ne(null)` 2) `x1.value` In both cases, we're trying to invoke methods that don't exist for type `Nothing` (and #2 doesn't exist for `Null`). This commit changes the desugaring so we generate a no-op for unapply when the value matched has type `Nothing` or `Null`. This works because the code we used to generate is never executed (because the `x1.ne(null)`) check.
1 parent fbcd879 commit 6b26186

File tree

5 files changed

+48
-4
lines changed

5 files changed

+48
-4
lines changed

compiler/src/dotty/tools/dotc/core/Types.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,12 @@ object Types {
233233
}
234234
}
235235

236+
/** Is this type exactly Null (no vars, aliases, refinements etc allowed)? */
237+
def isNullType(implicit ctx: Context): Boolean = this match {
238+
case tp: TypeRef => tp.symbol eq defn.NullClass
239+
case _ => false
240+
}
241+
236242
/** Is this type exactly Nothing (no vars, aliases, refinements etc allowed)? */
237243
def isBottomType(implicit ctx: Context): Boolean = this match {
238244
case tp: TypeRef => tp.symbol eq defn.NothingClass

compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ object PatternMatcher {
144144
case class ReturnPlan(var label: TermSymbol) extends Plan
145145
case class SeqPlan(var head: Plan, var tail: Plan) extends Plan
146146
case class ResultPlan(var tree: Tree) extends Plan
147+
case object NoOpPlan extends Plan
147148

148149
object TestPlan {
149150
def apply(test: Test, sym: Symbol, pos: Position, ons: Plan): TestPlan =
@@ -332,9 +333,19 @@ object PatternMatcher {
332333
val mt @ MethodType(_) = extractor.tpe.widen
333334
var unapp = extractor.appliedTo(ref(scrutinee).ensureConforms(mt.paramInfos.head))
334335
if (implicits.nonEmpty) unapp = unapp.appliedToArgs(implicits)
335-
val unappPlan = unapplyPlan(unapp, args)
336-
if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
337-
else TestPlan(NonNullTest, scrutinee, tree.pos, unappPlan)
336+
val scrutineeTpe = scrutinee.info.widenDealias
337+
if (scrutineeTpe.isBottomType || scrutineeTpe.isNullType) {
338+
// If the value being matched against has type `Nothing`, then the unapply code
339+
// will never execute.
340+
// If it has type `Null`, then the `NonNullTest` below guarantees that the unapply code
341+
// won't execute either.
342+
// So we don't need a plan in this case.
343+
NoOpPlan
344+
} else {
345+
val unappPlan = unapplyPlan(unapp, args)
346+
if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
347+
else TestPlan(NonNullTest, scrutinee, tree.pos, unappPlan)
348+
}
338349
case Bind(name, body) =>
339350
if (name == nme.WILDCARD) patternPlan(scrutinee, body, onSuccess)
340351
else {
@@ -407,13 +418,15 @@ object PatternMatcher {
407418
plan.tail = apply(plan.tail)
408419
plan
409420
}
421+
def applyNoOp: Plan = NoOpPlan
410422
def apply(plan: Plan): Plan = plan match {
411423
case plan: TestPlan => apply(plan)
412424
case plan: LetPlan => apply(plan)
413425
case plan: LabeledPlan => apply(plan)
414426
case plan: ReturnPlan => apply(plan)
415427
case plan: SeqPlan => apply(plan)
416428
case plan: ResultPlan => plan
429+
case NoOpPlan => applyNoOp
417430
}
418431
}
419432

@@ -662,7 +675,7 @@ object PatternMatcher {
662675
@tailrec
663676
private def canFallThrough(plan: Plan): Boolean = plan match {
664677
case _:ReturnPlan | _:ResultPlan => false
665-
case _:TestPlan | _:LabeledPlan => true
678+
case _:TestPlan | _:LabeledPlan | NoOpPlan => true
666679
case LetPlan(_, body) => canFallThrough(body)
667680
case SeqPlan(_, tail) => canFallThrough(tail)
668681
}
@@ -820,6 +833,9 @@ object PatternMatcher {
820833
}
821834
case ResultPlan(tree) =>
822835
Return(tree, ref(resultLabel))
836+
case NoOpPlan =>
837+
// TODO(abeln): optimize away
838+
Literal(Constant(true))
823839
}
824840
}
825841

@@ -858,6 +874,8 @@ object PatternMatcher {
858874
showPlan(tail)
859875
case ResultPlan(tree) =>
860876
sb.append(tree.show)
877+
case NoOpPlan =>
878+
sb.append("NoOp")
861879
}
862880
}
863881
showPlan(plan)

tests/pos/i5067.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
class Foo {
2+
val Some(_) = null
3+
val Some(_) = ???
4+
val (_, _) = null
5+
val (_, _, _) = ???
6+
??? match {
7+
case Some(_) => ()
8+
case (_, _, _) => ()
9+
}
10+
}

tests/run/i5067.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
matches null literal

tests/run/i5067.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
object Test {
2+
def main(args: Array[String]): Unit = {
3+
null match {
4+
case Some(_) => println("matches Some")
5+
case (_, _) => println("matches Pair")
6+
case null => println("matches null literal")
7+
}
8+
}
9+
}

0 commit comments

Comments
 (0)