From 368b041936d011c7a0704f553124dccd58831404 Mon Sep 17 00:00:00 2001 From: Abel Nieto Date: Mon, 3 Sep 2018 12:46:56 -0400 Subject: [PATCH] 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. --- .../src/dotty/tools/dotc/core/Types.scala | 6 ++++ .../tools/dotc/transform/PatternMatcher.scala | 24 ++++++++++++-- tests/pos/i5067.scala | 8 +++++ tests/run/i5067.check | 1 + tests/run/i5067b.check | 3 ++ tests/run/i5067b.scala | 33 +++++++++++++++++++ 6 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/pos/i5067.scala create mode 100644 tests/run/i5067.check create mode 100644 tests/run/i5067b.check create mode 100644 tests/run/i5067b.scala diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index e18b0de9ae3f..14196166b2ea 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index ebaca5b626b1..de44a96c240d 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -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 = @@ -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 { @@ -398,6 +409,7 @@ 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) @@ -405,6 +417,7 @@ object PatternMatcher { case plan: LabelledPlan => apply(plan) case plan: CallPlan => apply(plan) case plan: CodePlan => plan + case NoOpPlan => applyNoOp } } @@ -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)) } } @@ -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) diff --git a/tests/pos/i5067.scala b/tests/pos/i5067.scala new file mode 100644 index 000000000000..6128a9acaa27 --- /dev/null +++ b/tests/pos/i5067.scala @@ -0,0 +1,8 @@ +class Foo { + val Some(_) = ??? + val (_, _, _) = ??? + ??? match { + case Some(_) => () + case (_, _, _) => () + } +} diff --git a/tests/run/i5067.check b/tests/run/i5067.check new file mode 100644 index 000000000000..ee69e2a9b9f7 --- /dev/null +++ b/tests/run/i5067.check @@ -0,0 +1 @@ +matches null literal diff --git a/tests/run/i5067b.check b/tests/run/i5067b.check new file mode 100644 index 000000000000..be60186f7c25 --- /dev/null +++ b/tests/run/i5067b.check @@ -0,0 +1,3 @@ +match error +match error nested +not implemented error diff --git a/tests/run/i5067b.scala b/tests/run/i5067b.scala new file mode 100644 index 000000000000..36ff84478248 --- /dev/null +++ b/tests/run/i5067b.scala @@ -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") + } + } +}