Skip to content

Commit

Permalink
Fix scala#5067: Ycheck failure in pattern matching against a value of…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
abeln committed Sep 4, 2018
1 parent 6e79931 commit 368b041
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 3 deletions.
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,12 @@ object Types {
}
}

/** Is this type exactly Null (no vars, aliases, refinements etc allowed)? */
def isNullType(implicit ctx: Context): Boolean = this match {
case tp: TypeRef => tp.symbol eq defn.NullClass
case _ => false
}

/** Is this type exactly Nothing (no vars, aliases, refinements etc allowed)? */
def isBottomType(implicit ctx: Context): Boolean = this match {
case tp: TypeRef => tp.symbol eq defn.NothingClass
Expand Down
24 changes: 21 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ object PatternMatcher {
case class CodePlan(var tree: Tree) extends Plan
case class CallPlan(label: TermSymbol,
var args: List[(/*formal*/TermSymbol, /*actual*/TermSymbol)]) extends Plan
case object NoOpPlan extends Plan

object TestPlan {
def apply(test: Test, sym: Symbol, pos: Position, ons: Plan, onf: Plan): TestPlan =
Expand Down Expand Up @@ -332,9 +333,19 @@ object PatternMatcher {
val mt @ MethodType(_) = extractor.tpe.widen
var unapp = extractor.appliedTo(ref(scrutinee).ensureConforms(mt.paramInfos.head))
if (implicits.nonEmpty) unapp = unapp.appliedToArgs(implicits)
val unappPlan = unapplyPlan(unapp, args)
if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
else TestPlan(NonNullTest, scrutinee, tree.pos, unappPlan, onFailure)
val scrutineeTpe = scrutinee.info.widenDealias
if (scrutineeTpe.isBottomType || scrutineeTpe.isNullType) {
// If the value being matched against has type `Nothing`, then the unapply code
// will never execute.
// If it has type `Null`, then the `NonNullTest` below guarantees that the unapply code
// won't execute either.
// So we don't need a plan in this case.
NoOpPlan
} else {
val unappPlan = unapplyPlan(unapp, args)
if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
else TestPlan(NonNullTest, scrutinee, tree.pos, unappPlan, onFailure)
}
case Bind(name, body) =>
if (name == nme.WILDCARD) patternPlan(scrutinee, body, onSuccess, onFailure)
else {
Expand Down Expand Up @@ -398,13 +409,15 @@ object PatternMatcher {
labelled(plan.sym) = apply(labelled(plan.sym))
plan
}
def applyNoOp: Plan = NoOpPlan
def apply(plan: CallPlan): Plan = plan
def apply(plan: Plan): Plan = plan match {
case plan: TestPlan => apply(plan)
case plan: LetPlan => apply(plan)
case plan: LabelledPlan => apply(plan)
case plan: CallPlan => apply(plan)
case plan: CodePlan => plan
case NoOpPlan => applyNoOp
}
}

Expand Down Expand Up @@ -918,6 +931,9 @@ object PatternMatcher {
tree
case CallPlan(label, args) =>
ref(label).appliedToArgs(args.map { case (_, actual) => ref(actual) })
case NoOpPlan =>
// TODO(abeln): optimize away
Literal(Constant(true))
}
}

Expand Down Expand Up @@ -957,6 +973,8 @@ object PatternMatcher {
sb.append(tree.show)
case CallPlan(label, params) =>
sb.append(s"Call($label(${params.map(_._2)}%, %)")
case NoOpPlan =>
sb.append("NoOp")
}
}
showPlan(plan)
Expand Down
8 changes: 8 additions & 0 deletions tests/pos/i5067.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class Foo {
val Some(_) = ???
val (_, _, _) = ???
??? match {
case Some(_) => ()
case (_, _, _) => ()
}
}
1 change: 1 addition & 0 deletions tests/run/i5067.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
matches null literal
3 changes: 3 additions & 0 deletions tests/run/i5067b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
match error
match error nested
not implemented error
33 changes: 33 additions & 0 deletions tests/run/i5067b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
object Test {
def main(args: Array[String]): Unit = {
class B[T] {}
object B {
def unapply[T](x: Any): Option[B[T]] = None
}
try {
val B(_) = null
} catch {
case e: MatchError => println("match error")
}

null match {
case null =>
try {
null match {
case Some(_) => ()
}
} catch {
case e: MatchError => println("match error nested")
}
}

try {
??? match {
case (_, _) => ()
case _ => ()
}
} catch {
case e: NotImplementedError => println("not implemented error")
}
}
}

0 comments on commit 368b041

Please sign in to comment.