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

Fix analysis of conjunctive patterns for exhaustiveness #13020

Merged
merged 17 commits into from
Sep 29, 2022

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Apr 20, 2022

@edgarfgp
Copy link
Contributor

Improving the pattern matching in conjunction with AP would be really nice . I have hit some of above limitation, making hard to grasp for a newcomer

@dsyme dsyme changed the title [WIP] fix analysis of conjunctive patterns for exhaustiveness Fix analysis of conjunctive patterns for exhaustiveness Aug 22, 2022
@vzarytovskii
Copy link
Member

This probably needs a bunch of tests, from the linked issues.

@vzarytovskii
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@edgarfgp
Copy link
Contributor

This probably needs a bunch of tests, from the linked issues.

I did add some unites locally and it seems that only the second issue on the list is fixed.
The first one still show a warning "This rule will never be matched"

Happy to raise a PR to Don's fork with the unit tests :)

@dsyme
Copy link
Contributor Author

dsyme commented Sep 22, 2022

@edgarfgp Thank you so much for checking! I've checked too and it seems both are fixed - with the first issue, two warnings were being generated - one for incomplete matching, and one for "never matched". The second is correct to emit, the first is not.

I've added testing for both test cases.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 22, 2022

@vzarytovskii This is ready

@edgarfgp
Copy link
Contributor

@dsyme Thanks for fixing this . I use AP and this will make the experience much better

@edgarfgp
Copy link
Contributor

@vzarytovskii This seems to be ready and the CI timed out. Would be awesome to have this in :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants