Open nrinaudo opened 3 years ago
agree this is a bug.
a weird thing here is that if you add another clause, the checking comes back!
scala 2.13.4> def test(i: Int) = i match {
| case value if value < 0 => true
| case value if value > 10 => false
| case 5 => true
| }
^
warning: match may not be exhaustive.
So -Vpatmat
is the flag to get some pattern matching output, and normally it's an overwhelming amount of noise (that took me a few weeks to grok and be comfortable with) but very unexpectedly this code shows close to none!
$ m Ints.scala
class Test {
def test(i: Int) = i match {
case value if value < 0 => true
case value if value > 10 => false
}
}
$ scalac -Vpatmat Ints.scala
translating {case (value @ _) if value.<(0) => true
case (value @ _) if value.>(10) => false}
combining cases: {G(value.<(0)) >> B(true,Boolean)
G(value.>(10)) >> B(false,Boolean)}
def: (case <synthetic> val x1: Int = i,List(value x1),List(method test, class Test, package <empty>, package <root>))
Why is that? Because it can be emitted by a switch, so it doesn't do any analysis, because analysis was for those non-switchable sealed types! I'll try to fix this for the next release. I'll also try to suss out what it is about case 5
that changes that, as I think that should be switchable too (a separate bug, I guess).
I have a branch where I got this working. But making switchable types emit warnings fixes this and then highlights all the times (e.g. in the compiler) where internal implementations are switching on ints because of how fast it is, for the (let's say) 6 ints that are tracked internally. I think I ran out of steam/patience to fix all the cases in the compiler before I gave myself a break and considered that this might be just bad idea...
could something like an annotation @openswitch
that combines the functionality of @switch
and @unchecked
be useful there. This got me thinking about the two kinds of unchecked matches:
The match is exhaustive in the domain of the scrutinee, but scalac doesn't know it: The programmer knows more than the compiler about the generators. @unchecked
fits this case perfectly: you tell scalac it's ok it can't check exhaustiveness, and the reader that scalac isn't going to help you check exhaustiveness. This is the "not known to be exhaustive" variety.
The domain of the scrutinee is smaller than the domain of the static type of the scrutinee (example: HTTP Status code represented as Short). You accept as a programmer that not all values of the type of the scrutinee are represented in the match. This is the "know to be not exhaustive" variety -- which is the one that I initially expected to be less of a problem to warn for. In this case the programmer knows more about the domain of some value than the compiler. @unchecked
sounds like the wrong name for communicating this, and feels wrong to use.
In the case of switch matches, you commonly want to use a type that allows for more values than your domain, because you're restricted by performance considerations. Maybe it makes sense to combine the annotation for not caring about non-exhaustiveness and the annotation for caring about the switch performance.
Can we reuse @switch
? Two questions:
@switch
an int and I want to get exhaustivity checks?@switch
an int and I don't want to get exhaustivity checks?(I say int but I mean any switchable type: byte, short, int, char, String.)
I think the second one is easier: if you're not using a switch table and you've got an incomplete match, then @unchecked
it. But is the first one ok? 🤔
Figuring out what I mean exactly: I always want exhaustiveness checks on the domain of the match, but I can't always communicate the domain to the compiler. Even if I can't, I may still want the compiler to warn you that you don't have a default case. But maybe that's uncommon enough to either not support it, or to support it with a new opt-in annotation.
As for concrete answers to your questions:
def rep(p: Parser[A], max: Int, fact: Factory[A, B]): ParserB = (max @switch) match {
case 0 => Parser.pure(fact.newBuilder().result)
case 1 => p.map(a => fact.newBuilder().addOne(a).result()).?.map(_.getOrElse(fact.newBuilder().result))
case n => Repeater(p, n -1, fact)
}
status match {
case info if info >= 100 && info < 200 =>
case 200 =>
case ok if ok > 200 && ok < 300 =>
case 400 =>
case 401 =>
case 403 =>
case agentError if agentError >= 400 && agentError < 500 =>
case serverError if serverError >= 500 =>
}
For the second maybe (status @unchecked) match
is good enough, and outside the scope of this issue.
For the first, maybe it's enough to suck it up and don't have exhaustiveness checking even though you would prefer it, maybe introduce, I don't know, (max @checkedswitch) match
reproduction steps
Scala 2.13.4 is supposed to implement the following:
(Source)
Yet, the following code compiles without warning:
This has been tried with and without the
-Xlint:strict-unsealed-patmat
scalac option.problem
The way I understand the quoted sentence is: the compiler will consider both branches of the pattern match as potentially being
false
at the same time, which will cause it to emit a non-exhaustivity warning.This is not what's happening.