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

Irrefutable if let on a single-variant enum only produces () value #61788

Closed
moxian opened this issue Jun 12, 2019 · 8 comments · Fixed by #120479
Closed

Irrefutable if let on a single-variant enum only produces () value #61788

moxian opened this issue Jun 12, 2019 · 8 comments · Fixed by #120479
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-enhancement Category: An issue proposing an enhancement or a PR with one. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@moxian
Copy link
Contributor

moxian commented Jun 12, 2019

The following code (playground)

enum Enum{
    Variant(i32),
}

fn stuff(x: Enum) -> i32{
    if let Enum::Variant(value) = x {
        value
    }
}

gives the following error:

   Compiling playground v0.0.1 (/playground)
error[E0317]: if may be missing an else clause
 --> src/lib.rs:6:5
  |
6 | /     if let Enum::Variant(value) = x {
7 | |         value
8 | |     }
  | |_____^ expected (), found i32
  |
  = note: expected type `()`
             found type `i32`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0317`.
error: Could not compile `playground`.

which is weird, since the else clause would never be visited even if it was present.

The equivalent one-arm match

enum Enum{
    Variant(i32),
}

fn stuff(x: Enum) -> i32{
    match x {
        Enum::Variant(value) => value,
    }
}

compiles just fine (expectedly).

Adding an else { unreachable!() } branch to the if let is, of course, possible, but unelegant, and really shouldn't be needed, as it can be inferred at compile time.

@varkor
Copy link
Member

varkor commented Jun 12, 2019

You can match irrefutably without if in this case:

fn stuff(x: Enum) -> i32 {
    let Enum::Variant(value) = x;
    value
}

though maybe we should give this special diagnostics.

@varkor varkor added the A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. label Jun 12, 2019
@Centril
Copy link
Contributor

Centril commented Jun 12, 2019

I think this is too niche to deserve a special diagnostics (and I don't think we should substantially work on diagnostics for if until let_chains is implemented).

which is weird, since the else clause would never be visited even if it was present.

if let is defined by a desugaring:

if let PAT = EXPR BLOCK_IF [else BLOCK_ELSE]

becomes:

match EXPR {
    PAT => BLOCK_IF,
    _ => [ {} | BLOCK_ELSE ],
}

so what you really wrote is:

match x {
    Enum::Variant(value) => value,
    _ => {},
}

The block {} is typed at () which does not unify with value unless it is also typed at ().

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority labels Jun 13, 2019
@moxian
Copy link
Contributor Author

moxian commented Jun 14, 2019

I didn't know let Enum::Variant(value) = x; was a valid syntax. If I did, I probably wouldn't have filed the issue, because of course, in hindsight, that's how you write an infallible if let.

(With that knowledge in mind) I agree that it's only a diagnostics issue and no language change is needed here.
And I fully agree that it's a niche low-priority quirk that should maybe be fixed "someday". (I don't know what let_chains but even then I'd guess that this one is lower priority than that).

@estebank estebank added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Jul 25, 2019
@estebank
Copy link
Contributor

I think the appropriate thing to do here would be to lint on if let Irrefutable = foo {}, which would have clarified things.

@Centril
Copy link
Contributor

Centril commented Jul 25, 2019

@estebank
Copy link
Contributor

But we do only after typechecking, and we don't if there are any errors before then. Quite a few lints have similar issues related to not happening early enough.

@Centril
Copy link
Contributor

Centril commented Jul 25, 2019

@estebank How do you know that the pattern is irrefutable before match checking in HAIR?

@estebank
Copy link
Contributor

You can't, but you can write the lint in such a way that it triggers regardless. We're getting to typechecking, we should be able to detect irrefutability by then. It just can't use the standard linting machinery (like a few other lints already do to avoid being silenced by accident).

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 27, 2019
estebank added a commit to estebank/rust that referenced this issue Jan 29, 2024
When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead.

```
error[E0317]: `if` may be missing an `else` clause
  --> $DIR/irrefutable-if-let-without-else.rs:8:5
   |
LL |   fn foo(x: Enum) -> i32 {
   |                      --- expected `i32` because of this return type
LL | /     if let Enum::Variant(value) = x {
LL | |         value
LL | |     }
   | |_____^ expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = help: consider adding an `else` block that evaluates to the expected type
help: consider using an irrefutable `let` binding instead
   |
LL ~     let Enum::Variant(value) = x;
LL ~         value
   |
```

Fix rust-lang#61788.
estebank added a commit to estebank/rust that referenced this issue Jan 29, 2024
When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead.

```
error[E0317]: `if` may be missing an `else` clause
  --> $DIR/irrefutable-if-let-without-else.rs:8:5
   |
LL |   fn foo(x: Enum) -> i32 {
   |                      --- expected `i32` because of this return type
LL | /     if let Enum::Variant(value) = x {
LL | |         value
LL | |     }
   | |_____^ expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = help: consider adding an `else` block that evaluates to the expected type
help: consider using an irrefutable `let` binding instead
   |
LL ~     let Enum::Variant(value) = x;
LL ~         value
   |
```

Fix rust-lang#61788.
Nadrieril added a commit to Nadrieril/rust that referenced this issue Feb 7, 2024
Suggest turning `if let` into irrefutable `let` if appropriate

When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead.

```
error[E0317]: `if` may be missing an `else` clause
  --> $DIR/irrefutable-if-let-without-else.rs:8:5
   |
LL |   fn foo(x: Enum) -> i32 {
   |                      --- expected `i32` because of this return type
LL | /     if let Enum::Variant(value) = x {
LL | |         value
LL | |     }
   | |_____^ expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = help: consider adding an `else` block that evaluates to the expected type
help: consider using an irrefutable `let` binding instead
   |
LL ~     let Enum::Variant(value) = x;
LL ~         value
   |
```

Fix rust-lang#61788.
Nadrieril added a commit to Nadrieril/rust that referenced this issue Feb 7, 2024
Suggest turning `if let` into irrefutable `let` if appropriate

When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead.

```
error[E0317]: `if` may be missing an `else` clause
  --> $DIR/irrefutable-if-let-without-else.rs:8:5
   |
LL |   fn foo(x: Enum) -> i32 {
   |                      --- expected `i32` because of this return type
LL | /     if let Enum::Variant(value) = x {
LL | |         value
LL | |     }
   | |_____^ expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = help: consider adding an `else` block that evaluates to the expected type
help: consider using an irrefutable `let` binding instead
   |
LL ~     let Enum::Variant(value) = x;
LL ~         value
   |
```

Fix rust-lang#61788.
@bors bors closed this as completed in a939bad Feb 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 7, 2024
Rollup merge of rust-lang#120479 - estebank:issue-61788, r=wesleywiser

Suggest turning `if let` into irrefutable `let` if appropriate

When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead.

```
error[E0317]: `if` may be missing an `else` clause
  --> $DIR/irrefutable-if-let-without-else.rs:8:5
   |
LL |   fn foo(x: Enum) -> i32 {
   |                      --- expected `i32` because of this return type
LL | /     if let Enum::Variant(value) = x {
LL | |         value
LL | |     }
   | |_____^ expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = help: consider adding an `else` block that evaluates to the expected type
help: consider using an irrefutable `let` binding instead
   |
LL ~     let Enum::Variant(value) = x;
LL ~         value
   |
```

Fix rust-lang#61788.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-enhancement Category: An issue proposing an enhancement or a PR with one. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants