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 Dec 7, 2018
1 parent e9b50e7 commit 7bb7dcb
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 @@ -145,6 +145,7 @@ object PatternMatcher {
case class ReturnPlan(var label: TermSymbol) extends Plan
case class SeqPlan(var head: Plan, var tail: Plan) extends Plan
case class ResultPlan(var tree: Tree) extends Plan
case object NoOpPlan extends Plan

object TestPlan {
def apply(test: Test, sym: Symbol, pos: Position, ons: Plan): TestPlan =
Expand Down Expand Up @@ -338,9 +339,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)
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)
}
case Bind(name, body) =>
if (name == nme.WILDCARD) patternPlan(scrutinee, body, onSuccess)
else {
Expand Down Expand Up @@ -420,6 +431,7 @@ object PatternMatcher {
case plan: ReturnPlan => apply(plan)
case plan: SeqPlan => apply(plan)
case plan: ResultPlan => plan
case NoOpPlan => NoOpPlan
}
}

Expand Down Expand Up @@ -696,6 +708,7 @@ object PatternMatcher {
private def canFallThrough(plan: Plan): Boolean = plan match {
case _:ReturnPlan | _:ResultPlan => false
case _:TestPlan | _:LabeledPlan => true
case NoOpPlan => true
case LetPlan(_, body) => canFallThrough(body)
case SeqPlan(_, tail) => canFallThrough(tail)
}
Expand Down Expand Up @@ -854,6 +867,9 @@ object PatternMatcher {
case ResultPlan(tree) =>
if (tree.tpe <:< defn.NothingType) tree // For example MatchError
else Return(tree, ref(resultLabel))
case NoOpPlan =>
// TODO(abeln): optimize away
Literal(Constant(true))
}
}

Expand Down Expand Up @@ -892,6 +908,8 @@ object PatternMatcher {
showPlan(tail)
case ResultPlan(tree) =>
sb.append(tree.show)
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 7bb7dcb

Please sign in to comment.