Skip to content

Commit c397a5b

Browse files
liufengyunliufengyun
committed
address review
1 parent 199a5e4 commit c397a5b

File tree

6 files changed

+56
-34
lines changed

6 files changed

+56
-34
lines changed

‎compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -473,24 +473,21 @@ class PlainPrinter(_ctx: Context) extends Printer {
473473
("Scope{" ~ dclsText(sc.toList) ~ "}").close
474474

475475
def toText[T >: Untyped](tree: Tree[T]): Text = {
476-
tree match {
477-
case node: Positioned =>
478-
def toTextElem(elem: Any): Text = elem match {
479-
case elem: Showable => elem.toText(this)
480-
case elem: List[_] => "List(" ~ Text(elem map toTextElem, ",") ~ ")"
481-
case elem => elem.toString
482-
}
483-
val nodeName = node.productPrefix
484-
val elems =
485-
Text(node.productIterator.map(toTextElem).toList, ", ")
486-
val tpSuffix =
487-
if (ctx.settings.XprintTypes.value && tree.hasType)
488-
" | " ~ toText(tree.typeOpt)
489-
else
490-
Text()
491-
492-
nodeName ~ "(" ~ elems ~ tpSuffix ~ ")" ~ (Str(node.pos.toString) provided ctx.settings.YprintPos.value)
476+
def toTextElem(elem: Any): Text = elem match {
477+
case elem: Showable => elem.toText(this)
478+
case elem: List[_] => "List(" ~ Text(elem map toTextElem, ",") ~ ")"
479+
case elem => elem.toString
493480
}
481+
val nodeName = tree.productPrefix
482+
val elems =
483+
Text(tree.productIterator.map(toTextElem).toList, ", ")
484+
val tpSuffix =
485+
if (ctx.settings.XprintTypes.value && tree.hasType)
486+
" | " ~ toText(tree.typeOpt)
487+
else
488+
Text()
489+
490+
nodeName ~ "(" ~ elems ~ tpSuffix ~ ")" ~ (Str(tree.pos.toString) provided ctx.settings.YprintPos.value)
494491
}.close // todo: override in refined printer
495492

496493
def toText(result: SearchResult): Text = result match {

‎compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ public enum ErrorMessageID {
128128
ParamsNoInlineID,
129129
JavaSymbolIsNotAValueID,
130130
DoubleDeclarationID,
131+
MatchCaseOnlyNullWarningID,
131132
;
132133

133134
public int errorNumber() {

‎compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,11 +909,18 @@ object messages {
909909

910910
case class MatchCaseUnreachable()(implicit ctx: Context)
911911
extends Message(MatchCaseUnreachableID) {
912-
val kind = s"""Match ${hl"case"} Unreachable"""
912+
val kind = s"""Match case Unreachable"""
913913
val msg = "unreachable code"
914914
val explanation = ""
915915
}
916916

917+
case class MatchCaseOnlyNullWarning()(implicit ctx: Context)
918+
extends Message(MatchCaseOnlyNullWarningID) {
919+
val kind = s"""Only null matched"""
920+
val msg = s"Only ${hl"null"} is matched. Consider use `case null =>` instead."
921+
val explanation = ""
922+
}
923+
917924
case class SeqWildcardPatternPos()(implicit ctx: Context)
918925
extends Message(SeqWildcardPatternPosID) {
919926
val kind = "Syntax"

‎compiler/src/dotty/tools/dotc/transform/patmat/Space.scala‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
288288
private val scalaNilType = ctx.requiredModuleRef("scala.collection.immutable.Nil")
289289
private val scalaConsType = ctx.requiredClassRef("scala.collection.immutable.::")
290290

291+
private val nullType = ConstantType(Constant(null))
292+
private val nullSpace = Typ(nullType)
293+
291294
override def intersectUnrelatedAtomicTypes(tp1: Type, tp2: Type) = {
292295
val and = AndType(tp1, tp2)
293296
// Precondition: !(tp1 <:< tp2) && !(tp2 <:< tp1)
@@ -316,7 +319,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
316319
Typ(ConstantType(c), false)
317320
case _: BackquotedIdent => Typ(pat.tpe, false)
318321
case Ident(nme.WILDCARD) =>
319-
Or(Typ(pat.tpe.stripAnnots, false) :: Typ(ConstantType(Constant(null))) :: Nil)
322+
Or(Typ(pat.tpe.stripAnnots, false) :: nullSpace :: Nil)
320323
case Ident(_) | Select(_, _) =>
321324
Typ(pat.tpe.stripAnnots, false)
322325
case Alternative(trees) => Or(trees.map(project(_)))
@@ -376,7 +379,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
376379

377380
/** Is `tp1` a subtype of `tp2`? */
378381
def isSubType(tp1: Type, tp2: Type): Boolean = {
379-
val res = tp1 <:< tp2 && (tp1 != ConstantType(Constant(null)) || tp2 == ConstantType(Constant(null)))
382+
val res = tp1 <:< tp2 && (tp1 != nullType || tp2 == nullType)
380383
debug.println(s"${tp1.show} <:< ${tp2.show} = $res")
381384
res
382385
}
@@ -909,21 +912,22 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
909912
if (selTyp.classSymbol.isPrimitiveValueClass)
910913
Typ(selTyp, true)
911914
else
912-
Or(Typ(selTyp, true) :: Typ(ConstantType(Constant(null))) :: Nil)
915+
Or(Typ(selTyp, true) :: nullSpace :: Nil)
913916

914-
(0 until cases.length).foreach { i =>
915-
// in redundancy check, take guard as false in order to soundly approximate
916-
val prevs =
917-
if (i == 0)
918-
Empty
919-
else
920-
cases.take(i).map { x =>
921-
if (x.guard.isEmpty) project(x.pat)
922-
else Empty
923-
}.reduce((a, b) => Or(List(a, b)))
917+
// in redundancy check, take guard as false in order to soundly approximate
918+
def projectPrevCases(cases: List[CaseDef]): Space =
919+
cases.map { x =>
920+
if (x.guard.isEmpty) project(x.pat)
921+
else Empty
922+
}.reduce((a, b) => Or(List(a, b)))
923+
924+
(1 until cases.length).foreach { i =>
925+
val prevs = projectPrevCases(cases.take(i))
926+
927+
val pat = cases(i).pat
924928

925-
if (cases(i).pat != EmptyTree) { // rethrow case of catch uses EmptyTree
926-
val curr = project(cases(i).pat)
929+
if (pat != EmptyTree) { // rethrow case of catch uses EmptyTree
930+
val curr = project(pat)
927931

928932
debug.println(s"---------------reachable? ${show(curr)}")
929933
debug.println(s"prev: ${show(prevs)}")
@@ -936,7 +940,17 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
936940
if (covered == Empty) covered = curr
937941

938942
if (isSubspace(covered, prevs)) {
939-
ctx.warning(MatchCaseUnreachable(), cases(i).body.pos)
943+
ctx.warning(MatchCaseUnreachable(), pat.pos)
944+
}
945+
946+
// if last case is `_` and only matches `null`, produce a warning
947+
if (i == cases.length - 1) {
948+
simplify(minus(covered, prevs)) match {
949+
case Typ(ConstantType(Constant(null)), _) =>
950+
ctx.warning(MatchCaseOnlyNullWarning(), pat.pos)
951+
case _ =>
952+
}
953+
940954
}
941955
}
942956
}

‎tests/patmat/i4225b.check‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
10: Only null matched

‎tests/patmat/null.check‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
6: Match case Unreachable
2+
13: Only null matched
23
18: Match case Unreachable
4+
20: Only null matched

0 commit comments

Comments
 (0)