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

Match case incorrectly marked as unreachable, and the wrong case is executed #599

Closed
yorickpeterse opened this issue Jul 25, 2023 · 2 comments
Assignees
Labels
accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers bug Defects, unintended behaviour, etc compiler Changes related to the compiler
Milestone

Comments

@yorickpeterse
Copy link
Collaborator

yorickpeterse commented Jul 25, 2023

Please describe the bug

When compiling certain pattern match expressions, the compiler incorrectly flags certain cases as unreachable, even though they are in fact reachable. In addition, it seems the wrong branch is taken at runtime. This only happens when one case covers a pattern but uses a guard, and another case also covers the pattern but without a guard.

Please list the exact steps necessary to reproduce the bug

Run this code:

import std.stdio.STDOUT

class async Main {
  fn async main {
    let out = STDOUT.new

    match 20 {
      case 10 or 20 if false -> out.print('guard')
      case 10 -> out.print('10')
      case 20 -> out.print('20')
      case _ -> out.print('other')
    }
  }
}

This produces a warning that case 20 -> ... is unreachable, and when run the output in STDOUT is 10 instead of 20. The expected result i no warnings, and 20 in STDOUT.

Operating system

Fedora Silverblue 38

Inko version

main

Rust version

1.70.0

Related issues

@yorickpeterse yorickpeterse added accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers bug Defects, unintended behaviour, etc compiler Changes related to the compiler labels Jul 25, 2023
@yorickpeterse yorickpeterse changed the title Match case incorrectly marked as unreachable Match case incorrectly marked as unreachable, and the wrong case is executed Jul 25, 2023
@yorickpeterse
Copy link
Collaborator Author

Note that this only happens when the pattern is case A or B if C, if you instead use separate guarded patterns it works just fine.

@yorickpeterse
Copy link
Collaborator Author

From "ronsh" on the Discord:

I'm working on my own hobby language (#jin on the PL discord).
I'm in the process of implementing pattern matching, and I'm basing it (heavily) on your example implementation.
I just encountered the same issue as you while implementing guards: #599

I think I fixed it in my implementation, both for literal patterns (int/string), and constructor patterns(bool/etc...)

I thought I'd share my fixes with you since your work helped me so much in learning how pattern matching works!
Just a small disclaimer: I probably didn't test all cases, but these did fix the specific issue regarding guards.

Implementation for literal cases: https://github.com/r0nsha/jin/blob/master/compiler/src/mir/pmatch.rs#L316
Here the fix is quite hacky, but it works I guess. I keep track of whether I encountered a row with a guard.
If so, instead of pushing the row into the type_cases (raw_cases in your implementation), I treat all further rows as fallbacks.

Implementation for constructor cases: https://github.com/r0nsha/jin/blob/master/compiler/src/mir/pmatch.rs#L382
In this one, the issue was that guards inside an or pattern were all mapped to the same guard, leading to wrong branching logic.
The fix was to clone the guard, and add the cloned guards to the guards map after compilation: https://github.com/r0nsha/jin/blob/master/compiler/src/mir/lower.rs#L488

I hope this helps, although I'm sure there are issues hiding in my implementation...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers bug Defects, unintended behaviour, etc compiler Changes related to the compiler
Projects
None yet
Development

No branches or pull requests

1 participant