From 6d1a25ad7e8ce026a0a2b0c85046afc81ef529ba Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Mon, 30 Sep 2024 03:40:48 +0800 Subject: [PATCH] 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 229d0c36421..8cb63d5cb05 100644 --- a/compiler/rustc_lint/src/if_let_rescope.rs +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -122,7 +122,11 @@ fn probe_if_cascade<'tcx>(&mut self, cx: &LateContext<'tcx>, mut expr: &'tcx hir } 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 @@ fn probe_if_cascade<'tcx>(&mut self, cx: &LateContext<'tcx>, mut expr: &'tcx hir 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 @@ fn probe_if_cascade<'tcx>(&mut self, cx: &LateContext<'tcx>, mut expr: &'tcx hir 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 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { 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 f228783f88b..d579ce98236 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 241fb897fce..aab293cdcfc 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 25ca3cf1ca8..50801ae166f 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