rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.65k stars 12.75k forks source link

false positive for `if-let-rescope` where `if let` statements without `else` are also linted #133167

Open m4rch3n1ng opened 3 hours ago

m4rch3n1ng commented 3 hours ago

Code

remove the else from the example at https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/if_let_rescope/static.IF_LET_RESCOPE.html

#![warn(if_let_rescope)]
#![allow(unused_variables)]

struct Droppy;
impl Drop for Droppy {
    fn drop(&mut self) {
        // Custom destructor, including this `drop` implementation, is considered
        // significant.
        // Rust does not check whether this destructor emits side-effects that can
        // lead to observable change in program semantics, when the drop order changes.
        // Rust biases to be on the safe side, so that you can apply discretion whether
        // this change indeed breaches any contract or specification that your code needs
        // to honour.
        println!("dropped");
    }
}
impl Droppy {
    fn get(&self) -> Option<u8> {
        None
    }
}

fn main() {
    if let Some(value) = Droppy.get() {
        // do something
    }
}

Current output

warning: `if let` assigns a shorter lifetime since Edition 2024
  --> src/main.rs:24:5
   |
24 |     if let Some(value) = Droppy.get() {
   |        ^^^^^^^^^^^^^^^^^^------^^^^^^
   |                          |
   |                          this value has a significant drop implementation which may observe a major change in drop order and requires your discretion
   |
   = warning: this changes meaning in Rust 2024
   = note: for more information, see issue #124085 <https://github.com/rust-lang/rust/issues/124085>
help: the value is now dropped here in Edition 2024
  --> src/main.rs:26:2
   |
26 |     }
   |     ^
note: the lint level is defined here
  --> src/main.rs:1:9
   |
1  | #![warn(if_let_rescope)]
   |         ^^^^^^^^^^^^^^
help: a `match` with a single arm can preserve the drop order up to Edition 2021
   |
24 ~     match Droppy.get() { Some(value) => {
25 |         // do something
26 ~     } _ => {}}
   |

Desired output

Rationale and extra context

as far as i know, the new if-let-rescope is only relevant, when you have an else block, like in the actual example, because it cannot change to be dropped before the else, as there is no else, so (as far as i know) the place where it is dropped does not change.

more info if you change the code to print stuff ```rust #![warn(if_let_rescope)] #![allow(unused_variables)] struct Droppy; impl Drop for Droppy { fn drop(&mut self) { dbg!("drop"); } } impl Droppy { fn get(&self) -> Option { Some(0) } } fn main() { if let Some(value) = Droppy.get() { dbg!("one"); } dbg!("two"); } ``` and then run it with both edition 2024 and edition 2021 ```text $ rustup run nightly -- rustc --edition 2024 -Zunstable-options src/main.rs $ ./main [src/main.rs:18:3] "one" = "one" [src/main.rs:7:3] "drop" = "drop" [src/main.rs:20:2] "two" = "two" $ rustup run nightly -- rustc --edition 2021 src/main.rs $ ./main [src/main.rs:18:3] "one" = "one" [src/main.rs:7:3] "drop" = "drop" [src/main.rs:20:2] "two" = "two" ``` they both do the same

Other cases

while testing i found out that if you modify the original code to include anything after the if let statement it doesn't lint it anymore

#![warn(if_let_rescope)]
#![allow(unused_variables)]

struct Droppy;
impl Drop for Droppy {
    fn drop(&mut self) {
        println!("dropped");
    }
}
impl Droppy {
    fn get(&self) -> Option<u8> {
        None
    }
}

fn main() {
    if let Some(value) = Droppy.get() {
        // do something
    }
    // this code is still affected by the lint
}
#![warn(if_let_rescope)]
#![allow(unused_variables)]

struct Droppy;
impl Drop for Droppy {
    fn drop(&mut self) {
        println!("dropped");
    }
}
impl Droppy {
    fn get(&self) -> Option<u8> {
        None
    }
}

fn main() {
    if let Some(value) = Droppy.get() {
        // do something
    }
    println!("with this line it doesn't trigger the lint anymore");
}

Rust Version

rustc 1.84.0-nightly (5ec7d6eee 2024-11-17)
binary: rustc
commit-hash: 5ec7d6eee7e0f5236ec1559499070eaf836bc608
commit-date: 2024-11-17
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.3

Anything else?

No response

jieyouxu commented 2 hours ago

cc @dingxiangfei2009 as you may know more about the intention.

jieyouxu commented 2 hours ago

Marking this as C-discussion for now but please change it to C-bug if more suitable.