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

The implementation of compiling OR patterns is incorrect #2574

Open
yorickpeterse opened this issue Feb 2, 2024 · 4 comments
Open

The implementation of compiling OR patterns is incorrect #2574

yorickpeterse opened this issue Feb 2, 2024 · 4 comments
Labels
bug Something isn't working help wanted Contributions encouraged priority:low

Comments

@yorickpeterse
Copy link
Contributor

yorickpeterse commented Feb 2, 2024

Gleam uses the flatten_or function from https://github.com/yorickpeterse/pattern-matching-in-rust. I recently found out this implementation is incorrect, as it doesn't account for patterns such as foo or foo (i.e. OR patterns that bind variables in their branches). While such patterns are nonsensical (= they're always true), it would trigger a panic here.

The fix is to expand OR patterns earlier, before pushing bindings/wildcards out of the patterns. This is implemented here. The new setup could probably be improved performance wise, but as far as I can determine it works well enough.

For some additional background information, see inko-lang/inko#679 and inko-lang/inko#599 which is how I found out about this.

@michallepicki
Copy link
Contributor

@lpil should this be fixed before 1.0 as well?

@lpil
Copy link
Member

lpil commented Feb 7, 2024

It doesn't have to be I don't think. I've not yet verified that this impacts us as we use or patterns in a limited way currently.

@yorickpeterse
Copy link
Contributor Author

yorickpeterse commented Feb 7, 2024

@lpil Patterns such as this are unlikely to be found in the field, as you'd have to write something like this (using Inko syntax here):

match whatever {
  case 10 -> ...
  case a or a -> ...
}

This means it's not urgent by any means, as in the worst case you'd just run into a panic 😃

@lpil
Copy link
Member

lpil commented Feb 7, 2024

Ah! Thank you! 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Contributions encouraged priority:low
Projects
None yet
Development

No branches or pull requests

3 participants