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

Emit compile error if the same match arm variables in alternatives are not of the same type #5081

Closed
ironcev opened this issue Sep 2, 2023 · 4 comments · Fixed by #5104
Closed
Assignees
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: static-analysis compiler General compiler. Should eventually become more specific as the issue is triaged

Comments

@ironcev
Copy link
Member

ironcev commented Sep 2, 2023

At the moment there is no error if match arm variables with the same name in match arm alternatives are not of the same type. The type of the variable in the last alternative will win.

struct S {
    x: bool,
    y: u64
}

...
let _x = match s {
    S { x: y, y: x, .. } | S { x, y, .. } => x, // No type mismatch error but it should be.
           ^                      ^ y is u64 here
           |
           y is bool here 
    ...
}
@ironcev ironcev added compiler General compiler. Should eventually become more specific as the issue is triaged compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: static-analysis labels Sep 2, 2023
@ironcev ironcev self-assigned this Sep 2, 2023
@anton-trunov
Copy link
Contributor

This example is not allowed anymore after the recent PR, or did I misunderstand this?

@ironcev
Copy link
Member Author

ironcev commented Sep 6, 2023

The PR (#5085) was about duplicates. In this issue in general we do not need to have duplicates. All alternatives must have the same variables and a single variable must be of the same type in all of the alternatives. In the above example, there are no duplicates, just two variables x and y in both alternatives, which is fine. But their types are not the same and that's the issue.

I see now that I could have gone for a more understandable example, naming the variables a and b or similar. This was one of the crazy constellations I used in brainstorming and tests. Definitely not the best one for describing the issue :-)

So, this example was never supposed to be allowed, regardless of the duplicates, but the consistency of types was never checked.

@ironcev
Copy link
Member Author

ironcev commented Sep 6, 2023

For the record, here is the Rust error on the same issue:

error[E0308]: mismatched types
  --> src/main.rs:10:39
   |
9  |     let _x = match s {
   |                    - this expression has type `S`
10 |         S { x: y, y: x, .. } | S { x, y, .. } => x,
   |                -                      ^ expected `bool`, found `u64`
   |                |
   |                first introduced with type `bool` here
   |
   = note: in the same arm, a binding must have the same type in all alternatives

@anton-trunov
Copy link
Contributor

Got it, thanks @ironcev :)

ironcev added a commit that referenced this issue Sep 22, 2023
## Description

This PR fixes three issues related to variables in match arm
alternatives:
- removes the wrong error message that a variable in match arm
alternative is not defined (#5082).
- emits error if the same variable in match arm alternatives is not of
the same type in all alternatives (#5081).
- emits all errors in a match arm instead of short-circuiting after the
first error (#5108)

Closes #5081.
Closes #5082.
Closes #5108.

## Demo


![image](https://github.com/FuelLabs/sway/assets/4142833/fea1a3c9-d58a-423e-a9df-0ca594f3162f)


![image](https://github.com/FuelLabs/sway/assets/4142833/695daba6-1825-4548-92b3-789ab519512e)

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Anton Trunov <anton.a.trunov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: static-analysis compiler General compiler. Should eventually become more specific as the issue is triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants