From 2d61c0906a31ff1310603ee13582c05f29aa1190 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Mon, 2 Dec 2024 18:27:09 +0800 Subject: [PATCH] reduce false positives on some common cases from if-let-rescope --- compiler/rustc_lint/src/if_let_rescope.rs | 7 +++-- tests/ui/drop/lint-if-let-rescope.fixed | 24 ++++++++++++---- tests/ui/drop/lint-if-let-rescope.rs | 22 +++++++++++---- tests/ui/drop/lint-if-let-rescope.stderr | 34 +++++------------------ 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs index 0e874669043..2db229ed133 100644 --- 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 index fec2e3b2ae7..182190aa323 100644 --- 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 { + const fn get(&self) -> Option { 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 index ee184695b97..e1b38be0a0f 100644 --- 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 { + const fn get(&self) -> Option { 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 index ef60d141b79..cfb7070c097 100644 --- 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 -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 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 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 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