Skip to content

Commit 0d3aea1

Browse files
committed
Better handling of comparisons with null in pattern matcher
To desugar constructs like ``` val Some(_) = null ``` this uses the approach in scala#5075 as opposed to the older approach that used NoOpPlans.
1 parent 3d97b04 commit 0d3aea1

File tree

2 files changed

+19
-25
lines changed

2 files changed

+19
-25
lines changed

compiler/src/dotty/tools/dotc/ast/tpd.scala

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -889,9 +889,15 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
889889
else Erasure.Boxing.adaptToType(tree, tp)
890890

891891
/** `tree ne null` (might need a cast to be type correct) */
892-
def testNotNull(implicit ctx: Context): Tree =
893-
tree.ensureConforms(defn.ObjectType)
894-
.select(defn.Object_ne).appliedTo(Literal(Constant(null)))
892+
def testNotNull(implicit ctx: Context): Tree = {
893+
val receiver = if (defn.isBottomType(tree.tpe)) {
894+
// If the receiver is of type `Nothing` or `Null`, add an ascription so that the selection
895+
// succeeds: e.g. `null.ne(null)` doesn't type, but `(null: AnyRef).ne(null)` does.
896+
Typed(tree, TypeTree(defn.AnyRefType))
897+
}
898+
else tree.ensureConforms(defn.ObjectType)
899+
receiver.select(defn.Object_ne).appliedTo(Literal(Constant(null)))
900+
}
895901

896902
/** If inititializer tree is `_', the default value of its type,
897903
* otherwise the tree itself.

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

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

150149
object TestPlan {
151150
def apply(test: Test, sym: Symbol, span: Span, ons: Plan): TestPlan =
@@ -336,22 +335,18 @@ object PatternMatcher {
336335
patternPlan(casted, pat, onSuccess)
337336
})
338337
case UnApply(extractor, implicits, args) =>
339-
val mt @ MethodType(_) = extractor.tpe.widen
340-
var unapp = extractor.appliedTo(ref(scrutinee).ensureConforms(mt.paramInfos.head))
341-
if (implicits.nonEmpty) unapp = unapp.appliedToArgs(implicits)
342-
val scrutineeTpe = scrutinee.info.widenDealias
343-
if (scrutineeTpe.isBottomType || scrutineeTpe.isNullType) {
344-
// If the value being matched against has type `Nothing`, then the unapply code
345-
// will never execute.
346-
// If it has type `Null`, then the `NonNullTest` below guarantees that the unapply code
347-
// won't execute either.
348-
// So we don't need a plan in this case.
349-
NoOpPlan
338+
val unappPlan = if (defn.isBottomType(scrutinee.info)) {
339+
// Generate a throwaway but type-correct plan.
340+
// This plan will never execute because it'll be guarded by a `NonNullTest`.
341+
ResultPlan(tpd.Throw(tpd.Literal(Constant(null))))
350342
} else {
351-
val unappPlan = unapplyPlan(unapp, args)
352-
if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
353-
else TestPlan(NonNullTest, scrutinee, tree.span, unappPlan)
343+
val mt @ MethodType(_) = extractor.tpe.widen
344+
var unapp = extractor.appliedTo(ref(scrutinee).ensureConforms(mt.paramInfos.head))
345+
if (implicits.nonEmpty) unapp = unapp.appliedToArgs(implicits)
346+
unapplyPlan(unapp, args)
354347
}
348+
if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
349+
else TestPlan(NonNullTest, scrutinee, tree.span, unappPlan)
355350
case Bind(name, body) =>
356351
if (name == nme.WILDCARD) patternPlan(scrutinee, body, onSuccess)
357352
else {
@@ -431,7 +426,6 @@ object PatternMatcher {
431426
case plan: ReturnPlan => apply(plan)
432427
case plan: SeqPlan => apply(plan)
433428
case plan: ResultPlan => plan
434-
case NoOpPlan => NoOpPlan
435429
}
436430
}
437431

@@ -708,7 +702,6 @@ object PatternMatcher {
708702
private def canFallThrough(plan: Plan): Boolean = plan match {
709703
case _:ReturnPlan | _:ResultPlan => false
710704
case _:TestPlan | _:LabeledPlan => true
711-
case NoOpPlan => true
712705
case LetPlan(_, body) => canFallThrough(body)
713706
case SeqPlan(_, tail) => canFallThrough(tail)
714707
}
@@ -872,9 +865,6 @@ object PatternMatcher {
872865
case ResultPlan(tree) =>
873866
if (tree.tpe <:< defn.NothingType) tree // For example MatchError
874867
else Return(tree, ref(resultLabel))
875-
case NoOpPlan =>
876-
// TODO(abeln): optimize away
877-
Literal(Constant(true))
878868
}
879869
}
880870

@@ -913,8 +903,6 @@ object PatternMatcher {
913903
showPlan(tail)
914904
case ResultPlan(tree) =>
915905
sb.append(tree.show)
916-
case NoOpPlan =>
917-
sb.append("NoOp")
918906
}
919907
}
920908
showPlan(plan)

0 commit comments

Comments
 (0)