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 compiler checks for exhaustive match arms in match expressions. #701

Merged
merged 65 commits into from Mar 22, 2022

Conversation

emilyaherbert
Copy link
Contributor

@emilyaherbert emilyaherbert commented Jan 25, 2022

Closes #581

This PR does two things:

  1. I had to introduce a pretty significant change to the way Expressions are parsed. In order to reliably gather type information for exhaustivity checking, all "primary expressions" inside of match expressions must first be named as variables, then that new generated variable name is used within the match expression. The problem is that before this PR, match expressions could not declare variables as they had no access to creating AstNodes. This PR introduces a new type ParseResult that "bubbles up" a Vec that is populated by match expressions. Moreover, this change fixes a silent bug where if the value of your match expression was a function with side effects, that function would have been called n number of times for n match arms.
  2. Implement match exhaustivity checking and match arm reachability checking. There is a bit of math involved, but I describe it at a high level in sway-core/src/semantic_analysis/ast_node/expression/usefulness/mod.rs. Let me know if you have questions 😄

@emilyaherbert emilyaherbert self-assigned this Jan 25, 2022
sway-core/src/optimize.rs Outdated Show resolved Hide resolved
@mohammadfawaz mohammadfawaz added enhancement New feature or request compiler General compiler. Should eventually become more specific as the issue is triaged labels Mar 15, 2022
@emilyaherbert
Copy link
Contributor Author

Pinging @canndrew @sezna @mohammadfawaz @otrho for review 😄

@sezna
Copy link
Contributor

sezna commented Mar 18, 2022

Just one question, then I'm ready to approve: #701 (comment)

sezna
sezna previously approved these changes Mar 19, 2022
sezna
sezna previously approved these changes Mar 20, 2022
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like @otrho won the merge conflict race

sway-core/Cargo.toml Outdated Show resolved Hide resolved
emilyaherbert added 2 commits March 22, 2022 12:36
….com:FuelLabs/sway into emilyaherbert-581/exhaustive-pattern-matches
@emilyaherbert
Copy link
Contributor Author

Pinging @canndrew @sezna @mohammadfawaz @otrho for review 😄

Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>
@emilyaherbert emilyaherbert merged commit c1ef577 into master Mar 22, 2022
@emilyaherbert emilyaherbert deleted the emilyaherbert-581/exhaustive-pattern-matches branch March 22, 2022 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Implement exhaustive pattern match checking in match expressions
5 participants