reduce false positives on some common cases from if-let-rescope

matthiaskrgr · Dec 2, 2024 · 2d61c09 · 2d61c09
1 parent 76f3ff6
commit 2d61c09
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 40 deletions.
diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs
@@ -103,8 +103,11 @@ fn expr_parent_is_else(tcx: TyCtxt<'_>, hir_id: hir::HirId) -> bool {
 }
 
 fn expr_parent_is_stmt(tcx: TyCtxt<'_>, hir_id: hir::HirId) -> bool {
-    let Some((_, hir::Node::Stmt(stmt))) = tcx.hir().parent_iter(hir_id).next() else {
-        return false;
+    let mut parents = tcx.hir().parent_iter(hir_id);
+    let stmt = match parents.next() {
+        Some((_, hir::Node::Stmt(stmt))) => stmt,
+        Some((_, hir::Node::Block(_) | hir::Node::Arm(_))) => return true,
+        _ => return false,
     };
     let (hir::StmtKind::Semi(expr) | hir::StmtKind::Expr(expr)) = stmt.kind else { return false };
     expr.hir_id == hir_id

diff --git a/tests/ui/drop/lint-if-let-rescope.fixed b/tests/ui/drop/lint-if-let-rescope.fixed
@@ -14,7 +14,7 @@ impl Drop for Droppy {
     }
 }
 impl Droppy {
-    fn get(&self) -> Option<u8> {
+    const fn get(&self) -> Option<u8> {
         None
     }
 }
@@ -62,11 +62,10 @@ fn main() {
         //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021
     }
 
-    if let () = { match Droppy.get() { Some(_value) => {} _ => {}} } {
-        //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024
-        //~| WARN: this changes meaning in Rust 2024
-        //~| HELP: the value is now dropped here in Edition 2024
-        //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021
+    if let () = { if let Some(_value) = Droppy.get() {} } {
+        // This should not lint.
+        // This `if let` sits is a tail expression of a block.
+        // In Edition 2024, the temporaries are dropped before exiting the surrounding block.
     }
 
     #[rustfmt::skip]
@@ -94,4 +93,17 @@ fn main() {
         //~| HELP: the value is now dropped here in Edition 2024
         //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021
     }
+
+    // We want to keep the `if let`s below as direct descendents of match arms,
+    // so the formatting is suppressed.
+    #[rustfmt::skip]
+    match droppy().get() {
+        _ => if let Some(_value) = droppy().get() {},
+        // Should not lint
+        // There is implicitly a block surrounding the `if let`.
+        // Given that it is a tail expression, the temporaries are dropped duly before
+        // the execution is exiting the `match`.
+    }
+
+    if let Some(_value) = droppy().get() {}
 }
diff --git a/tests/ui/drop/lint-if-let-rescope.rs b/tests/ui/drop/lint-if-let-rescope.rs
@@ -14,7 +14,7 @@ impl Drop for Droppy {
     }
 }
 impl Droppy {
-    fn get(&self) -> Option<u8> {
+    const fn get(&self) -> Option<u8> {
         None
     }
 }
@@ -63,10 +63,9 @@ fn main() {
     }
 
     if let () = { if let Some(_value) = Droppy.get() {} } {
-        //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024
-        //~| WARN: this changes meaning in Rust 2024
-        //~| HELP: the value is now dropped here in Edition 2024
-        //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021
+        // This should not lint.
+        // This `if let` sits is a tail expression of a block.
+        // In Edition 2024, the temporaries are dropped before exiting the surrounding block.
     }
 
     #[rustfmt::skip]
@@ -94,4 +93,17 @@ fn main() {
         //~| HELP: the value is now dropped here in Edition 2024
         //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021
     }
+
+    // We want to keep the `if let`s below as direct descendents of match arms,
+    // so the formatting is suppressed.
+    #[rustfmt::skip]
+    match droppy().get() {
+        _ => if let Some(_value) = droppy().get() {},
+        // Should not lint
+        // There is implicitly a block surrounding the `if let`.
+        // Given that it is a tail expression, the temporaries are dropped duly before
+        // the execution is exiting the `match`.
+    }
+
+    if let Some(_value) = droppy().get() {}
 }
diff --git a/tests/ui/drop/lint-if-let-rescope.stderr b/tests/ui/drop/lint-if-let-rescope.stderr
@@ -112,27 +112,7 @@ LL |     if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ =
    |                        ~~~~~              +++++++++++++++++             ~~~~         +
 
 error: `if let` assigns a shorter lifetime since Edition 2024
-  --> $DIR/lint-if-let-rescope.rs:65:22
-   |
-LL |     if let () = { 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
-  --> $DIR/lint-if-let-rescope.rs:65:55
-   |
-LL |     if let () = { if let Some(_value) = Droppy.get() {} } {
-   |                                                       ^
-help: a `match` with a single arm can preserve the drop order up to Edition 2021
-   |
-LL |     if let () = { match Droppy.get() { Some(_value) => {} _ => {}} } {
-   |                   ~~~~~              +++++++++++++++++    ++++++++
-
-error: `if let` assigns a shorter lifetime since Edition 2024
-  --> $DIR/lint-if-let-rescope.rs:73:12
+  --> $DIR/lint-if-let-rescope.rs:72:12
    |
 LL |     if (if let Some(_value) = droppy().get() { true } else { false }) {
    |            ^^^^^^^^^^^^^^^^^^^--------^^^^^^
@@ -142,7 +122,7 @@ LL |     if (if let Some(_value) = droppy().get() { true } else { false }) {
    = 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
-  --> $DIR/lint-if-let-rescope.rs:73:53
+  --> $DIR/lint-if-let-rescope.rs:72:53
    |
 LL |     if (if let Some(_value) = droppy().get() { true } else { false }) {
    |                                                     ^
@@ -152,7 +132,7 @@ LL |     if (match droppy().get() { Some(_value) => { true } _ => { false }}) {
    |         ~~~~~                +++++++++++++++++          ~~~~          +
 
 error: `if let` assigns a shorter lifetime since Edition 2024
-  --> $DIR/lint-if-let-rescope.rs:79:21
+  --> $DIR/lint-if-let-rescope.rs:78:21
    |
 LL |     } else if (((if let Some(_value) = droppy().get() { true } else { false }))) {
    |                     ^^^^^^^^^^^^^^^^^^^--------^^^^^^
@@ -162,7 +142,7 @@ LL |     } else if (((if let Some(_value) = droppy().get() { true } else { false
    = 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
-  --> $DIR/lint-if-let-rescope.rs:79:62
+  --> $DIR/lint-if-let-rescope.rs:78:62
    |
 LL |     } else if (((if let Some(_value) = droppy().get() { true } else { false }))) {
    |                                                              ^
@@ -172,7 +152,7 @@ LL |     } else if (((match droppy().get() { Some(_value) => { true } _ => { fal
    |                  ~~~~~                +++++++++++++++++          ~~~~          +
 
 error: `if let` assigns a shorter lifetime since Edition 2024
-  --> $DIR/lint-if-let-rescope.rs:91:15
+  --> $DIR/lint-if-let-rescope.rs:90:15
    |
 LL |     while (if let Some(_value) = droppy().get() { false } else { true }) {
    |               ^^^^^^^^^^^^^^^^^^^--------^^^^^^
@@ -182,7 +162,7 @@ LL |     while (if let Some(_value) = droppy().get() { false } else { true }) {
    = 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
-  --> $DIR/lint-if-let-rescope.rs:91:57
+  --> $DIR/lint-if-let-rescope.rs:90:57
    |
 LL |     while (if let Some(_value) = droppy().get() { false } else { true }) {
    |                                                         ^
@@ -191,5 +171,5 @@ help: a `match` with a single arm can preserve the drop order up to Edition 2021
 LL |     while (match droppy().get() { Some(_value) => { false } _ => { true }}) {
    |            ~~~~~                +++++++++++++++++           ~~~~         +
 
-error: aborting due to 8 previous errors
+error: aborting due to 7 previous errors