From 6d1a25ad7e8ce026a0a2b0c85046afc81ef529ba Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Mon, 30 Sep 2024 03:40:48 +0800 Subject: [PATCH 1/2] preserve brackets around if-lets and skip while-lets --- compiler/rustc_lint/src/if_let_rescope.rs | 29 +++++++++++++++++++++-- tests/ui/drop/lint-if-let-rescope.fixed | 15 +++++++++++- tests/ui/drop/lint-if-let-rescope.rs | 15 +++++++++++- tests/ui/drop/lint-if-let-rescope.stderr | 22 ++++++++++++++++- 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs index 229d0c3642174..8cb63d5cb0542 100644 --- a/compiler/rustc_lint/src/if_let_rescope.rs +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -122,7 +122,11 @@ impl IfLetRescope { } let tcx = cx.tcx; let source_map = tcx.sess.source_map(); - let expr_end = expr.span.shrink_to_hi(); + let expr_end = match expr.kind { + hir::ExprKind::If(_cond, conseq, None) => conseq.span.shrink_to_hi(), + hir::ExprKind::If(_cond, _conseq, Some(alt)) => alt.span.shrink_to_hi(), + _ => return, + }; let mut add_bracket_to_match_head = match_head_needs_bracket(tcx, expr); let mut significant_droppers = vec![]; let mut lifetime_ends = vec![]; @@ -145,7 +149,10 @@ impl IfLetRescope { recovered: Recovered::No, }) = cond.kind { - let if_let_pat = expr.span.shrink_to_lo().between(init.span); + // Peel off round braces + let if_let_pat = source_map + .span_take_while(expr.span, |&ch| ch == '(' || ch.is_whitespace()) + .between(init.span); // The consequent fragment is always a block. let before_conseq = conseq.span.shrink_to_lo(); let lifetime_end = source_map.end_point(conseq.span); @@ -159,6 +166,8 @@ impl IfLetRescope { if ty_ascription.is_some() || !expr.span.can_be_used_for_suggestions() || !pat.span.can_be_used_for_suggestions() + || !if_let_pat.can_be_used_for_suggestions() + || !before_conseq.can_be_used_for_suggestions() { // Our `match` rewrites does not support type ascription, // so we just bail. @@ -240,6 +249,22 @@ impl<'tcx> LateLintPass<'tcx> for IfLetRescope { if let (Level::Allow, _) = cx.tcx.lint_level_at_node(IF_LET_RESCOPE, expr.hir_id) { return; } + if let hir::ExprKind::Loop(block, _label, hir::LoopSource::While, _span) = expr.kind + && let Some(value) = block.expr + && let hir::ExprKind::If(cond, _conseq, _alt) = value.kind + && let hir::ExprKind::Let(..) = cond.kind + { + // Recall that `while let` is lowered into this: + // ``` + // loop { + // if let .. { body } else { break; } + // } + // ``` + // There is no observable from the `{ break; }` block so the edition change + // means nothing substantial to this `while` statement. + self.skip.insert(value.hir_id); + return; + } if expr_parent_is_stmt(cx.tcx, expr.hir_id) && matches!(expr.kind, hir::ExprKind::If(_cond, _conseq, None)) { diff --git a/tests/ui/drop/lint-if-let-rescope.fixed b/tests/ui/drop/lint-if-let-rescope.fixed index f228783f88bc5..d579ce9823691 100644 --- a/tests/ui/drop/lint-if-let-rescope.fixed +++ b/tests/ui/drop/lint-if-let-rescope.fixed @@ -2,7 +2,7 @@ #![deny(if_let_rescope)] #![feature(if_let_rescope)] -#![allow(irrefutable_let_patterns)] +#![allow(irrefutable_let_patterns, unused_parens)] fn droppy() -> Droppy { Droppy @@ -68,4 +68,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 } + + if (match droppy().get() { Some(_value) => { true } _ => { false }}) { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| HELP: the value is now dropped here in Edition 2024 + // do something + } + + while let Some(_value) = droppy().get() { + // Should not lint + break; + } } diff --git a/tests/ui/drop/lint-if-let-rescope.rs b/tests/ui/drop/lint-if-let-rescope.rs index 241fb897fce17..aab293cdcfc77 100644 --- a/tests/ui/drop/lint-if-let-rescope.rs +++ b/tests/ui/drop/lint-if-let-rescope.rs @@ -2,7 +2,7 @@ #![deny(if_let_rescope)] #![feature(if_let_rescope)] -#![allow(irrefutable_let_patterns)] +#![allow(irrefutable_let_patterns, unused_parens)] fn droppy() -> Droppy { Droppy @@ -68,4 +68,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 } + + if (if let Some(_value) = droppy().get() { true } else { false }) { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| HELP: the value is now dropped here in Edition 2024 + // do something + } + + while let Some(_value) = droppy().get() { + // Should not lint + break; + } } diff --git a/tests/ui/drop/lint-if-let-rescope.stderr b/tests/ui/drop/lint-if-let-rescope.stderr index 25ca3cf1ca873..50801ae166ff0 100644 --- a/tests/ui/drop/lint-if-let-rescope.stderr +++ b/tests/ui/drop/lint-if-let-rescope.stderr @@ -131,5 +131,25 @@ 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: aborting due to 5 previous errors +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:72:12 + | +LL | if (if let Some(_value) = droppy().get() { true } else { false }) { + | ^^^^^^^^^^^^^^^^^^^--------^^^^^^ + | | + | 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:72:53 + | +LL | if (if let Some(_value) = droppy().get() { true } else { false }) { + | ^ +help: a `match` with a single arm can preserve the drop order up to Edition 2021 + | +LL | if (match droppy().get() { Some(_value) => { true } _ => { false }}) { + | ~~~~~ +++++++++++++++++ ~~~~ + + +error: aborting due to 6 previous errors From ed5443fcdf3b4d02a89aec929cd62aa97586096f Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Mon, 30 Sep 2024 22:21:45 +0800 Subject: [PATCH 2/2] apply suggestions --- compiler/rustc_lint/src/if_let_rescope.rs | 3 +- tests/ui/drop/lint-if-let-rescope.fixed | 17 ++++++++- tests/ui/drop/lint-if-let-rescope.rs | 17 ++++++++- tests/ui/drop/lint-if-let-rescope.stderr | 46 +++++++++++++++++++++-- 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs index 8cb63d5cb0542..8f643de836a02 100644 --- a/compiler/rustc_lint/src/if_let_rescope.rs +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -260,7 +260,8 @@ impl<'tcx> LateLintPass<'tcx> for IfLetRescope { // if let .. { body } else { break; } // } // ``` - // There is no observable from the `{ break; }` block so the edition change + // There is no observable change in drop order on the overall `if let` expression + // given that the `{ break; }` block is trivial so the edition change // means nothing substantial to this `while` statement. self.skip.insert(value.hir_id); return; diff --git a/tests/ui/drop/lint-if-let-rescope.fixed b/tests/ui/drop/lint-if-let-rescope.fixed index d579ce9823691..199068d0fd265 100644 --- a/tests/ui/drop/lint-if-let-rescope.fixed +++ b/tests/ui/drop/lint-if-let-rescope.fixed @@ -1,7 +1,7 @@ //@ run-rustfix #![deny(if_let_rescope)] -#![feature(if_let_rescope)] +#![feature(if_let_rescope, stmt_expr_attributes)] #![allow(irrefutable_let_patterns, unused_parens)] fn droppy() -> Droppy { @@ -69,16 +69,29 @@ fn main() { //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 } + #[rustfmt::skip] if (match droppy().get() { Some(_value) => { true } _ => { false }}) { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 //~| 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 // do something + } else if (((match droppy().get() { Some(_value) => { true } _ => { false }}))) { + //~^ 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 } while let Some(_value) = droppy().get() { // Should not lint break; } + + while (match droppy().get() { Some(_value) => { false } _ => { true }}) { + //~^ 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 + } } diff --git a/tests/ui/drop/lint-if-let-rescope.rs b/tests/ui/drop/lint-if-let-rescope.rs index aab293cdcfc77..4c043c0266cc4 100644 --- a/tests/ui/drop/lint-if-let-rescope.rs +++ b/tests/ui/drop/lint-if-let-rescope.rs @@ -1,7 +1,7 @@ //@ run-rustfix #![deny(if_let_rescope)] -#![feature(if_let_rescope)] +#![feature(if_let_rescope, stmt_expr_attributes)] #![allow(irrefutable_let_patterns, unused_parens)] fn droppy() -> Droppy { @@ -69,16 +69,29 @@ fn main() { //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 } + #[rustfmt::skip] if (if let Some(_value) = droppy().get() { true } else { false }) { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 //~| 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 // do something + } else if (((if let Some(_value) = droppy().get() { true } else { false }))) { + //~^ 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 } while let Some(_value) = droppy().get() { // Should not lint break; } + + while (if let Some(_value) = droppy().get() { false } else { true }) { + //~^ 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 + } } diff --git a/tests/ui/drop/lint-if-let-rescope.stderr b/tests/ui/drop/lint-if-let-rescope.stderr index 50801ae166ff0..ef60d141b7988 100644 --- a/tests/ui/drop/lint-if-let-rescope.stderr +++ b/tests/ui/drop/lint-if-let-rescope.stderr @@ -132,7 +132,7 @@ LL | if let () = { match Droppy.get() { Some(_value) => {} _ => {}} } { | ~~~~~ +++++++++++++++++ ++++++++ error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:72:12 + --> $DIR/lint-if-let-rescope.rs:73:12 | LL | if (if let Some(_value) = droppy().get() { true } else { false }) { | ^^^^^^^^^^^^^^^^^^^--------^^^^^^ @@ -142,7 +142,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:72:53 + --> $DIR/lint-if-let-rescope.rs:73:53 | LL | if (if let Some(_value) = droppy().get() { true } else { false }) { | ^ @@ -151,5 +151,45 @@ help: a `match` with a single arm can preserve the drop order up to Edition 2021 LL | if (match droppy().get() { Some(_value) => { true } _ => { false }}) { | ~~~~~ +++++++++++++++++ ~~~~ + -error: aborting due to 6 previous errors +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:79:21 + | +LL | } else if (((if let Some(_value) = droppy().get() { true } else { false }))) { + | ^^^^^^^^^^^^^^^^^^^--------^^^^^^ + | | + | 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:79:62 + | +LL | } else if (((if let Some(_value) = droppy().get() { true } else { false }))) { + | ^ +help: a `match` with a single arm can preserve the drop order up to Edition 2021 + | +LL | } else if (((match droppy().get() { Some(_value) => { true } _ => { false }}))) { + | ~~~~~ +++++++++++++++++ ~~~~ + + +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:91:15 + | +LL | while (if let Some(_value) = droppy().get() { false } else { true }) { + | ^^^^^^^^^^^^^^^^^^^--------^^^^^^ + | | + | 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:91:57 + | +LL | while (if let Some(_value) = droppy().get() { false } else { true }) { + | ^ +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