Skip to content
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

False unreachable case warning for inlined code #19157

Closed
odersky opened this issue Dec 1, 2023 · 2 comments · Fixed by #19190
Closed

False unreachable case warning for inlined code #19157

odersky opened this issue Dec 1, 2023 · 2 comments · Fixed by #19190

Comments

@odersky
Copy link
Contributor

odersky commented Dec 1, 2023

Compiler version

3.4.0 RC 1

Minimized example

  inline def count(inline x: Boolean) = x match
    case true => 1
    case false => 0

  assert(count(true) == 1)
  assert(count(false) == 0)
  var x = true
  assert(count(x) == 1)

Output

-- [E030] Match case Unreachable Warning: tl-count.scala:7:14 ------------------
7 |  assert(count(true) == 1)
  |         ^^^^^^^^^^^
  |         Unreachable case
  |-----------------------------------------------------------------------------
  |Inline stack trace
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |This location contains code that was inlined from tl-count.scala:5
5 |    case false => 0
  |         ^^^^^
   -----------------------------------------------------------------------------
-- [E030] Match case Unreachable Warning: tl-count.scala:8:14 ------------------
8 |  assert(count(false) == 0)
  |         ^^^^^^^^^^^^
  |         Unreachable case
  |-----------------------------------------------------------------------------
  |Inline stack trace
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |This location contains code that was inlined from tl-count.scala:4
4 |    case true => 1
  |         ^^^^
   -----------------------------------------------------------------------------

Expectation

No warnings. Generally, unreachable case warnings should be disabled for code that was inlined.

I encountered this in the wild where it did not find the source line correctly and was completely stumped until I changed the error message to give me a little bit more info. After the change I got this, which was more intelligible. I think that would be a good idea independently, but ideally we also mention previous cases as proposed in #18519.

7 |  assert(count(true) == 1)
  |         ^^^^^^^^^^^
  |         Unreachable case:
  |         Pattern false
  |         cannot possibly match a selector of type (true : Boolean)
  |         at this point in the match sequence.
  |-----------------------------------------------------------------------------
  |Inline stack trace
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |This location contains code that was inlined from tl-count.scala:5
5 |    case false => 0
  |         ^^^^^
   -----------------------------------------------------------------------------
-- [E030] Match case Unreachable Warning: tl-count.scala:8:14 ------------------
8 |  assert(count(false) == 0)
  |         ^^^^^^^^^^^^
  |         Unreachable case:
  |         Pattern true
  |         cannot possibly match a selector of type (false : Boolean)
  |         at this point in the match sequence.
  |-----------------------------------------------------------------------------
  |Inline stack trace
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |This location contains code that was inlined from tl-count.scala:4
4 |    case true => 1
  |         ^^^^
   -----------------------------------------------------------------------------

Overall, I believe that eliminating false unreachable warnings and improving error messages for real ones should be a priority for us. It's a major source of unfriendliness and head-scratching. And the other thing really needing improvement is a more robust reporting of inline traces. I noted when they come from different compilation units they are often wrong.

@odersky odersky added the stat:needs triage Every issue needs to have an "area" and "itype" label label Dec 1, 2023
@nicolasstucki
Copy link
Contributor

We should not check reachability for inlined code. Ideally, we should prune those cases from the tree.

@nicolasstucki nicolasstucki added area:pattern-matching area:inline itype:bug and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Dec 1, 2023
@nicolasstucki nicolasstucki self-assigned this Dec 1, 2023
@dwijnand dwijnand self-assigned this Dec 4, 2023
@dwijnand dwijnand linked a pull request Dec 4, 2023 that will close this issue
@dwijnand
Copy link
Member

dwijnand commented Dec 4, 2023

And the other thing really needing improvement is a more robust reporting of inline traces. I noted when they come from different compilation units they are often wrong.

I would be good to have a reproduction of that - I've not seen that, but could be I just wasn't looking right.

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