Skip to content

Vector.unapplySeq is refutable #12240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
martijnhoekstra opened this issue Nov 23, 2020 · 3 comments · Fixed by scala/scala#9343
Closed

Vector.unapplySeq is refutable #12240

martijnhoekstra opened this issue Nov 23, 2020 · 3 comments · Fixed by scala/scala#9343
Assignees
Milestone

Comments

@martijnhoekstra
Copy link

martijnhoekstra commented Nov 23, 2020

reproduction steps

using Scala 2.13.4

scala> Vector(1) match {
  case Vector(_ @ _*) => ???
}

problem

match may not be exhaustive.
It would fail on the following input: Vector1()

The match is totally exhaustive. Unfortunately, scalac doesn't know. Vector.unapplySeq seems to be using name-based pattern matching to return a SeqFactory.UnapplySeqWrapper. But name-based pattern matching can't AFAIK provide irrefutable patterns, so the pattern ends up refutable, and the warning (with the somewhat odd counterexample) is emitted.

@martijnhoekstra
Copy link
Author

@mrdziuban
Copy link

mrdziuban commented Nov 23, 2020

If/when this is fixed, I would hope that a match statement like this would also be considered exhaustive

Vector("test") match {
  case Vector() => "empty"
  case Vector(_) => "one"
  case Vector(_, _, _*) => "two or more"
}

which seems like a good alternative to using +: like my original question in gitter which can't prove complementary exhaustivity between Vector() and +:, e.g.

Vector("test") match {
  case Vector() => "empty"
  case _ +: Vector() => "one"
  case _ +: _ +: _ => "two or more"
}

@dwijnand
Copy link
Member

I wonder if changing UnapplySeqWrapper to have def isEmpty: false.type = false fixes this? Hopefully then two obviously exhaustive cases would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants