From 2401b3f662514a9d787f702cb9a2e807b83ab158 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 18 Aug 2020 16:16:02 +0100 Subject: [PATCH] Check exhaustivity of case class unapplySeq extractors too ProductExtractorTreeMaker handles non-custom extractors, i.e. the synthetic ones generates for case classes. I think this case was just accidentally missing from the original commit. The change to run/virtpatmat_unapplyprod makes sense to me - it's an inexhaustive match, albeit with complicated counter-examples... :( This is the actually the last remaining tree maker, so none will back off after this, from my reading - but I kept the back-off stuff as is, keeping the diff minimal, to try to land this as easily as possible. --- .../nsc/transform/patmat/MatchAnalysis.scala | 1 + test/files/neg/t5365e.check | 23 +++++++++++++++++++ test/files/neg/t5365e.flags | 1 + test/files/neg/t5365e.scala | 19 +++++++++++++++ test/files/run/virtpatmat_unapplyprod.check | 4 ++++ 5 files changed, 48 insertions(+) create mode 100644 test/files/neg/t5365e.check create mode 100644 test/files/neg/t5365e.flags create mode 100644 test/files/neg/t5365e.scala diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index 94f3ea37171b..8f37cde0ed71 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -541,6 +541,7 @@ trait MatchAnalysis extends MatchApproximation { approx.fullRewrite.applyOrElse[TreeMaker, Prop](tm, { case BodyTreeMaker(_, _) => True // irrelevant -- will be discarded by symbolCase later case ExtractorTreeMaker(_, _, _) + | ProductExtractorTreeMaker(_, _) | GuardTreeMaker(_) => if (strict) False else True case _ => // debug.patmat("backing off due to "+ tm) diff --git a/test/files/neg/t5365e.check b/test/files/neg/t5365e.check new file mode 100644 index 000000000000..77f4e59e2fa8 --- /dev/null +++ b/test/files/neg/t5365e.check @@ -0,0 +1,23 @@ +t5365e.scala:7: warning: match may not be exhaustive. +It would fail on the following input: Bar(_) + def f0(x: Exh) = x match { case Foo() => () } // don't back off + ^ +t5365e.scala:8: warning: match may not be exhaustive. +It would fail on the following input: Bar(_) + def f1(x: Exh) = x match { case Foo(x) => x } // don't back off + ^ +t5365e.scala:9: warning: match may not be exhaustive. +It would fail on the following input: Bar(_) + def f2(x: Exh) = x match { case Foo(x, y) => x + y } // don't back off + ^ +t5365e.scala:10: warning: match may not be exhaustive. +It would fail on the following input: Bar(_) + def fX(x: Exh) = x match { case Foo(xs @ _*) => xs } // don't back off + ^ +t5365e.scala:11: warning: match may not be exhaustive. +It would fail on the following input: Foo(_) + def b1(x: Exh) = x match { case Bar(x) => x } // inexhaustive + ^ +error: No warnings can be incurred under -Xfatal-warnings. +5 warnings found +one error found diff --git a/test/files/neg/t5365e.flags b/test/files/neg/t5365e.flags new file mode 100644 index 000000000000..85d8eb2ba295 --- /dev/null +++ b/test/files/neg/t5365e.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/t5365e.scala b/test/files/neg/t5365e.scala new file mode 100644 index 000000000000..fb77bd97bae3 --- /dev/null +++ b/test/files/neg/t5365e.scala @@ -0,0 +1,19 @@ +sealed trait Exh +final case class Foo(xs: String*) extends Exh +final case class Bar(x: String) extends Exh + +class Main { + def ex(x: Exh) = x match { case Foo(xs @ _*) => xs case Bar(x) => x } // exhaustive + def f0(x: Exh) = x match { case Foo() => () } // don't back off + def f1(x: Exh) = x match { case Foo(x) => x } // don't back off + def f2(x: Exh) = x match { case Foo(x, y) => x + y } // don't back off + def fX(x: Exh) = x match { case Foo(xs @ _*) => xs } // don't back off + def b1(x: Exh) = x match { case Bar(x) => x } // inexhaustive + def fb(x: Exh) = x match { case Foo(x) => x case Bar(x) => x } // optimistically exhaustive + // ^ under -Xstrict-patmat-analysis pessimistically approximates case Foo(x) as inexhaustive: + // test/files/neg/t5365e.scala:12: warning: match may not be exhaustive. + // It would fail on the following input: Foo(_) + // def fb(x: Exh) = x match { case Foo(x) => x case Bar(x) => x } // optimistically exhaustive + // ^ + // ... and the counter-example needs work -.- ... +} diff --git a/test/files/run/virtpatmat_unapplyprod.check b/test/files/run/virtpatmat_unapplyprod.check index 2660ff8f96a2..87f3008a967a 100644 --- a/test/files/run/virtpatmat_unapplyprod.check +++ b/test/files/run/virtpatmat_unapplyprod.check @@ -1,3 +1,7 @@ +virtpatmat_unapplyprod.scala:19: warning: match may not be exhaustive. +It would fail on the following inputs: FooSeq((x: Int forSome x not in 1), "a", _), FooSeq((x: Int forSome x not in 1), (x: String forSome x not in "a"), _), FooSeq(1, (x: String forSome x not in "a"), _) + FooSeq(1, "a", true, false, true) match { + ^ (2,3) (2,3) (2,3)