Skip to content

Spurious exhaustiveness warning for union of tuple types #8690

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
LPTK opened this issue Apr 7, 2020 · 7 comments · Fixed by #8698
Closed

Spurious exhaustiveness warning for union of tuple types #8690

LPTK opened this issue Apr 7, 2020 · 7 comments · Fixed by #8698

Comments

@LPTK
Copy link
Contributor

LPTK commented Apr 7, 2020

Minimized code

class A
class B

def test(x: (A, B) | (B, A)) = x match {
  case (u: A, v) => (u, v)
  case (u: B, v) => (v, u)
}

https://scastie.scala-lang.org/ifiEqLPETBq2eruEbFXfFQ

Output

match may not be exhaustive.
It would fail on pattern case: Tuple2(_, _)

Moreover, the type inferred for v is always Object, whereas it could be more precise here.

Expectation

No warning, and more precise types.

@LPTK LPTK added the itype:bug label Apr 7, 2020
@smarter
Copy link
Member

smarter commented Apr 7, 2020

Might have the same root cause as #8681 ?

@liufengyun
Copy link
Contributor

The warning is fixed in #8698. This bug is caused by a different problem from #8681. The warning in the latter is issued during erasure.

However, it seems complex to give a more precise type to v. Any ideas @smarter ?

@smarter
Copy link
Member

smarter commented Apr 9, 2020

However, it seems complex to give a more precise type to v. Any ideas @smarter ?

I had a quick look and it doesn't seem easy yeah, there's various places which assume that the unapply method type is fully inferred before typing the unapply arguments, I think that part of the issue should be considered a feature request and not a bug.

@LPTK
Copy link
Contributor Author

LPTK commented Apr 9, 2020

@smarter fair enough, as long as the fully-annotated version works:

def test(x: (A, B) | (B, A)) = x match {
  case (u: A, v: B) => (u, v)
  case (u: B, v: A) => (v, u)
}

(@liufengyun it would be good to test that code too.)

@liufengyun
Copy link
Contributor

@LPTK The exhaustivity check can handle the code above (thanks for the test), but in a later phase there's a warning saying:

the type test for A cannot be checked at runtime

So, you may need a ClassTag for A and B.

@LPTK
Copy link
Contributor Author

LPTK commented Apr 9, 2020

@liufengyun isn't that warning spurious? A and B are classes, not type parameters.

@liufengyun
Copy link
Contributor

@LPTK I adapted the test 030249b

nicolasstucki added a commit that referenced this issue Apr 14, 2020
Fix #8690: the signature for product should come from expected type
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.

3 participants