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_let_patterns" shouldn't fire for leading if-let guards #98361

Closed
goffrie opened this issue Jun 21, 2022 · 2 comments · Fixed by #103031
Closed

"irrefutable_let_patterns" shouldn't fire for leading if-let guards #98361

goffrie opened this issue Jun 21, 2022 · 2 comments · Fixed by #103031
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-if_let_guard `#![feature(if_let_guard)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@goffrie
Copy link
Contributor

goffrie commented Jun 21, 2022

sample code:

#![feature(if_let_guard, let_chains)]

pub fn example(x: Option<Option<&str>>) {
    match x {
        Some(y) if let z = y.expect("can't be Some(None)") && z != "foo" => {
            dbg!(z);
        }
        _ => (),
    }
}

yields this warning:

warning: leading irrefutable pattern in let chain
 --> src/lib.rs:5:20
  |
5 |         Some(y) if let z = y.expect("can't be Some(None)") && z != "foo" => {
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(irrefutable_let_patterns)]` on by default
  = note: this pattern will always match
  = help: consider moving it outside of the construct

However, the suggestion to move it out of the construct doesn't make sense, as y isn't bound until we've entered the match arm in the first place.

@est31
Copy link
Member

est31 commented Oct 14, 2022

I've made a PR to turn off the lint in this instance: #103031

The ideal fix would be to check for usages in the prefix of the bindings created by the match, but this is better than the status quo as for lints, a true negative is better than a false positive.

@Rageking8
Copy link
Contributor

Rageking8 commented Oct 14, 2022

@rustbot label +T-compiler +A-lint +C-bug +F-if_let_guard

@rustbot rustbot added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-if_let_guard `#![feature(if_let_guard)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 14, 2022
albertlarsan68 added a commit to albertlarsan68/rust that referenced this issue Oct 14, 2022
… r=oli-obk

Suppress irrefutable let patterns lint for prefixes in match guards

In match guards, irrefutable prefixes might use the bindings created by the match pattern. Ideally, we check for this, but we can do the next best thing and just not lint for irrefutable prefixes in match guards.

Fixes rust-lang#98361
@bors bors closed this as completed in 7cf09c5 Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-if_let_guard `#![feature(if_let_guard)]` 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.

4 participants