From 1becce1c05016b43eedf7f9a8fbbbe057acc361d Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 16 Sep 2020 13:58:49 +0100 Subject: [PATCH] Exhaustivity: don't widen & then warn on singleton types --- .../scala/tools/nsc/transform/CleanUp.scala | 21 +++++++++++++----- .../transform/patmat/MatchTranslation.scala | 4 ++-- test/files/pos/t11603.scala | 6 +++++ .../run/blame_eye_triple_eee-double.check | 3 +++ .../run/blame_eye_triple_eee-float.check | 3 +++ test/files/run/patmat-seq.check | 8 +++---- test/files/run/t6288.check | 22 +++++++++---------- test/files/run/t7039.check | 3 +++ test/files/run/virtpatmat_literal.check | 3 +++ 9 files changed, 50 insertions(+), 23 deletions(-) create mode 100644 test/files/pos/t11603.scala diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala index 452020a21a28..13e802c9f94d 100644 --- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala +++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala @@ -398,12 +398,15 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL { // transform scrutinee of all matches to ints def transformSwitch(sw: Match): Tree = { import CODE._ - sw.selector.tpe match { + sw.selector.tpe.widen match { case IntTpe => sw // can switch directly on ints case StringTpe => // these assumptions about the shape of the tree are justified by the codegen in MatchOptimization - val Match(Typed(selTree: Ident, _), cases) = sw - val sel = selTree.symbol + val Match(Typed(selTree, _), cases) = sw + def selArg = selTree match { + case x: Ident => REF(x.symbol) + case x: Literal => x + } val restpe = sw.tpe val swPos = sw.pos.focus @@ -429,7 +432,7 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL { */ val stats = mutable.ListBuffer.empty[Tree] - var failureBody = Throw(New(definitions.MatchErrorClass.tpe_*, REF(sel))) : Tree + var failureBody = Throw(New(definitions.MatchErrorClass.tpe_*, selArg)) : Tree // genbcode isn't thrilled about seeing labels with Unit arguments, so `success`'s type is one of // `${sw.tpe} => ${sw.tpe}` or `() => Unit` depending. @@ -447,7 +450,13 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL { val failure = currentOwner.newLabel(unit.freshTermName("matchEnd"), swPos).setInfo(MethodType(Nil, restpe)) def fail(): Tree = atPos(swPos) { Apply(REF(failure), Nil) } - val newSel = atPos(sel.pos) { IF (sel OBJ_EQ NULL) THEN LIT(0) ELSE (Apply(REF(sel) DOT Object_hashCode, Nil)) } + val ifNull = LIT(0) + val noNull = Apply(selArg DOT Object_hashCode, Nil) + + val newSel = selTree match { + case _: Ident => atPos(selTree.symbol.pos) { IF(selTree.symbol OBJ_EQ NULL) THEN ifNull ELSE noNull } + case x: Literal => atPos(selTree.pos) { if (x.value.value == null) ifNull else noNull } + } val casesByHash = cases.flatMap { case cd@CaseDef(StringsPattern(strs), _, body) => @@ -465,7 +474,7 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL { case (next, (pat, jump, pos)) => val comparison = if (pat == null) Object_eq else Object_equals atPos(pos) { - IF(LIT(pat) DOT comparison APPLY REF(sel)) THEN (REF(jump) APPLY Nil) ELSE next + IF(LIT(pat) DOT comparison APPLY selArg) THEN (REF(jump) APPLY Nil) ELSE next } } CaseDef(LIT(hash), EmptyTree, newBody) diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala index 3c6ecef35a4d..0d2d9c53e08e 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala @@ -67,7 +67,7 @@ trait MatchTranslation { private lazy val extractor = ExtractorCall(tree) def pos = tree.pos - def tpe = binder.info.dealiasWiden // the type of the variable bound to the pattern + def tpe = binder.info.dealias // the type of the variable bound to the pattern def pt = unbound match { case Star(tpt) => this glbWith seqType(tpt.tpe) case TypeBound(tpe) => tpe @@ -217,7 +217,7 @@ trait MatchTranslation { val start = if (StatisticsStatics.areSomeColdStatsEnabled) statistics.startTimer(statistics.patmatNanos) else null - val selectorTp = repeatedToSeq(elimAnonymousClass(selector.tpe.widen.withoutAnnotations)) + val selectorTp = repeatedToSeq(elimAnonymousClass(selector.tpe.withoutAnnotations)) // when one of the internal cps-type-state annotations is present, strip all CPS annotations val origPt = removeCPSFromPt(match_.tpe) diff --git a/test/files/pos/t11603.scala b/test/files/pos/t11603.scala new file mode 100644 index 000000000000..01a59a532bac --- /dev/null +++ b/test/files/pos/t11603.scala @@ -0,0 +1,6 @@ +// scalac: -Werror +class C { + def m(x: true) = x match { + case true => println("the one true path") + } +} diff --git a/test/files/run/blame_eye_triple_eee-double.check b/test/files/run/blame_eye_triple_eee-double.check index 53eac99ecd18..ebe949834aed 100644 --- a/test/files/run/blame_eye_triple_eee-double.check +++ b/test/files/run/blame_eye_triple_eee-double.check @@ -1,3 +1,6 @@ +blame_eye_triple_eee-double.scala:49: warning: unreachable code + case _ => println("NaN matching was good") + ^ if (NaN == NaN) is good if (x == x) is good if (x == NaN) is good diff --git a/test/files/run/blame_eye_triple_eee-float.check b/test/files/run/blame_eye_triple_eee-float.check index 53eac99ecd18..6ba063a13990 100644 --- a/test/files/run/blame_eye_triple_eee-float.check +++ b/test/files/run/blame_eye_triple_eee-float.check @@ -1,3 +1,6 @@ +blame_eye_triple_eee-float.scala:49: warning: unreachable code + case _ => println("NaN matching was good") + ^ if (NaN == NaN) is good if (x == x) is good if (x == NaN) is good diff --git a/test/files/run/patmat-seq.check b/test/files/run/patmat-seq.check index a2c779203a61..1a95226a06fb 100644 --- a/test/files/run/patmat-seq.check +++ b/test/files/run/patmat-seq.check @@ -63,7 +63,7 @@ package { () }; def t: Any = { - case val x1: Int = 2; + case val x1: Int(2) = 2; case16(){ val o18: collection.SeqFactory.UnapplySeqWrapper[Int] = A.unapplySeq(x1); if (o18.isEmpty.unary_!) @@ -193,7 +193,7 @@ package { case39() }; case39(){ - matchEnd15(throw new MatchError(x1)) + matchEnd15(throw new MatchError(2)) }; matchEnd15(x: Any){ x @@ -264,7 +264,7 @@ package { () }; def t(): Object = { - case val x1: Int = 2; + case val x1: Int(2) = 2; case16(){ val o18: scala.collection.SeqOps = A.unapplySeq(x1); if (scala.collection.SeqFactory.UnapplySeqWrapper.isEmpty$extension(o18).unary_!()) @@ -394,7 +394,7 @@ package { case39() }; case39(){ - matchEnd15(throw new MatchError(scala.Int.box(x1))) + matchEnd15(throw new MatchError(scala.Int.box(2))) }; matchEnd15(x: Object){ x diff --git a/test/files/run/t6288.check b/test/files/run/t6288.check index 021a1d9c89c0..ca377e92af68 100644 --- a/test/files/run/t6288.check +++ b/test/files/run/t6288.check @@ -7,9 +7,9 @@ }; [17:60]def unapply([29:35]z: [32:35]): [21]Option[Int] = [52:60][52:56][52:56]new [52:56]Some[Int]([57:59]-1); [64:66]{ - [64:66]case val x1: [64]String = [64:66]""; + [64:66]case val x1: [64]String("") = [64:66]""; [64:66]case5(){ - [84:93]if ([89][89]x1.ne([89]null)) + [84:93]if ([89][89][89]"".ne([89]null)) [84:89]{ [84:89] val o7: [84]Option[Int] = [84:89][84:89]Case3.unapply([84]x1); [97:99]if ([84]o7.isEmpty.unary_!) @@ -21,7 +21,7 @@ [89][89]case6() }; [64:66]case6(){ - [64:66][64:66]matchEnd4([64:66]throw [64:66][64:66][64:66]new [64:66]MatchError([64:66]x1)) + [64:66][64:66]matchEnd4([64:66]throw [64:66][64:66][64:66]new [64:66]MatchError([64:66]"")) }; [64:66]matchEnd4(x: [NoPosition]Unit){ [64:66]x @@ -35,9 +35,9 @@ }; [123:171]def unapplySeq([138:144]z: [141:144]): [127]Option[List[Int]] = [167:171]scala.None; [175:177]{ - [175:177]case val x1: [175]String = [175:177]""; + [175:177]case val x1: [175]String("") = [175:177]""; [175:177]case5(){ - [195:204]if ([200][200]x1.ne([200]null)) + [195:204]if ([200][200][200]"".ne([200]null)) [195:200]{ [195:200] val o7: [195]Option[List[Int]] = [195:200][195:200]Case4.unapplySeq([195]x1); [208:210]if ([195][195]o7.isEmpty.unary_!.&&([195][195][195][195]o7.get.!=([195]null).&&([195][195][195][195]o7.get.lengthCompare([195]1).==([195]0)))) @@ -49,7 +49,7 @@ [200][200]case6() }; [175:177]case6(){ - [175:177][175:177]matchEnd4([175:177]throw [175:177][175:177][175:177]new [175:177]MatchError([175:177]x1)) + [175:177][175:177]matchEnd4([175:177]throw [175:177][175:177][175:177]new [175:177]MatchError([175:177]"")) }; [175:177]matchEnd4(x: [NoPosition]Unit){ [175:177]x @@ -63,9 +63,9 @@ }; [234:269]def unapply([246:252]z: [249:252]): [238]Boolean = [265:269]true; [273:275]{ - [273:275]case val x1: [273]String = [273:275]""; + [273:275]case val x1: [273]String("") = [273:275]""; [273:275]case5(){ - [293:300]if ([298][298]x1.ne([298]null)) + [293:300]if ([298][298][298]"".ne([298]null)) [293:298]{ [293:298] val o7: [293]Option[List[Int]] = [293:298][293:298]Case4.unapplySeq([293]x1); [304:306]if ([293][293]o7.isEmpty.unary_!.&&([293][293][293][293]o7.get.!=([293]null).&&([293][293][293][293]o7.get.lengthCompare([293]0).==([293]0)))) @@ -77,7 +77,7 @@ [298][298]case6() }; [273:275]case6(){ - [273:275][273:275]matchEnd4([273:275]throw [273:275][273:275][273:275]new [273:275]MatchError([273:275]x1)) + [273:275][273:275]matchEnd4([273:275]throw [273:275][273:275][273:275]new [273:275]MatchError([273:275]"")) }; [273:275]matchEnd4(x: [NoPosition]Unit){ [273:275]x @@ -91,7 +91,7 @@ }; [330:373]def unapply([342:348]z: [345:348]): [334]Option[Int] = [365:373][365:369][365:369]new [365:369]Some[Int]([370:372]-1); [377:378]{ - [377:378]case val x1: [377]Int = [377:378]0; + [377:378]case val x1: [377]Int(0) = [377:378]0; [377:378]case5()[396:401]{ [396:401] val o7: [396]Option[Int] = [396:401][396:401]Case6.unapply([396]x1); [409:411]if ([396]o7.isEmpty.unary_!) @@ -100,7 +100,7 @@ [396][396]case6() }; [377:378]case6(){ - [377:378][377:378]matchEnd4([377:378]throw [377:378][377:378][377:378]new [377:378]MatchError([377:378]x1)) + [377:378][377:378]matchEnd4([377:378]throw [377:378][377:378][377:378]new [377:378]MatchError([377:378]0)) }; [377:378]matchEnd4(x: [NoPosition]Unit){ [377:378]x diff --git a/test/files/run/t7039.check b/test/files/run/t7039.check index 010632d5a278..e10cdabab834 100644 --- a/test/files/run/t7039.check +++ b/test/files/run/t7039.check @@ -1 +1,4 @@ +t7039.scala:8: warning: unreachable code + case UnapplySeqTest(5, 1) => println("Matched!") // compiles + ^ null matched diff --git a/test/files/run/virtpatmat_literal.check b/test/files/run/virtpatmat_literal.check index 0eabe3671300..1b832d16c5d1 100644 --- a/test/files/run/virtpatmat_literal.check +++ b/test/files/run/virtpatmat_literal.check @@ -1,3 +1,6 @@ +virtpatmat_literal.scala:6: warning: unreachable code + case `a` => println("FAILED") + ^ OK OK OK