Skip to content

Commit

Permalink
Emit unreachable arm warning for OR match arms (#6505)
Browse files Browse the repository at this point in the history
## Description

This PR fixes #6388. If a whole OR match arm is unreachable a warning
will be emitted like in this case:

```Sway
match v {
    1 => (),
    2 => (),
    1 | 2 => (), <<<--- Warning emitted here.
    _ => (),
};
```

The PR strictly fixes the issue reported in #6388. It does not fix a
previously known, more general issue, described in #5097 where we expect
warnings to be emitted also on individual elements of an OR arm, even if
the whole arm is reachable. E.g.:

```Sway
match v {
    1 => (),
    2 => (),
    1 | 3 => (), <<<--- Warning should be emitted here that `1` is not reachable.
    2 | 4 => (), <<<--- Warning should be emitted here that `2` is not reachable.
    _ => (),
};
```

Fixing this issue requires more invasive restructuring of the
propagation of the collected witness information and will be done in a
separate PR.

Closes #6388.

## 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).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [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.
  • Loading branch information
ironcev committed Sep 6, 2024
1 parent eaac649 commit 6fdc598
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ use super::{
/// `2`, any cases that the pattern `2` covers would previously be caught by the
/// catchall pattern.
///
/// Usefulness used in the exhaustivity algorithm.
/// Usefulness is used in the exhaustivity algorithm.
///
/// # Witnesses
///
Expand All @@ -148,11 +148,11 @@ use super::{
/// }
/// ```
///
/// the witness for pattern `1` would be the pattern "1" as the pattern `1`
/// the witness for pattern `1` would be the value "1" as the pattern `1`
/// would catch the concrete hypothetical matched value "1" and no other
/// previous cases would have caught it. The witness for pattern `_` is an
/// or-pattern of all of the remaining integers they wouldn't be caught by `0`
/// and `1`, so "3 | .. | MAX".
/// and `1`, so "2 | .. | MAX".
///
/// Given:
///
Expand All @@ -166,7 +166,7 @@ use super::{
/// }
/// ```
///
/// the pattern `1` (noted with an arrow) would not have any witnesses as there
/// the pattern `1` (noted with an arrow) would not have any witnesses
/// that it catches that are not caught by previous patterns.
///
/// # Putting it all together
Expand Down Expand Up @@ -205,7 +205,7 @@ pub(crate) fn check_match_expression_usefulness(
let mut matrix = Matrix::empty();
let mut arms_reachability = vec![];

// If the provided type does has no valid constructor and there are no
// If the provided type does not have a valid constructor and there are no
// branches in the match expression (i.e. no scrutinees to check), then
// every scrutinee (i.e. 0 scrutinees) are useful! We return early in this
// case.
Expand Down Expand Up @@ -489,7 +489,7 @@ fn is_useful_or(
) -> Result<WitnessReport, ErrorEmitted> {
let (_, q_rest) = q.split_first(handler, span)?;
let mut p = p.clone();
let mut witness_report = WitnessReport::Witnesses(PatStack::empty());
let mut witness_report = WitnessReport::NoWitnesses;
for pat in pats.into_iter() {
// 1. For each *k* 0..*a* compute *q'* as \[*rₖ q₂ ... qₙ*\].
let mut v = PatStack::from_pattern(pat);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[[package]]
name = "core"
source = "path+from-root-7FA7C6C8132DFBE7"

[[package]]
name = "match_expressions_unreachable_or"
source = "member"
dependencies = ["core"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <contact@fuel.sh>"]
license = "Apache-2.0"
name = "match_expressions_unreachable_or"
entry = "main.sw"

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// This test proves that https://github.com/FuelLabs/sway/issues/6388 is fixed.
script;

struct S {
x: bool
}

fn main() {
let v = 0u64;
// We want to have only one error in the case below,
// for the whole or-pattern elements and not for individual
// parts.
match v {
1 => (),
1 | 1 => (),
_ => (),
};

match v {
1 => (),
2 => (),
1 | 2 => (),
_ => (),
};

// TODO: Once https://github.com/FuelLabs/sway/issues/5097 is fixed, the below examples
// will also emit warnings for unreacahbility of individual or-pattern elements.
// Extend filecheck checks to cover those warning.

match v {
1 => (),
2 => (),
1 | 3 => (),
2 | 4 => (),
_ => (),
};

let s = S { x: false };
let _x = match s {
S { x } | S { x } => { x },
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
category = "compile"
expected_warnings = 2

#check: $()warning
#sameln: $()Match arm is unreachable
#check: $()1 => (),
#nextln: $()Preceding match arms already match all the values that `1 | 1` can match.
#check: $()1 | 1 => (),
#nextln: $()Match arm `1 | 1` is unreachable.

#check: $()warning
#sameln: $()Match arm is unreachable
#check: $()2 => (),
#nextln: $()Preceding match arms already match all the values that `1 | 2` can match.
#check: $()1 | 2 => (),
#nextln: $()Match arm `1 | 2` is unreachable.

0 comments on commit 6fdc598

Please sign in to comment.