From f93df1f7dcea3588620b77e2a0eaac12b46a7678 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Tue, 24 Jan 2023 16:06:35 +0800 Subject: [PATCH 1/4] rescope temp lifetime in let-chain into IfElse apply rules by span edition --- .../src/diagnostics/conflict_errors.rs | 29 +- .../src/diagnostics/explain_borrow.rs | 95 +++++- compiler/rustc_feature/src/unstable.rs | 2 + .../rustc_hir_analysis/src/check/region.rs | 14 +- compiler/rustc_lint/messages.ftl | 5 + compiler/rustc_lint/src/if_let_rescope.rs | 312 ++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_middle/src/middle/region.rs | 6 + compiler/rustc_middle/src/mir/mod.rs | 3 + compiler/rustc_middle/src/ty/rvalue_scopes.rs | 10 +- .../rustc_mir_build/src/build/expr/as_temp.rs | 10 + compiler/rustc_mir_build/src/thir/cx/expr.rs | 8 +- compiler/rustc_span/src/symbol.rs | 1 + tests/ui/drop/drop_order.rs | 22 ++ tests/ui/drop/drop_order_if_let_rescope.rs | 122 +++++++ .../if-let-rescope-borrowck-suggestions.fixed | 26 ++ .../if-let-rescope-borrowck-suggestions.rs | 25 ++ ...if-let-rescope-borrowck-suggestions.stderr | 26 ++ tests/ui/drop/lint-if-let-rescope-gated.rs | 31 ++ ...let-rescope-gated.with_feature_gate.stderr | 30 ++ .../ui/drop/lint-if-let-rescope-with-macro.rs | 41 +++ .../lint-if-let-rescope-with-macro.stderr | 39 +++ tests/ui/drop/lint-if-let-rescope.fixed | 48 +++ tests/ui/drop/lint-if-let-rescope.rs | 48 +++ tests/ui/drop/lint-if-let-rescope.stderr | 74 +++++ .../feature-gate-if-let-rescope.rs | 27 ++ .../feature-gate-if-let-rescope.stderr | 18 + tests/ui/mir/mir_let_chains_drop_order.rs | 41 ++- ...=> issue-54556-niconii.edition2021.stderr} | 2 +- tests/ui/nll/issue-54556-niconii.rs | 19 +- 30 files changed, 1102 insertions(+), 35 deletions(-) create mode 100644 compiler/rustc_lint/src/if_let_rescope.rs create mode 100644 tests/ui/drop/drop_order_if_let_rescope.rs create mode 100644 tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed create mode 100644 tests/ui/drop/if-let-rescope-borrowck-suggestions.rs create mode 100644 tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr create mode 100644 tests/ui/drop/lint-if-let-rescope-gated.rs create mode 100644 tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr create mode 100644 tests/ui/drop/lint-if-let-rescope-with-macro.rs create mode 100644 tests/ui/drop/lint-if-let-rescope-with-macro.stderr create mode 100644 tests/ui/drop/lint-if-let-rescope.fixed create mode 100644 tests/ui/drop/lint-if-let-rescope.rs create mode 100644 tests/ui/drop/lint-if-let-rescope.stderr create mode 100644 tests/ui/feature-gates/feature-gate-if-let-rescope.rs create mode 100644 tests/ui/feature-gates/feature-gate-if-let-rescope.stderr rename tests/ui/nll/{issue-54556-niconii.stderr => issue-54556-niconii.edition2021.stderr} (96%) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index a47518fca3f..215e0476f01 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1999,19 +1999,32 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ) { let used_in_call = matches!( explanation, - BorrowExplanation::UsedLater(LaterUseKind::Call | LaterUseKind::Other, _call_span, _) + BorrowExplanation::UsedLater( + _, + LaterUseKind::Call | LaterUseKind::Other, + _call_span, + _ + ) ); if !used_in_call { debug!("not later used in call"); return; } + if matches!( + self.body.local_decls[issued_borrow.borrowed_place.local].local_info(), + LocalInfo::IfThenRescopeTemp { .. } + ) { + // A better suggestion will be issued by the `if_let_rescope` lint + return; + } - let use_span = - if let BorrowExplanation::UsedLater(LaterUseKind::Other, use_span, _) = explanation { - Some(use_span) - } else { - None - }; + let use_span = if let BorrowExplanation::UsedLater(_, LaterUseKind::Other, use_span, _) = + explanation + { + Some(use_span) + } else { + None + }; let outer_call_loc = if let TwoPhaseActivation::ActivatedAt(loc) = issued_borrow.activation_location { @@ -2862,7 +2875,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // and `move` will not help here. ( Some(name), - BorrowExplanation::UsedLater(LaterUseKind::ClosureCapture, var_or_use_span, _), + BorrowExplanation::UsedLater(_, LaterUseKind::ClosureCapture, var_or_use_span, _), ) if borrow_spans.for_coroutine() || borrow_spans.for_closure() => self .report_escaping_closure_capture( borrow_spans, diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index 419e72df00b..22327d2ba0d 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -30,7 +30,7 @@ use crate::{MirBorrowckCtxt, WriteKind}; #[derive(Debug)] pub(crate) enum BorrowExplanation<'tcx> { - UsedLater(LaterUseKind, Span, Option), + UsedLater(Local, LaterUseKind, Span, Option), UsedLaterInLoop(LaterUseKind, Span, Option), UsedLaterWhenDropped { drop_loc: Location, @@ -99,7 +99,12 @@ impl<'tcx> BorrowExplanation<'tcx> { } } match *self { - BorrowExplanation::UsedLater(later_use_kind, var_or_use_span, path_span) => { + BorrowExplanation::UsedLater( + dropped_local, + later_use_kind, + var_or_use_span, + path_span, + ) => { let message = match later_use_kind { LaterUseKind::TraitCapture => "captured here by trait object", LaterUseKind::ClosureCapture => "captured here by closure", @@ -107,9 +112,26 @@ impl<'tcx> BorrowExplanation<'tcx> { LaterUseKind::FakeLetRead => "stored here", LaterUseKind::Other => "used here", }; - // We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same - if path_span.map(|path_span| path_span == var_or_use_span).unwrap_or(true) { - if borrow_span.map(|sp| !sp.overlaps(var_or_use_span)).unwrap_or(true) { + let local_decl = &body.local_decls[dropped_local]; + + if let &LocalInfo::IfThenRescopeTemp { if_then } = local_decl.local_info() + && let Some((_, hir::Node::Expr(expr))) = tcx.hir().parent_iter(if_then).next() + && let hir::ExprKind::If(cond, conseq, alt) = expr.kind + && let hir::ExprKind::Let(&hir::LetExpr { + span: _, + pat, + init, + // FIXME(#101728): enable rewrite when type ascription is stabilized again + ty: None, + recovered: _, + }) = cond.kind + && pat.span.can_be_used_for_suggestions() + && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span) + { + suggest_rewrite_if_let(expr, &pat, init, conseq, alt, err); + } else if path_span.map_or(true, |path_span| path_span == var_or_use_span) { + // We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same + if borrow_span.map_or(true, |sp| !sp.overlaps(var_or_use_span)) { err.span_label( var_or_use_span, format!("{borrow_desc}borrow later {message}"), @@ -255,6 +277,22 @@ impl<'tcx> BorrowExplanation<'tcx> { Applicability::MaybeIncorrect, ); }; + } else if let &LocalInfo::IfThenRescopeTemp { if_then } = + local_decl.local_info() + && let hir::Node::Expr(expr) = tcx.hir_node(if_then) + && let hir::ExprKind::If(cond, conseq, alt) = expr.kind + && let hir::ExprKind::Let(&hir::LetExpr { + span: _, + pat, + init, + // FIXME(#101728): enable rewrite when type ascription is stabilized again + ty: None, + recovered: _, + }) = cond.kind + && pat.span.can_be_used_for_suggestions() + && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span) + { + suggest_rewrite_if_let(expr, &pat, init, conseq, alt, err); } } } @@ -390,6 +428,38 @@ impl<'tcx> BorrowExplanation<'tcx> { } } +fn suggest_rewrite_if_let<'tcx>( + expr: &hir::Expr<'tcx>, + pat: &str, + init: &hir::Expr<'tcx>, + conseq: &hir::Expr<'tcx>, + alt: Option<&hir::Expr<'tcx>>, + err: &mut Diag<'_>, +) { + err.span_note( + conseq.span.shrink_to_hi(), + "lifetime for temporaries generated in `if let`s have been shorted in Edition 2024", + ); + if expr.span.can_be_used_for_suggestions() && conseq.span.can_be_used_for_suggestions() { + let mut sugg = vec![ + (expr.span.shrink_to_lo().between(init.span), "match ".into()), + (conseq.span.shrink_to_lo(), format!(" {{ {pat} => ")), + ]; + let expr_end = expr.span.shrink_to_hi(); + if let Some(alt) = alt { + sugg.push((conseq.span.between(alt.span), format!(" _ => "))); + sugg.push((expr_end, "}".into())); + } else { + sugg.push((expr_end, " _ => {} }".into())); + } + err.multipart_suggestion( + "consider rewriting the `if` into `match` which preserves the extended lifetime", + sugg, + Applicability::MachineApplicable, + ); + } +} + impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> { fn free_region_constraint_info( &self, @@ -465,14 +535,21 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> { .or_else(|| self.borrow_spans(span, location)); if use_in_later_iteration_of_loop { - let later_use = self.later_use_kind(borrow, spans, use_location); - BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1, later_use.2) + let (later_use_kind, var_or_use_span, path_span) = + self.later_use_kind(borrow, spans, use_location); + BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span, path_span) } else { // Check if the location represents a `FakeRead`, and adapt the error // message to the `FakeReadCause` it is from: in particular, // the ones inserted in optimized `let var = ` patterns. - let later_use = self.later_use_kind(borrow, spans, location); - BorrowExplanation::UsedLater(later_use.0, later_use.1, later_use.2) + let (later_use_kind, var_or_use_span, path_span) = + self.later_use_kind(borrow, spans, location); + BorrowExplanation::UsedLater( + borrow.borrowed_place.local, + later_use_kind, + var_or_use_span, + path_span, + ) } } diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 3254dab9a03..1359b6248dc 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -497,6 +497,8 @@ declare_features! ( (unstable, half_open_range_patterns_in_slices, "1.66.0", Some(67264)), /// Allows `if let` guard in match arms. (unstable, if_let_guard, "1.47.0", Some(51114)), + /// Rescoping temporaries in `if let` to align with Rust 2024. + (unstable, if_let_rescope, "CURRENT_RUSTC_VERSION", Some(124085)), /// Allows `impl Trait` to be used inside associated types (RFC 2515). (unstable, impl_trait_in_assoc_type, "1.70.0", Some(63063)), /// Allows `impl Trait` as output type in `Fn` traits in return position of functions. diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index 2d6147cff2a..33e58a92e37 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -472,7 +472,12 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h hir::ExprKind::If(cond, then, Some(otherwise)) => { let expr_cx = visitor.cx; - visitor.enter_scope(Scope { id: then.hir_id.local_id, data: ScopeData::IfThen }); + let data = if expr.span.at_least_rust_2024() && visitor.tcx.features().if_let_rescope { + ScopeData::IfThenRescope + } else { + ScopeData::IfThen + }; + visitor.enter_scope(Scope { id: then.hir_id.local_id, data }); visitor.cx.var_parent = visitor.cx.parent; visitor.visit_expr(cond); visitor.visit_expr(then); @@ -482,7 +487,12 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h hir::ExprKind::If(cond, then, None) => { let expr_cx = visitor.cx; - visitor.enter_scope(Scope { id: then.hir_id.local_id, data: ScopeData::IfThen }); + let data = if expr.span.at_least_rust_2024() && visitor.tcx.features().if_let_rescope { + ScopeData::IfThenRescope + } else { + ScopeData::IfThen + }; + visitor.enter_scope(Scope { id: then.hir_id.local_id, data }); visitor.cx.var_parent = visitor.cx.parent; visitor.visit_expr(cond); visitor.visit_expr(then); diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 7d4dee45c26..1f8dca337fc 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -334,6 +334,11 @@ lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> *[other] {" "}{$identifier_type} } Unicode general security profile +lint_if_let_rescope = `if let` assigns a shorter lifetime since Edition 2024 + .label = this value has a significant drop implementation which may observe a major change in drop order and requires your discretion + .help = the value is now dropped here in Edition 2024 + .suggestion = rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level lint_ill_formed_attribute_input = {$num_suggestions -> diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs new file mode 100644 index 00000000000..143965ae58f --- /dev/null +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -0,0 +1,312 @@ +use std::ops::ControlFlow; + +use hir::intravisit::Visitor; +use rustc_ast::Recovered; +use rustc_hir as hir; +use rustc_macros::{LintDiagnostic, Subdiagnostic}; +use rustc_session::lint::FutureIncompatibilityReason; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::edition::Edition; +use rustc_span::Span; + +use crate::{LateContext, LateLintPass}; + +declare_lint! { + /// The `if_let_rescope` lint detects cases where a temporary value with + /// significant drop is generated on the right hand side of `if let` + /// and suggests a rewrite into `match` when possible. + /// + /// ### Example + /// + /// ```rust,edition2021 + /// #![feature(if_let_rescope)] + /// #![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 { + /// None + /// } + /// } + /// + /// fn main() { + /// if let Some(value) = Droppy.get() { + /// // do something + /// } else { + /// // do something else + /// } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// With Edition 2024, temporaries generated while evaluating `if let`s + /// will be dropped before the `else` block. + /// This lint captures a possible change in runtime behaviour due to + /// a change in sequence of calls to significant `Drop::drop` destructors. + /// + /// A significant [`Drop::drop`](https://doc.rust-lang.org/std/ops/trait.Drop.html) + /// destructor here refers to an explicit, arbitrary implementation of the `Drop` trait on the type + /// with exceptions including `Vec`, `Box`, `Rc`, `BTreeMap` and `HashMap` + /// that are marked by the compiler otherwise so long that the generic types have + /// no significant destructor recursively. + /// In other words, a type has a significant drop destructor when it has a `Drop` implementation + /// or its destructor invokes a significant destructor on a type. + /// Since we cannot completely reason about the change by just inspecting the existence of + /// a significant destructor, this lint remains only a suggestion and is set to `allow` by default. + /// + /// Whenever possible, a rewrite into an equivalent `match` expression that + /// observe the same order of calls to such destructors is proposed by this lint. + /// Authors may take their own discretion whether the rewrite suggestion shall be + /// accepted, or rejected to continue the use of the `if let` expression. + pub IF_LET_RESCOPE, + Allow, + "`if let` assigns a shorter lifetime to temporary values being pattern-matched against in Edition 2024 and \ + rewriting in `match` is an option to preserve the semantics up to Edition 2021", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), + reference: "issue #124085 ", + }; +} + +declare_lint_pass!( + /// Lint for potential change in program semantics of `if let`s + IfLetRescope => [IF_LET_RESCOPE] +); + +impl<'tcx> LateLintPass<'tcx> for IfLetRescope { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { + if !expr.span.edition().at_least_rust_2021() || !cx.tcx.features().if_let_rescope { + return; + } + let hir::ExprKind::If(cond, conseq, alt) = expr.kind else { return }; + let hir::ExprKind::Let(&hir::LetExpr { + span, + pat, + init, + ty: ty_ascription, + recovered: Recovered::No, + }) = cond.kind + else { + return; + }; + let source_map = cx.tcx.sess.source_map(); + let expr_end = expr.span.shrink_to_hi(); + let if_let_pat = expr.span.shrink_to_lo().between(init.span); + let before_conseq = conseq.span.shrink_to_lo(); + let lifetime_end = source_map.end_point(conseq.span); + + if let ControlFlow::Break(significant_dropper) = + (FindSignificantDropper { cx }).visit_expr(init) + { + let lint_without_suggestion = || { + cx.tcx.emit_node_span_lint( + IF_LET_RESCOPE, + expr.hir_id, + span, + IfLetRescopeRewrite { significant_dropper, lifetime_end, sugg: None }, + ) + }; + if ty_ascription.is_some() + || !expr.span.can_be_used_for_suggestions() + || !pat.span.can_be_used_for_suggestions() + { + // Our `match` rewrites does not support type ascription, + // so we just bail. + // Alternatively when the span comes from proc macro expansion, + // we will also bail. + // FIXME(#101728): change this when type ascription syntax is stabilized again + lint_without_suggestion(); + } else { + let Ok(pat) = source_map.span_to_snippet(pat.span) else { + lint_without_suggestion(); + return; + }; + if let Some(alt) = alt { + let alt_start = conseq.span.between(alt.span); + if !alt_start.can_be_used_for_suggestions() { + lint_without_suggestion(); + return; + } + cx.tcx.emit_node_span_lint( + IF_LET_RESCOPE, + expr.hir_id, + span, + IfLetRescopeRewrite { + significant_dropper, + lifetime_end, + sugg: Some(IfLetRescopeRewriteSuggestion::WithElse { + if_let_pat, + before_conseq, + pat, + expr_end, + alt_start, + }), + }, + ); + } else { + cx.tcx.emit_node_span_lint( + IF_LET_RESCOPE, + expr.hir_id, + span, + IfLetRescopeRewrite { + significant_dropper, + lifetime_end, + sugg: Some(IfLetRescopeRewriteSuggestion::WithoutElse { + if_let_pat, + before_conseq, + pat, + expr_end, + }), + }, + ); + } + } + } + } +} + +#[derive(LintDiagnostic)] +#[diag(lint_if_let_rescope)] +struct IfLetRescopeRewrite { + #[label] + significant_dropper: Span, + #[help] + lifetime_end: Span, + #[subdiagnostic] + sugg: Option, +} + +#[derive(Subdiagnostic)] +enum IfLetRescopeRewriteSuggestion { + #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] + WithElse { + #[suggestion_part(code = "match ")] + if_let_pat: Span, + #[suggestion_part(code = " {{ {pat} => ")] + before_conseq: Span, + pat: String, + #[suggestion_part(code = "}}")] + expr_end: Span, + #[suggestion_part(code = " _ => ")] + alt_start: Span, + }, + #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] + WithoutElse { + #[suggestion_part(code = "match ")] + if_let_pat: Span, + #[suggestion_part(code = " {{ {pat} => ")] + before_conseq: Span, + pat: String, + #[suggestion_part(code = " _ => {{}} }}")] + expr_end: Span, + }, +} + +struct FindSignificantDropper<'tcx, 'a> { + cx: &'a LateContext<'tcx>, +} + +impl<'tcx, 'a> Visitor<'tcx> for FindSignificantDropper<'tcx, 'a> { + type Result = ControlFlow; + + fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Self::Result { + if self + .cx + .typeck_results() + .expr_ty(expr) + .has_significant_drop(self.cx.tcx, self.cx.param_env) + { + return ControlFlow::Break(expr.span); + } + match expr.kind { + hir::ExprKind::ConstBlock(_) + | hir::ExprKind::Lit(_) + | hir::ExprKind::Path(_) + | hir::ExprKind::Assign(_, _, _) + | hir::ExprKind::AssignOp(_, _, _) + | hir::ExprKind::Break(_, _) + | hir::ExprKind::Continue(_) + | hir::ExprKind::Ret(_) + | hir::ExprKind::Become(_) + | hir::ExprKind::InlineAsm(_) + | hir::ExprKind::OffsetOf(_, _) + | hir::ExprKind::Repeat(_, _) + | hir::ExprKind::Err(_) + | hir::ExprKind::Struct(_, _, _) + | hir::ExprKind::Closure(_) + | hir::ExprKind::Block(_, _) + | hir::ExprKind::DropTemps(_) + | hir::ExprKind::Loop(_, _, _, _) => ControlFlow::Continue(()), + + hir::ExprKind::Tup(exprs) | hir::ExprKind::Array(exprs) => { + for expr in exprs { + self.visit_expr(expr)?; + } + ControlFlow::Continue(()) + } + hir::ExprKind::Call(callee, args) => { + self.visit_expr(callee)?; + for expr in args { + self.visit_expr(expr)?; + } + ControlFlow::Continue(()) + } + hir::ExprKind::MethodCall(_, receiver, args, _) => { + self.visit_expr(receiver)?; + for expr in args { + self.visit_expr(expr)?; + } + ControlFlow::Continue(()) + } + hir::ExprKind::Index(left, right, _) | hir::ExprKind::Binary(_, left, right) => { + self.visit_expr(left)?; + self.visit_expr(right) + } + hir::ExprKind::Unary(_, expr) + | hir::ExprKind::Cast(expr, _) + | hir::ExprKind::Type(expr, _) + | hir::ExprKind::Yield(expr, _) + | hir::ExprKind::AddrOf(_, _, expr) + | hir::ExprKind::Match(expr, _, _) + | hir::ExprKind::Field(expr, _) + | hir::ExprKind::Let(&hir::LetExpr { + init: expr, + span: _, + pat: _, + ty: _, + recovered: Recovered::No, + }) => self.visit_expr(expr), + hir::ExprKind::Let(_) => ControlFlow::Continue(()), + + hir::ExprKind::If(cond, _, _) => { + if let hir::ExprKind::Let(hir::LetExpr { + init, + span: _, + pat: _, + ty: _, + recovered: Recovered::No, + }) = cond.kind + { + self.visit_expr(init)?; + } + ControlFlow::Continue(()) + } + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index a9627e9b789..58b51240a90 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -56,6 +56,7 @@ mod expect; mod for_loops_over_fallibles; mod foreign_modules; pub mod hidden_unicode_codepoints; +mod if_let_rescope; mod impl_trait_overcaptures; mod internal; mod invalid_from_utf8; @@ -94,6 +95,7 @@ use drop_forget_useless::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; use for_loops_over_fallibles::*; use hidden_unicode_codepoints::*; +use if_let_rescope::IfLetRescope; use impl_trait_overcaptures::ImplTraitOvercaptures; use internal::*; use invalid_from_utf8::*; @@ -243,6 +245,7 @@ late_lint_methods!( NonLocalDefinitions: NonLocalDefinitions::default(), ImplTraitOvercaptures: ImplTraitOvercaptures, TailExprDropOrder: TailExprDropOrder, + IfLetRescope: IfLetRescope, ] ] ); diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index 6ef1801717c..999743ed73b 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -96,6 +96,7 @@ impl fmt::Debug for Scope { ScopeData::Arguments => write!(fmt, "Arguments({:?})", self.id), ScopeData::Destruction => write!(fmt, "Destruction({:?})", self.id), ScopeData::IfThen => write!(fmt, "IfThen({:?})", self.id), + ScopeData::IfThenRescope => write!(fmt, "IfThen[edition2024]({:?})", self.id), ScopeData::Remainder(fsi) => write!( fmt, "Remainder {{ block: {:?}, first_statement_index: {}}}", @@ -126,6 +127,11 @@ pub enum ScopeData { /// Used for variables introduced in an if-let expression. IfThen, + /// Scope of the condition and then block of an if expression + /// Used for variables introduced in an if-let expression, + /// whose lifetimes do not cross beyond this scope. + IfThenRescope, + /// Scope following a `let id = expr;` binding in a block. Remainder(FirstStatementIndex), } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 081a23b6ff3..557d2b29e75 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1084,6 +1084,9 @@ pub enum LocalInfo<'tcx> { /// (with no intervening statement context). // FIXME(matthewjasper) Don't store in this in `Body` BlockTailTemp(BlockTailInfo), + /// A temporary created during evaluating `if` predicate, possibly for pattern matching for `let`s, + /// and subject to Edition 2024 temporary lifetime rules + IfThenRescopeTemp { if_then: HirId }, /// A temporary created during the pass `Derefer` to avoid it's retagging DerefTemp, /// A temporary created for borrow checking. diff --git a/compiler/rustc_middle/src/ty/rvalue_scopes.rs b/compiler/rustc_middle/src/ty/rvalue_scopes.rs index bcab54cf8ba..37dcf7c0d64 100644 --- a/compiler/rustc_middle/src/ty/rvalue_scopes.rs +++ b/compiler/rustc_middle/src/ty/rvalue_scopes.rs @@ -41,7 +41,15 @@ impl RvalueScopes { debug!("temporary_scope({expr_id:?}) = {id:?} [enclosing]"); return Some(id); } - _ => id = p, + ScopeData::IfThenRescope => { + debug!("temporary_scope({expr_id:?}) = {p:?} [enclosing]"); + return Some(p); + } + ScopeData::Node + | ScopeData::CallSite + | ScopeData::Arguments + | ScopeData::IfThen + | ScopeData::Remainder(_) => id = p, } } diff --git a/compiler/rustc_mir_build/src/build/expr/as_temp.rs b/compiler/rustc_mir_build/src/build/expr/as_temp.rs index af5940ff50e..b8b5e4634ed 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_temp.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_temp.rs @@ -1,7 +1,9 @@ //! See docs in build/expr/mod.rs use rustc_data_structures::stack::ensure_sufficient_stack; +use rustc_hir::HirId; use rustc_middle::middle::region; +use rustc_middle::middle::region::{Scope, ScopeData}; use rustc_middle::mir::*; use rustc_middle::thir::*; use tracing::{debug, instrument}; @@ -73,11 +75,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { _ if let Some(tail_info) = this.block_context.currently_in_block_tail() => { LocalInfo::BlockTailTemp(tail_info) } + + _ if let Some(Scope { data: ScopeData::IfThenRescope, id }) = temp_lifetime => { + LocalInfo::IfThenRescopeTemp { + if_then: HirId { owner: this.hir_id.owner, local_id: id }, + } + } + _ => LocalInfo::Boring, }; **local_decl.local_info.as_mut().assert_crate_local() = local_info; this.local_decls.push(local_decl) }; + debug!(?temp); if deduplicate_temps { this.fixed_temps.insert(expr_id, temp); } diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 89f98a40201..aa8ccc8b7dd 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -706,7 +706,13 @@ impl<'tcx> Cx<'tcx> { hir::ExprKind::If(cond, then, else_opt) => ExprKind::If { if_then_scope: region::Scope { id: then.hir_id.local_id, - data: region::ScopeData::IfThen, + data: { + if expr.span.at_least_rust_2024() && tcx.features().if_let_rescope { + region::ScopeData::IfThenRescope + } else { + region::ScopeData::IfThen + } + }, }, cond: self.mirror_expr(cond), then: self.mirror_expr(then), diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d5240a05310..d340b56fbe4 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1017,6 +1017,7 @@ symbols! { ident, if_let, if_let_guard, + if_let_rescope, if_while_or_patterns, ignore, impl_header_lifetime_elision, diff --git a/tests/ui/drop/drop_order.rs b/tests/ui/drop/drop_order.rs index 54e9e491f78..cf062538007 100644 --- a/tests/ui/drop/drop_order.rs +++ b/tests/ui/drop/drop_order.rs @@ -1,6 +1,11 @@ //@ run-pass //@ compile-flags: -Z validate-mir +//@ revisions: edition2021 edition2024 +//@ [edition2021] edition: 2021 +//@ [edition2024] compile-flags: -Z unstable-options +//@ [edition2024] edition: 2024 #![feature(let_chains)] +#![cfg_attr(edition2024, feature(if_let_rescope))] use std::cell::RefCell; use std::convert::TryInto; @@ -55,11 +60,18 @@ impl DropOrderCollector { } fn if_let(&self) { + #[cfg(edition2021)] if let None = self.option_loud_drop(2) { unreachable!(); } else { self.print(1); } + #[cfg(edition2024)] + if let None = self.option_loud_drop(1) { + unreachable!(); + } else { + self.print(2); + } if let Some(_) = self.option_loud_drop(4) { self.print(3); @@ -194,6 +206,7 @@ impl DropOrderCollector { self.print(3); // 3 } + #[cfg(edition2021)] // take the "else" branch if self.option_loud_drop(5).is_some() // 1 && self.option_loud_drop(6).is_some() // 2 @@ -202,6 +215,15 @@ impl DropOrderCollector { } else { self.print(7); // 3 } + #[cfg(edition2024)] + // take the "else" branch + if self.option_loud_drop(5).is_some() // 1 + && self.option_loud_drop(6).is_some() // 2 + && let None = self.option_loud_drop(7) { // 4 + unreachable!(); + } else { + self.print(8); // 3 + } // let exprs interspersed if self.option_loud_drop(9).is_some() // 1 diff --git a/tests/ui/drop/drop_order_if_let_rescope.rs b/tests/ui/drop/drop_order_if_let_rescope.rs new file mode 100644 index 00000000000..ae9f381820e --- /dev/null +++ b/tests/ui/drop/drop_order_if_let_rescope.rs @@ -0,0 +1,122 @@ +//@ run-pass +//@ edition:2024 +//@ compile-flags: -Z validate-mir -Zunstable-options + +#![feature(let_chains)] +#![feature(if_let_rescope)] + +use std::cell::RefCell; +use std::convert::TryInto; + +#[derive(Default)] +struct DropOrderCollector(RefCell>); + +struct LoudDrop<'a>(&'a DropOrderCollector, u32); + +impl Drop for LoudDrop<'_> { + fn drop(&mut self) { + println!("{}", self.1); + self.0.0.borrow_mut().push(self.1); + } +} + +impl DropOrderCollector { + fn option_loud_drop(&self, n: u32) -> Option { + Some(LoudDrop(self, n)) + } + + fn print(&self, n: u32) { + println!("{}", n); + self.0.borrow_mut().push(n) + } + + fn assert_sorted(self) { + assert!( + self.0 + .into_inner() + .into_iter() + .enumerate() + .all(|(idx, item)| idx + 1 == item.try_into().unwrap()) + ); + } + + fn if_let(&self) { + if let None = self.option_loud_drop(1) { + unreachable!(); + } else { + self.print(2); + } + + if let Some(_) = self.option_loud_drop(4) { + self.print(3); + } + + if let Some(_d) = self.option_loud_drop(6) { + self.print(5); + } + } + + fn let_chain(&self) { + // take the "then" branch + if self.option_loud_drop(1).is_some() // 1 + && self.option_loud_drop(2).is_some() // 2 + && let Some(_d) = self.option_loud_drop(4) + // 4 + { + self.print(3); // 3 + } + + // take the "else" branch + if self.option_loud_drop(5).is_some() // 1 + && self.option_loud_drop(6).is_some() // 2 + && let None = self.option_loud_drop(7) + // 3 + { + unreachable!(); + } else { + self.print(8); // 4 + } + + // let exprs interspersed + if self.option_loud_drop(9).is_some() // 1 + && let Some(_d) = self.option_loud_drop(13) // 5 + && self.option_loud_drop(10).is_some() // 2 + && let Some(_e) = self.option_loud_drop(12) + // 4 + { + self.print(11); // 3 + } + + // let exprs first + if let Some(_d) = self.option_loud_drop(18) // 5 + && let Some(_e) = self.option_loud_drop(17) // 4 + && self.option_loud_drop(14).is_some() // 1 + && self.option_loud_drop(15).is_some() + // 2 + { + self.print(16); // 3 + } + + // let exprs last + if self.option_loud_drop(19).is_some() // 1 + && self.option_loud_drop(20).is_some() // 2 + && let Some(_d) = self.option_loud_drop(23) // 5 + && let Some(_e) = self.option_loud_drop(22) + // 4 + { + self.print(21); // 3 + } + } +} + +fn main() { + println!("-- if let --"); + let collector = DropOrderCollector::default(); + collector.if_let(); + collector.assert_sorted(); + + println!("-- let chain --"); + let collector = DropOrderCollector::default(); + collector.let_chain(); + collector.assert_sorted(); +} diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed b/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed new file mode 100644 index 00000000000..129e78ea9fe --- /dev/null +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed @@ -0,0 +1,26 @@ +//@ edition: 2024 +//@ compile-flags: -Z validate-mir -Zunstable-options +//@ run-rustfix + +#![feature(if_let_rescope)] +#![deny(if_let_rescope)] + +struct Droppy; +impl Drop for Droppy { + fn drop(&mut self) { + println!("dropped"); + } +} +impl Droppy { + fn get_ref(&self) -> Option<&u8> { + None + } +} + +fn do_something(_: &T) {} + +fn main() { + let binding = Droppy; + do_something(match binding.get_ref() { Some(value) => { value } _ => { &0 }}); + //~^ ERROR: temporary value dropped while borrowed +} diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs new file mode 100644 index 00000000000..1970d5c0f7e --- /dev/null +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs @@ -0,0 +1,25 @@ +//@ edition: 2024 +//@ compile-flags: -Z validate-mir -Zunstable-options +//@ run-rustfix + +#![feature(if_let_rescope)] +#![deny(if_let_rescope)] + +struct Droppy; +impl Drop for Droppy { + fn drop(&mut self) { + println!("dropped"); + } +} +impl Droppy { + fn get_ref(&self) -> Option<&u8> { + None + } +} + +fn do_something(_: &T) {} + +fn main() { + do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); + //~^ ERROR: temporary value dropped while borrowed +} diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr b/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr new file mode 100644 index 00000000000..fa154f01a12 --- /dev/null +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr @@ -0,0 +1,26 @@ +error[E0716]: temporary value dropped while borrowed + --> $DIR/if-let-rescope-borrowck-suggestions.rs:23:39 + | +LL | do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); + | ^^^^^^ - temporary value is freed at the end of this statement + | | + | creates a temporary value which is freed while still in use + | +note: lifetime for temporaries generated in `if let`s have been shorted in Edition 2024 + --> $DIR/if-let-rescope-borrowck-suggestions.rs:23:65 + | +LL | do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); + | ^ +help: consider using a `let` binding to create a longer lived value + | +LL ~ let binding = Droppy; +LL ~ do_something(if let Some(value) = binding.get_ref() { value } else { &0 }); + | +help: consider rewriting the `if` into `match` which preserves the extended lifetime + | +LL | do_something(match Droppy.get_ref() { Some(value) => { value } _ => { &0 }}); + | ~~~~~ ++++++++++++++++ ~~~~ + + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0716`. diff --git a/tests/ui/drop/lint-if-let-rescope-gated.rs b/tests/ui/drop/lint-if-let-rescope-gated.rs new file mode 100644 index 00000000000..f8844a09f7c --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope-gated.rs @@ -0,0 +1,31 @@ +// This test checks that the lint `if_let_rescope` only actions +// when the feature gate is enabled. +// Edition 2021 is used here because the lint should work especially +// when edition migration towards 2024 is run. + +//@ revisions: with_feature_gate without_feature_gate +//@ [without_feature_gate] check-pass +//@ edition: 2021 + +#![cfg_attr(with_feature_gate, feature(if_let_rescope))] +#![deny(if_let_rescope)] +#![allow(irrefutable_let_patterns)] + +struct Droppy; +impl Drop for Droppy { + fn drop(&mut self) { + println!("dropped"); + } +} +impl Droppy { + fn get(&self) -> Option { + None + } +} + +fn main() { + if let Some(_value) = Droppy.get() { + //[with_feature_gate]~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //[with_feature_gate]~| WARN: this changes meaning in Rust 2024 + }; +} diff --git a/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr b/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr new file mode 100644 index 00000000000..7c295b7f9d8 --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr @@ -0,0 +1,30 @@ +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope-gated.rs:27:8 + | +LL | 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-gated.rs:30:5 + | +LL | }; + | ^ +note: the lint level is defined here + --> $DIR/lint-if-let-rescope-gated.rs:11:9 + | +LL | #![deny(if_let_rescope)] + | ^^^^^^^^^^^^^^ +help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + | +LL ~ match Droppy.get() { Some(_value) => { +LL | +LL | +LL ~ } _ => {} }; + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/drop/lint-if-let-rescope-with-macro.rs b/tests/ui/drop/lint-if-let-rescope-with-macro.rs new file mode 100644 index 00000000000..282b3320d30 --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope-with-macro.rs @@ -0,0 +1,41 @@ +// This test ensures that no suggestion is emitted if the span originates from +// an expansion that is probably not under a user's control. + +//@ edition:2021 +//@ compile-flags: -Z unstable-options + +#![feature(if_let_rescope)] +#![deny(if_let_rescope)] +#![allow(irrefutable_let_patterns)] + +macro_rules! edition_2021_if_let { + ($p:pat, $e:expr, { $($conseq:tt)* } { $($alt:tt)* }) => { + if let $p = $e { $($conseq)* } else { $($alt)* } + //~^ ERROR `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN this changes meaning in Rust 2024 + } +} + +fn droppy() -> Droppy { + Droppy +} +struct Droppy; +impl Drop for Droppy { + fn drop(&mut self) { + println!("dropped"); + } +} +impl Droppy { + fn get(&self) -> Option { + None + } +} + +fn main() { + edition_2021_if_let! { + Some(_value), + droppy().get(), + {} + {} + }; +} diff --git a/tests/ui/drop/lint-if-let-rescope-with-macro.stderr b/tests/ui/drop/lint-if-let-rescope-with-macro.stderr new file mode 100644 index 00000000000..5fd0c61d17a --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope-with-macro.stderr @@ -0,0 +1,39 @@ +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope-with-macro.rs:13:12 + | +LL | if let $p = $e { $($conseq)* } else { $($alt)* } + | ^^^ +... +LL | / edition_2021_if_let! { +LL | | Some(_value), +LL | | droppy().get(), + | | -------- this value has a significant drop implementation which may observe a major change in drop order and requires your discretion +LL | | {} +LL | | {} +LL | | }; + | |_____- in this macro invocation + | + = 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-with-macro.rs:13:38 + | +LL | if let $p = $e { $($conseq)* } else { $($alt)* } + | ^ +... +LL | / edition_2021_if_let! { +LL | | Some(_value), +LL | | droppy().get(), +LL | | {} +LL | | {} +LL | | }; + | |_____- in this macro invocation +note: the lint level is defined here + --> $DIR/lint-if-let-rescope-with-macro.rs:8:9 + | +LL | #![deny(if_let_rescope)] + | ^^^^^^^^^^^^^^ + = note: this error originates in the macro `edition_2021_if_let` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 1 previous error + diff --git a/tests/ui/drop/lint-if-let-rescope.fixed b/tests/ui/drop/lint-if-let-rescope.fixed new file mode 100644 index 00000000000..7ed7e4b279c --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope.fixed @@ -0,0 +1,48 @@ +//@ edition:2024 +//@ compile-flags: -Z validate-mir -Zunstable-options +//@ run-rustfix + +#![feature(if_let_rescope)] +#![deny(if_let_rescope)] +#![allow(irrefutable_let_patterns)] + +fn droppy() -> Droppy { + Droppy +} +struct Droppy; +impl Drop for Droppy { + fn drop(&mut self) { + println!("dropped"); + } +} +impl Droppy { + fn get(&self) -> Option { + None + } +} + +fn main() { + match droppy().get() { Some(_value) => { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into a `match` + // do something + } _ => { + //~^ HELP: the value is now dropped here in Edition 2024 + // do something else + }} + + if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ => { None }} } { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into a `match` + //~| HELP: the value is now dropped here in Edition 2024 + } + + 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: rewrite this `if let` into a `match` + //~| HELP: the value is now dropped here in Edition 2024 + } +} diff --git a/tests/ui/drop/lint-if-let-rescope.rs b/tests/ui/drop/lint-if-let-rescope.rs new file mode 100644 index 00000000000..d282cd2626b --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope.rs @@ -0,0 +1,48 @@ +//@ edition:2024 +//@ compile-flags: -Z validate-mir -Zunstable-options +//@ run-rustfix + +#![feature(if_let_rescope)] +#![deny(if_let_rescope)] +#![allow(irrefutable_let_patterns)] + +fn droppy() -> Droppy { + Droppy +} +struct Droppy; +impl Drop for Droppy { + fn drop(&mut self) { + println!("dropped"); + } +} +impl Droppy { + fn get(&self) -> Option { + None + } +} + +fn main() { + if let Some(_value) = droppy().get() { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into a `match` + // do something + } else { + //~^ HELP: the value is now dropped here in Edition 2024 + // do something else + } + + if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into a `match` + //~| HELP: the value is now dropped here in Edition 2024 + } + + 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: rewrite this `if let` into a `match` + //~| HELP: the value is now dropped here in Edition 2024 + } +} diff --git a/tests/ui/drop/lint-if-let-rescope.stderr b/tests/ui/drop/lint-if-let-rescope.stderr new file mode 100644 index 00000000000..832e73b898c --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope.stderr @@ -0,0 +1,74 @@ +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:25:8 + | +LL | 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:30:5 + | +LL | } else { + | ^ +note: the lint level is defined here + --> $DIR/lint-if-let-rescope.rs:6:9 + | +LL | #![deny(if_let_rescope)] + | ^^^^^^^^^^^^^^ +help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + | +LL ~ match droppy().get() { Some(_value) => { +LL | +... +LL | // do something +LL ~ } _ => { +LL | +LL | // do something else +LL ~ }} + | + +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:35:27 + | +LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { + | ^^^^^^^^^^^^^^^^^^^------^^^^^^ + | | + | 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:35:69 + | +LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { + | ^ +help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + | +LL | if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ => { None }} } { + | ~~~~~ +++++++++++++++++ ~~~~ + + +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:42: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:42:55 + | +LL | if let () = { if let Some(_value) = Droppy.get() {} } { + | ^ +help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + | +LL | if let () = { match Droppy.get() { Some(_value) => {} _ => {} } } { + | ~~~~~ +++++++++++++++++ +++++++++ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/feature-gates/feature-gate-if-let-rescope.rs b/tests/ui/feature-gates/feature-gate-if-let-rescope.rs new file mode 100644 index 00000000000..bd1efd4fb7c --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-if-let-rescope.rs @@ -0,0 +1,27 @@ +// This test shows the code that could have been accepted by enabling #![feature(if_let_rescope)] + +struct A; +struct B<'a, T>(&'a mut T); + +impl A { + fn f(&mut self) -> Option> { + Some(B(self)) + } +} + +impl<'a, T> Drop for B<'a, T> { + fn drop(&mut self) { + // this is needed to keep NLL's hands off and to ensure + // the inner mutable borrow stays alive + } +} + +fn main() { + let mut a = A; + if let None = a.f().as_ref() { + unreachable!() + } else { + a.f().unwrap(); + //~^ ERROR cannot borrow `a` as mutable more than once at a time + }; +} diff --git a/tests/ui/feature-gates/feature-gate-if-let-rescope.stderr b/tests/ui/feature-gates/feature-gate-if-let-rescope.stderr new file mode 100644 index 00000000000..ff1846ae0b1 --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-if-let-rescope.stderr @@ -0,0 +1,18 @@ +error[E0499]: cannot borrow `a` as mutable more than once at a time + --> $DIR/feature-gate-if-let-rescope.rs:24:9 + | +LL | if let None = a.f().as_ref() { + | ----- + | | + | first mutable borrow occurs here + | a temporary with access to the first borrow is created here ... +... +LL | a.f().unwrap(); + | ^ second mutable borrow occurs here +LL | +LL | }; + | - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option>` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0499`. diff --git a/tests/ui/mir/mir_let_chains_drop_order.rs b/tests/ui/mir/mir_let_chains_drop_order.rs index 5cbfdf2e35a..daf0a62fc85 100644 --- a/tests/ui/mir/mir_let_chains_drop_order.rs +++ b/tests/ui/mir/mir_let_chains_drop_order.rs @@ -1,9 +1,14 @@ //@ run-pass //@ needs-unwind +//@ revisions: edition2021 edition2024 +//@ [edition2021] edition: 2021 +//@ [edition2024] compile-flags: -Z unstable-options +//@ [edition2024] edition: 2024 // See `mir_drop_order.rs` for more information #![feature(let_chains)] +#![cfg_attr(edition2024, feature(if_let_rescope))] #![allow(irrefutable_let_patterns)] use std::cell::RefCell; @@ -39,25 +44,32 @@ fn main() { 0, d( 1, - if let Some(_) = d(2, Some(true)).extra && let DropLogger { .. } = d(3, None) { + if let Some(_) = d(2, Some(true)).extra + && let DropLogger { .. } = d(3, None) + { None } else { Some(true) - } - ).extra + }, + ) + .extra, ), d(4, None), &d(5, None), d(6, None), - if let DropLogger { .. } = d(7, None) && let DropLogger { .. } = d(8, None) { + if let DropLogger { .. } = d(7, None) + && let DropLogger { .. } = d(8, None) + { d(9, None) - } - else { + } else { // 10 is not constructed d(10, None) }, ); + #[cfg(edition2021)] assert_eq!(get(), vec![8, 7, 1, 3, 2]); + #[cfg(edition2024)] + assert_eq!(get(), vec![3, 2, 8, 7, 1]); } assert_eq!(get(), vec![0, 4, 6, 9, 5]); @@ -73,21 +85,26 @@ fn main() { None } else { Some(true) - } - ).extra + }, + ) + .extra, ), d(15, None), &d(16, None), d(17, None), - if let DropLogger { .. } = d(18, None) && let DropLogger { .. } = d(19, None) { + if let DropLogger { .. } = d(18, None) + && let DropLogger { .. } = d(19, None) + { d(20, None) - } - else { + } else { // 10 is not constructed d(21, None) }, - panic::panic_any(InjectedFailure) + panic::panic_any(InjectedFailure), ); }); + #[cfg(edition2021)] assert_eq!(get(), vec![20, 17, 15, 11, 19, 18, 16, 12, 14, 13]); + #[cfg(edition2024)] + assert_eq!(get(), vec![14, 13, 19, 18, 20, 17, 15, 11, 16, 12]); } diff --git a/tests/ui/nll/issue-54556-niconii.stderr b/tests/ui/nll/issue-54556-niconii.edition2021.stderr similarity index 96% rename from tests/ui/nll/issue-54556-niconii.stderr rename to tests/ui/nll/issue-54556-niconii.edition2021.stderr index 015d9e18212..31a03abbc98 100644 --- a/tests/ui/nll/issue-54556-niconii.stderr +++ b/tests/ui/nll/issue-54556-niconii.edition2021.stderr @@ -1,5 +1,5 @@ error[E0597]: `counter` does not live long enough - --> $DIR/issue-54556-niconii.rs:22:20 + --> $DIR/issue-54556-niconii.rs:30:20 | LL | let counter = Mutex; | ------- binding `counter` declared here diff --git a/tests/ui/nll/issue-54556-niconii.rs b/tests/ui/nll/issue-54556-niconii.rs index cae389e8ccb..1a7ad17cc84 100644 --- a/tests/ui/nll/issue-54556-niconii.rs +++ b/tests/ui/nll/issue-54556-niconii.rs @@ -6,6 +6,14 @@ // of temp drop order, and thus why inserting a semi-colon after the // `if let` expression in `main` works. +//@ revisions: edition2021 edition2024 +//@ [edition2021] edition: 2021 +//@ [edition2024] edition: 2024 +//@ [edition2024] compile-flags: -Z unstable-options +//@ [edition2024] check-pass + +#![cfg_attr(edition2024, feature(if_let_rescope))] + struct Mutex; struct MutexGuard<'a>(&'a Mutex); @@ -19,8 +27,10 @@ impl Mutex { fn main() { let counter = Mutex; - if let Ok(_) = counter.lock() { } //~ ERROR does not live long enough + if let Ok(_) = counter.lock() { } + //[edition2021]~^ ERROR: does not live long enough + // Up until Edition 2021: // With this code as written, the dynamic semantics here implies // that `Mutex::drop` for `counter` runs *before* // `MutexGuard::drop`, which would be unsound since `MutexGuard` @@ -28,4 +38,11 @@ fn main() { // // The goal of #54556 is to explain that within a compiler // diagnostic. + + // From Edition 2024: + // Now `MutexGuard::drop` runs *before* `Mutex::drop` because + // the lifetime of the `MutexGuard` is shortened to cover only + // from `if let` until the end of the consequent block. + // Therefore, Niconii's issue is properly solved thanks to the new + // temporary lifetime rule for `if let`s. } From e2120a7c3853226e486988b40196ff87b5eacb42 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 6 Sep 2024 03:03:10 +0800 Subject: [PATCH 2/4] coalesce lint suggestions that can intersect --- compiler/rustc_lint/messages.ftl | 4 +- compiler/rustc_lint/src/if_let_rescope.rs | 323 ++++++++++++------ compiler/rustc_lint/src/lib.rs | 2 +- tests/ui/drop/lint-if-let-rescope-gated.rs | 5 +- ...let-rescope-gated.with_feature_gate.stderr | 44 ++- tests/ui/drop/lint-if-let-rescope.fixed | 69 +++- tests/ui/drop/lint-if-let-rescope.rs | 63 +++- tests/ui/drop/lint-if-let-rescope.stderr | 223 ++++++++++-- 8 files changed, 572 insertions(+), 161 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 1f8dca337fc..2d062465bcf 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -337,7 +337,9 @@ lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> lint_if_let_rescope = `if let` assigns a shorter lifetime since Edition 2024 .label = this value has a significant drop implementation which may observe a major change in drop order and requires your discretion .help = the value is now dropped here in Edition 2024 - .suggestion = rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + +lint_if_let_rescope_suggestion = a `match` with a single arm can preserve the drop order up to Edition 2021 + .suggestion = rewrite this `if let` into `match` lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs index 143965ae58f..e5bf192d23e 100644 --- a/compiler/rustc_lint/src/if_let_rescope.rs +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -1,11 +1,16 @@ +use std::iter::repeat; use std::ops::ControlFlow; use hir::intravisit::Visitor; use rustc_ast::Recovered; -use rustc_hir as hir; +use rustc_errors::{ + Applicability, Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic, SuggestionStyle, +}; +use rustc_hir::{self as hir, HirIdSet}; use rustc_macros::{LintDiagnostic, Subdiagnostic}; -use rustc_session::lint::FutureIncompatibilityReason; -use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_middle::ty::TyCtxt; +use rustc_session::lint::{FutureIncompatibilityReason, Level}; +use rustc_session::{declare_lint, impl_lint_pass}; use rustc_span::edition::Edition; use rustc_span::Span; @@ -84,138 +89,236 @@ declare_lint! { }; } -declare_lint_pass!( - /// Lint for potential change in program semantics of `if let`s +/// Lint for potential change in program semantics of `if let`s +#[derive(Default)] +pub(crate) struct IfLetRescope { + skip: HirIdSet, +} + +fn expr_parent_is_else(tcx: TyCtxt<'_>, hir_id: hir::HirId) -> bool { + let Some((_, hir::Node::Expr(expr))) = tcx.hir().parent_iter(hir_id).next() else { + return false; + }; + let hir::ExprKind::If(_cond, _conseq, Some(alt)) = expr.kind else { return false }; + alt.hir_id == hir_id +} + +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 (hir::StmtKind::Semi(expr) | hir::StmtKind::Expr(expr)) = stmt.kind else { return false }; + expr.hir_id == hir_id +} + +fn match_head_needs_bracket(tcx: TyCtxt<'_>, expr: &hir::Expr<'_>) -> bool { + expr_parent_is_else(tcx, expr.hir_id) && matches!(expr.kind, hir::ExprKind::If(..)) +} + +impl IfLetRescope { + fn probe_if_cascade<'tcx>(&mut self, cx: &LateContext<'tcx>, mut expr: &'tcx hir::Expr<'tcx>) { + if self.skip.contains(&expr.hir_id) { + return; + } + let tcx = cx.tcx; + let source_map = tcx.sess.source_map(); + let expr_end = expr.span.shrink_to_hi(); + let mut add_bracket_to_match_head = match_head_needs_bracket(tcx, expr); + let mut closing_brackets = 0; + let mut alt_heads = vec![]; + let mut match_heads = vec![]; + let mut consequent_heads = vec![]; + let mut first_if_to_rewrite = None; + let mut empty_alt = false; + while let hir::ExprKind::If(cond, conseq, alt) = expr.kind { + self.skip.insert(expr.hir_id); + let hir::ExprKind::Let(&hir::LetExpr { + span, + pat, + init, + ty: ty_ascription, + recovered: Recovered::No, + }) = cond.kind + else { + if let Some(alt) = alt { + add_bracket_to_match_head = matches!(alt.kind, hir::ExprKind::If(..)); + expr = alt; + continue; + } else { + // finalize and emit span + break; + } + }; + let if_let_pat = expr.span.shrink_to_lo().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); + + if let ControlFlow::Break(significant_dropper) = + (FindSignificantDropper { cx }).visit_expr(init) + { + tcx.emit_node_span_lint( + IF_LET_RESCOPE, + expr.hir_id, + span, + IfLetRescopeLint { significant_dropper, lifetime_end }, + ); + if ty_ascription.is_some() + || !expr.span.can_be_used_for_suggestions() + || !pat.span.can_be_used_for_suggestions() + { + // Our `match` rewrites does not support type ascription, + // so we just bail. + // Alternatively when the span comes from proc macro expansion, + // we will also bail. + // FIXME(#101728): change this when type ascription syntax is stabilized again + } else if let Ok(pat) = source_map.span_to_snippet(pat.span) { + let emit_suggestion = || { + first_if_to_rewrite = + first_if_to_rewrite.or_else(|| Some((expr.span, expr.hir_id))); + if add_bracket_to_match_head { + closing_brackets += 2; + match_heads.push(SingleArmMatchBegin::WithOpenBracket(if_let_pat)); + } else { + // It has to be a block + closing_brackets += 1; + match_heads.push(SingleArmMatchBegin::WithoutOpenBracket(if_let_pat)); + } + consequent_heads.push(ConsequentRewrite { span: before_conseq, pat }); + }; + if let Some(alt) = alt { + let alt_head = conseq.span.between(alt.span); + if alt_head.can_be_used_for_suggestions() { + // lint + emit_suggestion(); + alt_heads.push(AltHead(alt_head)); + } + } else { + emit_suggestion(); + empty_alt = true; + break; + } + } + } + if let Some(alt) = alt { + add_bracket_to_match_head = matches!(alt.kind, hir::ExprKind::If(..)); + expr = alt; + } else { + break; + } + } + if let Some((span, hir_id)) = first_if_to_rewrite { + tcx.emit_node_span_lint( + IF_LET_RESCOPE, + hir_id, + span, + IfLetRescopeRewrite { + match_heads, + consequent_heads, + closing_brackets: ClosingBrackets { + span: expr_end, + count: closing_brackets, + empty_alt, + }, + alt_heads, + }, + ); + } + } +} + +impl_lint_pass!( IfLetRescope => [IF_LET_RESCOPE] ); impl<'tcx> LateLintPass<'tcx> for IfLetRescope { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { - if !expr.span.edition().at_least_rust_2021() || !cx.tcx.features().if_let_rescope { + if expr.span.edition().at_least_rust_2024() || !cx.tcx.features().if_let_rescope { return; } - let hir::ExprKind::If(cond, conseq, alt) = expr.kind else { return }; - let hir::ExprKind::Let(&hir::LetExpr { - span, - pat, - init, - ty: ty_ascription, - recovered: Recovered::No, - }) = cond.kind - else { + if let (Level::Allow, _) = cx.tcx.lint_level_at_node(IF_LET_RESCOPE, expr.hir_id) { return; - }; - let source_map = cx.tcx.sess.source_map(); - let expr_end = expr.span.shrink_to_hi(); - let if_let_pat = expr.span.shrink_to_lo().between(init.span); - let before_conseq = conseq.span.shrink_to_lo(); - let lifetime_end = source_map.end_point(conseq.span); - - if let ControlFlow::Break(significant_dropper) = - (FindSignificantDropper { cx }).visit_expr(init) + } + if expr_parent_is_stmt(cx.tcx, expr.hir_id) + && matches!(expr.kind, hir::ExprKind::If(_cond, _conseq, None)) { - let lint_without_suggestion = || { - cx.tcx.emit_node_span_lint( - IF_LET_RESCOPE, - expr.hir_id, - span, - IfLetRescopeRewrite { significant_dropper, lifetime_end, sugg: None }, - ) - }; - if ty_ascription.is_some() - || !expr.span.can_be_used_for_suggestions() - || !pat.span.can_be_used_for_suggestions() - { - // Our `match` rewrites does not support type ascription, - // so we just bail. - // Alternatively when the span comes from proc macro expansion, - // we will also bail. - // FIXME(#101728): change this when type ascription syntax is stabilized again - lint_without_suggestion(); - } else { - let Ok(pat) = source_map.span_to_snippet(pat.span) else { - lint_without_suggestion(); - return; - }; - if let Some(alt) = alt { - let alt_start = conseq.span.between(alt.span); - if !alt_start.can_be_used_for_suggestions() { - lint_without_suggestion(); - return; - } - cx.tcx.emit_node_span_lint( - IF_LET_RESCOPE, - expr.hir_id, - span, - IfLetRescopeRewrite { - significant_dropper, - lifetime_end, - sugg: Some(IfLetRescopeRewriteSuggestion::WithElse { - if_let_pat, - before_conseq, - pat, - expr_end, - alt_start, - }), - }, - ); - } else { - cx.tcx.emit_node_span_lint( - IF_LET_RESCOPE, - expr.hir_id, - span, - IfLetRescopeRewrite { - significant_dropper, - lifetime_end, - sugg: Some(IfLetRescopeRewriteSuggestion::WithoutElse { - if_let_pat, - before_conseq, - pat, - expr_end, - }), - }, - ); - } - } + // `if let` statement without an `else` branch has no observable change + // so we can skip linting it + return; } + self.probe_if_cascade(cx, expr); } } #[derive(LintDiagnostic)] #[diag(lint_if_let_rescope)] -struct IfLetRescopeRewrite { +struct IfLetRescopeLint { #[label] significant_dropper: Span, #[help] lifetime_end: Span, +} + +#[derive(LintDiagnostic)] +#[diag(lint_if_let_rescope_suggestion)] +struct IfLetRescopeRewrite { #[subdiagnostic] - sugg: Option, + match_heads: Vec, + #[subdiagnostic] + consequent_heads: Vec, + #[subdiagnostic] + closing_brackets: ClosingBrackets, + #[subdiagnostic] + alt_heads: Vec, } #[derive(Subdiagnostic)] -enum IfLetRescopeRewriteSuggestion { +#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] +struct AltHead(#[suggestion_part(code = " _ => ")] Span); + +#[derive(Subdiagnostic)] +#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] +struct ConsequentRewrite { + #[suggestion_part(code = "{{ {pat} => ")] + span: Span, + pat: String, +} + +struct ClosingBrackets { + span: Span, + count: usize, + empty_alt: bool, +} + +impl Subdiagnostic for ClosingBrackets { + fn add_to_diag_with>( + self, + diag: &mut Diag<'_, G>, + f: &F, + ) { + let code: String = self + .empty_alt + .then_some(" _ => {}".chars()) + .into_iter() + .flatten() + .chain(repeat('}').take(self.count)) + .collect(); + let msg = f(diag, crate::fluent_generated::lint_suggestion.into()); + diag.multipart_suggestion_with_style( + msg, + vec![(self.span, code)], + Applicability::MachineApplicable, + SuggestionStyle::ShowCode, + ); + } +} + +#[derive(Subdiagnostic)] +enum SingleArmMatchBegin { #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] - WithElse { - #[suggestion_part(code = "match ")] - if_let_pat: Span, - #[suggestion_part(code = " {{ {pat} => ")] - before_conseq: Span, - pat: String, - #[suggestion_part(code = "}}")] - expr_end: Span, - #[suggestion_part(code = " _ => ")] - alt_start: Span, - }, + WithOpenBracket(#[suggestion_part(code = "{{ match ")] Span), #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] - WithoutElse { - #[suggestion_part(code = "match ")] - if_let_pat: Span, - #[suggestion_part(code = " {{ {pat} => ")] - before_conseq: Span, - pat: String, - #[suggestion_part(code = " _ => {{}} }}")] - expr_end: Span, - }, + WithoutOpenBracket(#[suggestion_part(code = "match ")] Span), } struct FindSignificantDropper<'tcx, 'a> { diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 58b51240a90..0c32157758c 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -245,7 +245,7 @@ late_lint_methods!( NonLocalDefinitions: NonLocalDefinitions::default(), ImplTraitOvercaptures: ImplTraitOvercaptures, TailExprDropOrder: TailExprDropOrder, - IfLetRescope: IfLetRescope, + IfLetRescope: IfLetRescope::default(), ] ] ); diff --git a/tests/ui/drop/lint-if-let-rescope-gated.rs b/tests/ui/drop/lint-if-let-rescope-gated.rs index f8844a09f7c..08ded67a512 100644 --- a/tests/ui/drop/lint-if-let-rescope-gated.rs +++ b/tests/ui/drop/lint-if-let-rescope-gated.rs @@ -26,6 +26,9 @@ impl Droppy { fn main() { if let Some(_value) = Droppy.get() { //[with_feature_gate]~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //[with_feature_gate]~| ERROR: a `match` with a single arm can preserve the drop order up to Edition 2021 //[with_feature_gate]~| WARN: this changes meaning in Rust 2024 - }; + //[with_feature_gate]~| WARN: this changes meaning in Rust 2024 + } else { + } } diff --git a/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr b/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr index 7c295b7f9d8..e0f2bebdbf1 100644 --- a/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr +++ b/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr @@ -9,22 +9,46 @@ LL | if let Some(_value) = Droppy.get() { = 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-gated.rs:30:5 + --> $DIR/lint-if-let-rescope-gated.rs:32:5 | -LL | }; +LL | } else { | ^ note: the lint level is defined here --> $DIR/lint-if-let-rescope-gated.rs:11:9 | LL | #![deny(if_let_rescope)] | ^^^^^^^^^^^^^^ -help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 - | -LL ~ match Droppy.get() { Some(_value) => { -LL | -LL | -LL ~ } _ => {} }; - | -error: aborting due to 1 previous error +error: a `match` with a single arm can preserve the drop order up to Edition 2021 + --> $DIR/lint-if-let-rescope-gated.rs:27:5 + | +LL | / if let Some(_value) = Droppy.get() { +LL | | +LL | | +LL | | +LL | | +LL | | } else { +LL | | } + | |_____^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #124085 +help: rewrite this `if let` into `match` + | +LL | match Droppy.get() { + | ~~~~~ +help: rewrite this `if let` into `match` + | +LL | if let Some(_value) = Droppy.get() { Some(_value) => { + | +++++++++++++++++ +help: rewrite this `if let` into `match` + | +LL | }} + | + +help: rewrite this `if let` into `match` + | +LL | } _ => { + | ~~~~ + +error: aborting due to 2 previous errors diff --git a/tests/ui/drop/lint-if-let-rescope.fixed b/tests/ui/drop/lint-if-let-rescope.fixed index 7ed7e4b279c..85ffa320796 100644 --- a/tests/ui/drop/lint-if-let-rescope.fixed +++ b/tests/ui/drop/lint-if-let-rescope.fixed @@ -1,9 +1,7 @@ -//@ edition:2024 -//@ compile-flags: -Z validate-mir -Zunstable-options //@ run-rustfix -#![feature(if_let_rescope)] #![deny(if_let_rescope)] +#![feature(if_let_rescope)] #![allow(irrefutable_let_patterns)] fn droppy() -> Droppy { @@ -22,27 +20,80 @@ impl Droppy { } fn main() { - match droppy().get() { Some(_value) => { + if let Some(_value) = droppy().get() { + // Should not lint + } + + match droppy().get() { Some(_value) => { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into a `match` + //~| WARN: this changes meaning in Rust 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` // do something } _ => { //~^ HELP: the value is now dropped here in Edition 2024 + //~| HELP: rewrite this `if let` into `match` // do something else }} + //~^ HELP: rewrite this `if let` into `match` - if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ => { None }} } { + match droppy().get() { Some(_value) => { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into a `match` + //~| WARN: this changes meaning in Rust 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + // do something + } _ => { match droppy().get() { Some(_value) => { + //~^ HELP: the value is now dropped here in Edition 2024 + //~| ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + // do something else + } _ => {}}}} + //~^ HELP: rewrite this `if let` into `match` + //~| HELP: the value is now dropped here in Edition 2024 + + if droppy().get().is_some() { + // Should not lint + } else { match droppy().get() { Some(_value) => { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| WARN: this changes meaning in Rust 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + } _ => if droppy().get().is_none() { + //~^ HELP: the value is now dropped here in Edition 2024 + //~| HELP: rewrite this `if let` into `match` + }}} + //~^ HELP: rewrite this `if let` into `match` + + if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ => { None }} } { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| WARN: this changes meaning in Rust 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` //~| HELP: the value is now dropped here in Edition 2024 } - if let () = { match Droppy.get() { Some(_value) => {} _ => {} } } { + if let () = { match Droppy.get() { Some(_value) => {} _ => {}} } { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into a `match` + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` //~| HELP: the value is now dropped here in Edition 2024 } } diff --git a/tests/ui/drop/lint-if-let-rescope.rs b/tests/ui/drop/lint-if-let-rescope.rs index d282cd2626b..776b34fdbea 100644 --- a/tests/ui/drop/lint-if-let-rescope.rs +++ b/tests/ui/drop/lint-if-let-rescope.rs @@ -1,9 +1,7 @@ -//@ edition:2024 -//@ compile-flags: -Z validate-mir -Zunstable-options //@ run-rustfix -#![feature(if_let_rescope)] #![deny(if_let_rescope)] +#![feature(if_let_rescope)] #![allow(irrefutable_let_patterns)] fn droppy() -> Droppy { @@ -22,27 +20,80 @@ impl Droppy { } fn main() { + if let Some(_value) = droppy().get() { + // Should not lint + } + if let Some(_value) = droppy().get() { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into a `match` + //~| WARN: this changes meaning in Rust 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` // do something } else { //~^ HELP: the value is now dropped here in Edition 2024 + //~| HELP: rewrite this `if let` into `match` // do something else } + //~^ HELP: rewrite this `if let` into `match` + + if let Some(_value) = droppy().get() { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + // do something + } else if let Some(_value) = droppy().get() { + //~^ HELP: the value is now dropped here in Edition 2024 + //~| ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + // do something else + } + //~^ HELP: rewrite this `if let` into `match` + //~| HELP: the value is now dropped here in Edition 2024 + + if droppy().get().is_some() { + // Should not lint + } else if let Some(_value) = droppy().get() { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 + //~| WARN: this changes meaning in Rust 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + } else if droppy().get().is_none() { + //~^ HELP: the value is now dropped here in Edition 2024 + //~| HELP: rewrite this `if let` into `match` + } + //~^ HELP: rewrite this `if let` into `match` if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into a `match` + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` //~| HELP: the value is now dropped here in Edition 2024 } if let () = { if let Some(_value) = Droppy.get() {} } { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into a `match` + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` + //~| HELP: rewrite this `if let` into `match` //~| HELP: the value is now dropped here in Edition 2024 } } diff --git a/tests/ui/drop/lint-if-let-rescope.stderr b/tests/ui/drop/lint-if-let-rescope.stderr index 832e73b898c..a08d95e1138 100644 --- a/tests/ui/drop/lint-if-let-rescope.stderr +++ b/tests/ui/drop/lint-if-let-rescope.stderr @@ -1,5 +1,5 @@ error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:25:8 + --> $DIR/lint-if-let-rescope.rs:27:8 | LL | if let Some(_value) = droppy().get() { | ^^^^^^^^^^^^^^^^^^^--------^^^^^^ @@ -9,29 +9,168 @@ LL | if let Some(_value) = droppy().get() { = 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:30:5 + --> $DIR/lint-if-let-rescope.rs:35:5 | LL | } else { | ^ note: the lint level is defined here - --> $DIR/lint-if-let-rescope.rs:6:9 + --> $DIR/lint-if-let-rescope.rs:3:9 | LL | #![deny(if_let_rescope)] | ^^^^^^^^^^^^^^ -help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + +error: a `match` with a single arm can preserve the drop order up to Edition 2021 + --> $DIR/lint-if-let-rescope.rs:27:5 | -LL ~ match droppy().get() { Some(_value) => { -LL | -... -LL | // do something -LL ~ } _ => { -LL | -LL | // do something else -LL ~ }} +LL | / if let Some(_value) = droppy().get() { +LL | | +LL | | +LL | | +... | +LL | | // do something else +LL | | } + | |_____^ | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #124085 +help: rewrite this `if let` into `match` + | +LL | match droppy().get() { + | ~~~~~ +help: rewrite this `if let` into `match` + | +LL | if let Some(_value) = droppy().get() { Some(_value) => { + | +++++++++++++++++ +help: rewrite this `if let` into `match` + | +LL | }} + | + +help: rewrite this `if let` into `match` + | +LL | } _ => { + | ~~~~ error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:35:27 + --> $DIR/lint-if-let-rescope.rs:42:8 + | +LL | 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:50:5 + | +LL | } else if let Some(_value) = droppy().get() { + | ^ + +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:50:15 + | +LL | } else 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:58:5 + | +LL | } + | ^ + +error: a `match` with a single arm can preserve the drop order up to Edition 2021 + --> $DIR/lint-if-let-rescope.rs:42:5 + | +LL | / if let Some(_value) = droppy().get() { +LL | | +LL | | +LL | | +... | +LL | | // do something else +LL | | } + | |_____^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #124085 +help: rewrite this `if let` into `match` + | +LL | match droppy().get() { + | ~~~~~ +help: rewrite this `if let` into `match` + | +LL | } else { match droppy().get() { + | ~~~~~~~ +help: rewrite this `if let` into `match` + | +LL | if let Some(_value) = droppy().get() { Some(_value) => { + | +++++++++++++++++ +help: rewrite this `if let` into `match` + | +LL | } else if let Some(_value) = droppy().get() { Some(_value) => { + | +++++++++++++++++ +help: rewrite this `if let` into `match` + | +LL | } _ => {}}}} + | ++++++++++ +help: rewrite this `if let` into `match` + | +LL | } _ => if let Some(_value) = droppy().get() { + | ~~~~ + +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:64:15 + | +LL | } else 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:71:5 + | +LL | } else if droppy().get().is_none() { + | ^ + +error: a `match` with a single arm can preserve the drop order up to Edition 2021 + --> $DIR/lint-if-let-rescope.rs:64:12 + | +LL | } else if let Some(_value) = droppy().get() { + | ____________^ +LL | | +LL | | +LL | | +... | +LL | | +LL | | } + | |_____^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #124085 +help: rewrite this `if let` into `match` + | +LL | } else { match droppy().get() { + | ~~~~~~~ +help: rewrite this `if let` into `match` + | +LL | } else if let Some(_value) = droppy().get() { Some(_value) => { + | +++++++++++++++++ +help: rewrite this `if let` into `match` + | +LL | }}} + | ++ +help: rewrite this `if let` into `match` + | +LL | } _ => if droppy().get().is_none() { + | ~~~~ + +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:77:27 | LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { | ^^^^^^^^^^^^^^^^^^^------^^^^^^ @@ -41,17 +180,38 @@ LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else = 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:35:69 + --> $DIR/lint-if-let-rescope.rs:77:69 | LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { | ^ -help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + +error: a `match` with a single arm can preserve the drop order up to Edition 2021 + --> $DIR/lint-if-let-rescope.rs:77:24 | -LL | if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ => { None }} } { - | ~~~~~ +++++++++++++++++ ~~~~ + +LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #124085 +help: rewrite this `if let` into `match` + | +LL | if let Some(1) = { match Droppy.get() { Some(1) } else { None } } { + | ~~~~~ +help: rewrite this `if let` into `match` + | +LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(_value) => { Some(1) } else { None } } { + | +++++++++++++++++ +help: rewrite this `if let` into `match` + | +LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None }} } { + | + +help: rewrite this `if let` into `match` + | +LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } _ => { None } } { + | ~~~~ error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:42:22 + --> $DIR/lint-if-let-rescope.rs:89:22 | LL | if let () = { if let Some(_value) = Droppy.get() {} } { | ^^^^^^^^^^^^^^^^^^^------^^^^^^ @@ -61,14 +221,31 @@ LL | if let () = { if let Some(_value) = Droppy.get() {} } { = 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:42:55 + --> $DIR/lint-if-let-rescope.rs:89:55 | LL | if let () = { if let Some(_value) = Droppy.get() {} } { | ^ -help: rewrite this `if let` into a `match` with a single arm to preserve the drop order up to Edition 2021 + +error: a `match` with a single arm can preserve the drop order up to Edition 2021 + --> $DIR/lint-if-let-rescope.rs:89:19 | -LL | if let () = { match Droppy.get() { Some(_value) => {} _ => {} } } { - | ~~~~~ +++++++++++++++++ +++++++++ +LL | if let () = { if let Some(_value) = Droppy.get() {} } { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #124085 +help: rewrite this `if let` into `match` + | +LL | if let () = { match Droppy.get() {} } { + | ~~~~~ +help: rewrite this `if let` into `match` + | +LL | if let () = { if let Some(_value) = Droppy.get() { Some(_value) => {} } { + | +++++++++++++++++ +help: rewrite this `if let` into `match` + | +LL | if let () = { if let Some(_value) = Droppy.get() {} _ => {}} } { + | ++++++++ -error: aborting due to 3 previous errors +error: aborting due to 11 previous errors From 89682a53131803c81c8d6c5f013d1bbe410c8ae1 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Wed, 11 Sep 2024 04:07:13 +0800 Subject: [PATCH 3/4] downgrade borrowck suggestion level due to possible span conflict --- .../src/diagnostics/explain_borrow.rs | 43 +++++++---- .../if-let-rescope-borrowck-suggestions.fixed | 26 ------- .../if-let-rescope-borrowck-suggestions.rs | 10 ++- ...if-let-rescope-borrowck-suggestions.stderr | 77 +++++++++++++++++-- 4 files changed, 108 insertions(+), 48 deletions(-) delete mode 100644 tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index 22327d2ba0d..d9b5fd8b5c1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -128,7 +128,7 @@ impl<'tcx> BorrowExplanation<'tcx> { && pat.span.can_be_used_for_suggestions() && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span) { - suggest_rewrite_if_let(expr, &pat, init, conseq, alt, err); + suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err); } else if path_span.map_or(true, |path_span| path_span == var_or_use_span) { // We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same if borrow_span.map_or(true, |sp| !sp.overlaps(var_or_use_span)) { @@ -292,7 +292,7 @@ impl<'tcx> BorrowExplanation<'tcx> { && pat.span.can_be_used_for_suggestions() && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span) { - suggest_rewrite_if_let(expr, &pat, init, conseq, alt, err); + suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err); } } } @@ -428,34 +428,49 @@ impl<'tcx> BorrowExplanation<'tcx> { } } -fn suggest_rewrite_if_let<'tcx>( - expr: &hir::Expr<'tcx>, +fn suggest_rewrite_if_let( + tcx: TyCtxt<'_>, + expr: &hir::Expr<'_>, pat: &str, - init: &hir::Expr<'tcx>, - conseq: &hir::Expr<'tcx>, - alt: Option<&hir::Expr<'tcx>>, + init: &hir::Expr<'_>, + conseq: &hir::Expr<'_>, + alt: Option<&hir::Expr<'_>>, err: &mut Diag<'_>, ) { + let source_map = tcx.sess.source_map(); err.span_note( - conseq.span.shrink_to_hi(), - "lifetime for temporaries generated in `if let`s have been shorted in Edition 2024", + source_map.end_point(conseq.span), + "lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead", ); if expr.span.can_be_used_for_suggestions() && conseq.span.can_be_used_for_suggestions() { + let needs_block = if let Some(hir::Node::Expr(expr)) = + alt.and_then(|alt| tcx.hir().parent_iter(alt.hir_id).next()).map(|(_, node)| node) + { + matches!(expr.kind, hir::ExprKind::If(..)) + } else { + false + }; let mut sugg = vec![ - (expr.span.shrink_to_lo().between(init.span), "match ".into()), + ( + expr.span.shrink_to_lo().between(init.span), + if needs_block { "{ match ".into() } else { "match ".into() }, + ), (conseq.span.shrink_to_lo(), format!(" {{ {pat} => ")), ]; let expr_end = expr.span.shrink_to_hi(); + let mut expr_end_code; if let Some(alt) = alt { - sugg.push((conseq.span.between(alt.span), format!(" _ => "))); - sugg.push((expr_end, "}".into())); + sugg.push((conseq.span.between(alt.span), " _ => ".into())); + expr_end_code = "}".to_string(); } else { - sugg.push((expr_end, " _ => {} }".into())); + expr_end_code = " _ => {} }".into(); } + expr_end_code.push('}'); + sugg.push((expr_end, expr_end_code)); err.multipart_suggestion( "consider rewriting the `if` into `match` which preserves the extended lifetime", sugg, - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); } } diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed b/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed deleted file mode 100644 index 129e78ea9fe..00000000000 --- a/tests/ui/drop/if-let-rescope-borrowck-suggestions.fixed +++ /dev/null @@ -1,26 +0,0 @@ -//@ edition: 2024 -//@ compile-flags: -Z validate-mir -Zunstable-options -//@ run-rustfix - -#![feature(if_let_rescope)] -#![deny(if_let_rescope)] - -struct Droppy; -impl Drop for Droppy { - fn drop(&mut self) { - println!("dropped"); - } -} -impl Droppy { - fn get_ref(&self) -> Option<&u8> { - None - } -} - -fn do_something(_: &T) {} - -fn main() { - let binding = Droppy; - do_something(match binding.get_ref() { Some(value) => { value } _ => { &0 }}); - //~^ ERROR: temporary value dropped while borrowed -} diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs index 1970d5c0f7e..2476f7cf258 100644 --- a/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs @@ -1,6 +1,5 @@ //@ edition: 2024 //@ compile-flags: -Z validate-mir -Zunstable-options -//@ run-rustfix #![feature(if_let_rescope)] #![deny(if_let_rescope)] @@ -22,4 +21,13 @@ fn do_something(_: &T) {} fn main() { do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); //~^ ERROR: temporary value dropped while borrowed + do_something(if let Some(value) = Droppy.get_ref() { + //~^ ERROR: temporary value dropped while borrowed + value + } else if let Some(value) = Droppy.get_ref() { + //~^ ERROR: temporary value dropped while borrowed + value + } else { + &0 + }); } diff --git a/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr b/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr index fa154f01a12..0c6f1ea28d2 100644 --- a/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr @@ -1,16 +1,16 @@ error[E0716]: temporary value dropped while borrowed - --> $DIR/if-let-rescope-borrowck-suggestions.rs:23:39 + --> $DIR/if-let-rescope-borrowck-suggestions.rs:22:39 | LL | do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); | ^^^^^^ - temporary value is freed at the end of this statement | | | creates a temporary value which is freed while still in use | -note: lifetime for temporaries generated in `if let`s have been shorted in Edition 2024 - --> $DIR/if-let-rescope-borrowck-suggestions.rs:23:65 +note: lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead + --> $DIR/if-let-rescope-borrowck-suggestions.rs:22:64 | LL | do_something(if let Some(value) = Droppy.get_ref() { value } else { &0 }); - | ^ + | ^ help: consider using a `let` binding to create a longer lived value | LL ~ let binding = Droppy; @@ -18,9 +18,72 @@ LL ~ do_something(if let Some(value) = binding.get_ref() { value } else { &0 | help: consider rewriting the `if` into `match` which preserves the extended lifetime | -LL | do_something(match Droppy.get_ref() { Some(value) => { value } _ => { &0 }}); - | ~~~~~ ++++++++++++++++ ~~~~ + +LL | do_something({ match Droppy.get_ref() { Some(value) => { value } _ => { &0 }}}); + | ~~~~~~~ ++++++++++++++++ ~~~~ ++ -error: aborting due to 1 previous error +error[E0716]: temporary value dropped while borrowed + --> $DIR/if-let-rescope-borrowck-suggestions.rs:24:39 + | +LL | do_something(if let Some(value) = Droppy.get_ref() { + | ^^^^^^ creates a temporary value which is freed while still in use +... +LL | } else if let Some(value) = Droppy.get_ref() { + | - temporary value is freed at the end of this statement + | +note: lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead + --> $DIR/if-let-rescope-borrowck-suggestions.rs:27:5 + | +LL | } else if let Some(value) = Droppy.get_ref() { + | ^ +help: consider using a `let` binding to create a longer lived value + | +LL ~ let binding = Droppy; +LL ~ do_something(if let Some(value) = binding.get_ref() { + | +help: consider rewriting the `if` into `match` which preserves the extended lifetime + | +LL ~ do_something({ match Droppy.get_ref() { Some(value) => { +LL | +LL | value +LL ~ } _ => if let Some(value) = Droppy.get_ref() { +LL | +... +LL | &0 +LL ~ }}}); + | + +error[E0716]: temporary value dropped while borrowed + --> $DIR/if-let-rescope-borrowck-suggestions.rs:27:33 + | +LL | } else if let Some(value) = Droppy.get_ref() { + | ^^^^^^ creates a temporary value which is freed while still in use +... +LL | } else { + | - temporary value is freed at the end of this statement + | +note: lifetimes for temporaries generated in `if let`s have been shortened in Edition 2024 so that they are dropped here instead + --> $DIR/if-let-rescope-borrowck-suggestions.rs:30:5 + | +LL | } else { + | ^ +help: consider using a `let` binding to create a longer lived value + | +LL ~ let binding = Droppy; +LL ~ do_something(if let Some(value) = Droppy.get_ref() { +LL | +LL | value +LL ~ } else if let Some(value) = binding.get_ref() { + | +help: consider rewriting the `if` into `match` which preserves the extended lifetime + | +LL ~ } else { match Droppy.get_ref() { Some(value) => { +LL | +LL | value +LL ~ } _ => { +LL | &0 +LL ~ }}}); + | + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0716`. From b4b2b356d958664f87c5b12344256c536e9e1b14 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 13 Sep 2024 02:43:49 +0800 Subject: [PATCH 4/4] simplify the suggestion notes --- compiler/rustc_lint/messages.ftl | 4 +- compiler/rustc_lint/src/if_let_rescope.rs | 237 ++++++++++-------- tests/ui/drop/lint-if-let-rescope-gated.rs | 4 +- ...let-rescope-gated.with_feature_gate.stderr | 45 +--- tests/ui/drop/lint-if-let-rescope.fixed | 40 +-- tests/ui/drop/lint-if-let-rescope.rs | 40 +-- tests/ui/drop/lint-if-let-rescope.stderr | 220 ++++------------ 7 files changed, 205 insertions(+), 385 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 2d062465bcf..4aa86944a0b 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -337,9 +337,7 @@ lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> lint_if_let_rescope = `if let` assigns a shorter lifetime since Edition 2024 .label = this value has a significant drop implementation which may observe a major change in drop order and requires your discretion .help = the value is now dropped here in Edition 2024 - -lint_if_let_rescope_suggestion = a `match` with a single arm can preserve the drop order up to Edition 2021 - .suggestion = rewrite this `if let` into `match` + .suggestion = a `match` with a single arm can preserve the drop order up to Edition 2021 lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs index e5bf192d23e..5b65541bea6 100644 --- a/compiler/rustc_lint/src/if_let_rescope.rs +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -7,7 +7,7 @@ use rustc_errors::{ Applicability, Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic, SuggestionStyle, }; use rustc_hir::{self as hir, HirIdSet}; -use rustc_macros::{LintDiagnostic, Subdiagnostic}; +use rustc_macros::LintDiagnostic; use rustc_middle::ty::TyCtxt; use rustc_session::lint::{FutureIncompatibilityReason, Level}; use rustc_session::{declare_lint, impl_lint_pass}; @@ -124,103 +124,109 @@ impl IfLetRescope { let source_map = tcx.sess.source_map(); let expr_end = expr.span.shrink_to_hi(); let mut add_bracket_to_match_head = match_head_needs_bracket(tcx, expr); + let mut significant_droppers = vec![]; + let mut lifetime_ends = vec![]; let mut closing_brackets = 0; let mut alt_heads = vec![]; let mut match_heads = vec![]; let mut consequent_heads = vec![]; - let mut first_if_to_rewrite = None; + let mut first_if_to_lint = None; + let mut first_if_to_rewrite = false; let mut empty_alt = false; while let hir::ExprKind::If(cond, conseq, alt) = expr.kind { self.skip.insert(expr.hir_id); - let hir::ExprKind::Let(&hir::LetExpr { + // We are interested in `let` fragment of the condition. + // Otherwise, we probe into the `else` fragment. + if let hir::ExprKind::Let(&hir::LetExpr { span, pat, init, ty: ty_ascription, recovered: Recovered::No, }) = cond.kind - else { - if let Some(alt) = alt { - add_bracket_to_match_head = matches!(alt.kind, hir::ExprKind::If(..)); - expr = alt; - continue; - } else { - // finalize and emit span - break; - } - }; - let if_let_pat = expr.span.shrink_to_lo().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); - - if let ControlFlow::Break(significant_dropper) = - (FindSignificantDropper { cx }).visit_expr(init) { - tcx.emit_node_span_lint( - IF_LET_RESCOPE, - expr.hir_id, - span, - IfLetRescopeLint { significant_dropper, lifetime_end }, - ); - if ty_ascription.is_some() - || !expr.span.can_be_used_for_suggestions() - || !pat.span.can_be_used_for_suggestions() + let if_let_pat = expr.span.shrink_to_lo().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); + + if let ControlFlow::Break(significant_dropper) = + (FindSignificantDropper { cx }).visit_expr(init) { - // Our `match` rewrites does not support type ascription, - // so we just bail. - // Alternatively when the span comes from proc macro expansion, - // we will also bail. - // FIXME(#101728): change this when type ascription syntax is stabilized again - } else if let Ok(pat) = source_map.span_to_snippet(pat.span) { - let emit_suggestion = || { - first_if_to_rewrite = - first_if_to_rewrite.or_else(|| Some((expr.span, expr.hir_id))); - if add_bracket_to_match_head { - closing_brackets += 2; - match_heads.push(SingleArmMatchBegin::WithOpenBracket(if_let_pat)); + first_if_to_lint = first_if_to_lint.or_else(|| Some((span, expr.hir_id))); + significant_droppers.push(significant_dropper); + lifetime_ends.push(lifetime_end); + if ty_ascription.is_some() + || !expr.span.can_be_used_for_suggestions() + || !pat.span.can_be_used_for_suggestions() + { + // Our `match` rewrites does not support type ascription, + // so we just bail. + // Alternatively when the span comes from proc macro expansion, + // we will also bail. + // FIXME(#101728): change this when type ascription syntax is stabilized again + } else if let Ok(pat) = source_map.span_to_snippet(pat.span) { + let emit_suggestion = |alt_span| { + first_if_to_rewrite = true; + if add_bracket_to_match_head { + closing_brackets += 2; + match_heads.push(SingleArmMatchBegin::WithOpenBracket(if_let_pat)); + } else { + // Sometimes, wrapping `match` into a block is undesirable, + // because the scrutinee temporary lifetime is shortened and + // the proposed fix will not work. + closing_brackets += 1; + match_heads + .push(SingleArmMatchBegin::WithoutOpenBracket(if_let_pat)); + } + consequent_heads.push(ConsequentRewrite { span: before_conseq, pat }); + if let Some(alt_span) = alt_span { + alt_heads.push(AltHead(alt_span)); + } + }; + if let Some(alt) = alt { + let alt_head = conseq.span.between(alt.span); + if alt_head.can_be_used_for_suggestions() { + // We lint only when the `else` span is user code, too. + emit_suggestion(Some(alt_head)); + } } else { - // It has to be a block - closing_brackets += 1; - match_heads.push(SingleArmMatchBegin::WithoutOpenBracket(if_let_pat)); + // This is the end of the `if .. else ..` cascade. + // We can stop here. + emit_suggestion(None); + empty_alt = true; + break; } - consequent_heads.push(ConsequentRewrite { span: before_conseq, pat }); - }; - if let Some(alt) = alt { - let alt_head = conseq.span.between(alt.span); - if alt_head.can_be_used_for_suggestions() { - // lint - emit_suggestion(); - alt_heads.push(AltHead(alt_head)); - } - } else { - emit_suggestion(); - empty_alt = true; - break; } } } + // At this point, any `if let` fragment in the cascade is definitely preceeded by `else`, + // so a opening bracket is mandatory before each `match`. + add_bracket_to_match_head = true; if let Some(alt) = alt { - add_bracket_to_match_head = matches!(alt.kind, hir::ExprKind::If(..)); expr = alt; } else { break; } } - if let Some((span, hir_id)) = first_if_to_rewrite { + if let Some((span, hir_id)) = first_if_to_lint { tcx.emit_node_span_lint( IF_LET_RESCOPE, hir_id, span, - IfLetRescopeRewrite { - match_heads, - consequent_heads, - closing_brackets: ClosingBrackets { - span: expr_end, - count: closing_brackets, - empty_alt, - }, - alt_heads, + IfLetRescopeLint { + significant_droppers, + lifetime_ends, + rewrite: first_if_to_rewrite.then_some(IfLetRescopeRewrite { + match_heads, + consequent_heads, + closing_brackets: ClosingBrackets { + span: expr_end, + count: closing_brackets, + empty_alt, + }, + alt_heads, + }), }, ); } @@ -254,32 +260,68 @@ impl<'tcx> LateLintPass<'tcx> for IfLetRescope { #[diag(lint_if_let_rescope)] struct IfLetRescopeLint { #[label] - significant_dropper: Span, + significant_droppers: Vec, #[help] - lifetime_end: Span, + lifetime_ends: Vec, + #[subdiagnostic] + rewrite: Option, } -#[derive(LintDiagnostic)] -#[diag(lint_if_let_rescope_suggestion)] +// #[derive(Subdiagnostic)] struct IfLetRescopeRewrite { - #[subdiagnostic] match_heads: Vec, - #[subdiagnostic] consequent_heads: Vec, - #[subdiagnostic] closing_brackets: ClosingBrackets, - #[subdiagnostic] alt_heads: Vec, } -#[derive(Subdiagnostic)] -#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] -struct AltHead(#[suggestion_part(code = " _ => ")] Span); +impl Subdiagnostic for IfLetRescopeRewrite { + fn add_to_diag_with>( + self, + diag: &mut Diag<'_, G>, + f: &F, + ) { + let mut suggestions = vec![]; + for match_head in self.match_heads { + match match_head { + SingleArmMatchBegin::WithOpenBracket(span) => { + suggestions.push((span, "{ match ".into())) + } + SingleArmMatchBegin::WithoutOpenBracket(span) => { + suggestions.push((span, "match ".into())) + } + } + } + for ConsequentRewrite { span, pat } in self.consequent_heads { + suggestions.push((span, format!("{{ {pat} => "))); + } + for AltHead(span) in self.alt_heads { + suggestions.push((span, " _ => ".into())); + } + let closing_brackets = self.closing_brackets; + suggestions.push(( + closing_brackets.span, + closing_brackets + .empty_alt + .then_some(" _ => {}".chars()) + .into_iter() + .flatten() + .chain(repeat('}').take(closing_brackets.count)) + .collect(), + )); + let msg = f(diag, crate::fluent_generated::lint_suggestion.into()); + diag.multipart_suggestion_with_style( + msg, + suggestions, + Applicability::MachineApplicable, + SuggestionStyle::ShowCode, + ); + } +} + +struct AltHead(Span); -#[derive(Subdiagnostic)] -#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] struct ConsequentRewrite { - #[suggestion_part(code = "{{ {pat} => ")] span: Span, pat: String, } @@ -289,36 +331,9 @@ struct ClosingBrackets { count: usize, empty_alt: bool, } - -impl Subdiagnostic for ClosingBrackets { - fn add_to_diag_with>( - self, - diag: &mut Diag<'_, G>, - f: &F, - ) { - let code: String = self - .empty_alt - .then_some(" _ => {}".chars()) - .into_iter() - .flatten() - .chain(repeat('}').take(self.count)) - .collect(); - let msg = f(diag, crate::fluent_generated::lint_suggestion.into()); - diag.multipart_suggestion_with_style( - msg, - vec![(self.span, code)], - Applicability::MachineApplicable, - SuggestionStyle::ShowCode, - ); - } -} - -#[derive(Subdiagnostic)] enum SingleArmMatchBegin { - #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] - WithOpenBracket(#[suggestion_part(code = "{{ match ")] Span), - #[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] - WithoutOpenBracket(#[suggestion_part(code = "match ")] Span), + WithOpenBracket(Span), + WithoutOpenBracket(Span), } struct FindSignificantDropper<'tcx, 'a> { diff --git a/tests/ui/drop/lint-if-let-rescope-gated.rs b/tests/ui/drop/lint-if-let-rescope-gated.rs index 08ded67a512..cef5de5a8fe 100644 --- a/tests/ui/drop/lint-if-let-rescope-gated.rs +++ b/tests/ui/drop/lint-if-let-rescope-gated.rs @@ -26,9 +26,9 @@ impl Droppy { fn main() { if let Some(_value) = Droppy.get() { //[with_feature_gate]~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //[with_feature_gate]~| ERROR: a `match` with a single arm can preserve the drop order up to Edition 2021 - //[with_feature_gate]~| WARN: this changes meaning in Rust 2024 + //[with_feature_gate]~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 //[with_feature_gate]~| WARN: this changes meaning in Rust 2024 } else { + //[with_feature_gate]~^ HELP: the value is now dropped here in Edition 2024 } } diff --git a/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr b/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr index e0f2bebdbf1..48b7f3e11a6 100644 --- a/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr +++ b/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr @@ -9,7 +9,7 @@ LL | if let Some(_value) = Droppy.get() { = 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-gated.rs:32:5 + --> $DIR/lint-if-let-rescope-gated.rs:31:5 | LL | } else { | ^ @@ -18,37 +18,16 @@ note: the lint level is defined here | LL | #![deny(if_let_rescope)] | ^^^^^^^^^^^^^^ +help: a `match` with a single arm can preserve the drop order up to Edition 2021 + | +LL ~ match Droppy.get() { Some(_value) => { +LL | +LL | +LL | +LL ~ } _ => { +LL | +LL ~ }} + | -error: a `match` with a single arm can preserve the drop order up to Edition 2021 - --> $DIR/lint-if-let-rescope-gated.rs:27:5 - | -LL | / if let Some(_value) = Droppy.get() { -LL | | -LL | | -LL | | -LL | | -LL | | } else { -LL | | } - | |_____^ - | - = warning: this changes meaning in Rust 2024 - = note: for more information, see issue #124085 -help: rewrite this `if let` into `match` - | -LL | match Droppy.get() { - | ~~~~~ -help: rewrite this `if let` into `match` - | -LL | if let Some(_value) = Droppy.get() { Some(_value) => { - | +++++++++++++++++ -help: rewrite this `if let` into `match` - | -LL | }} - | + -help: rewrite this `if let` into `match` - | -LL | } _ => { - | ~~~~ - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error diff --git a/tests/ui/drop/lint-if-let-rescope.fixed b/tests/ui/drop/lint-if-let-rescope.fixed index 85ffa320796..f228783f88b 100644 --- a/tests/ui/drop/lint-if-let-rescope.fixed +++ b/tests/ui/drop/lint-if-let-rescope.fixed @@ -27,73 +27,45 @@ fn main() { match droppy().get() { Some(_value) => { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 // do something } _ => { //~^ HELP: the value is now dropped here in Edition 2024 - //~| HELP: rewrite this `if let` into `match` // do something else }} - //~^ HELP: rewrite this `if let` into `match` match droppy().get() { Some(_value) => { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 // do something } _ => { match droppy().get() { Some(_value) => { //~^ HELP: the value is now dropped here in Edition 2024 - //~| ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` // do something else } _ => {}}}} - //~^ HELP: rewrite this `if let` into `match` - //~| HELP: the value is now dropped here in Edition 2024 + //~^ HELP: the value is now dropped here in Edition 2024 if droppy().get().is_some() { // Should not lint } else { match droppy().get() { Some(_value) => { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 } _ => if droppy().get().is_none() { //~^ HELP: the value is now dropped here in Edition 2024 - //~| HELP: rewrite this `if let` into `match` }}} - //~^ HELP: rewrite this `if let` into `match` if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ => { None }} } { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` //~| 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 () = { match Droppy.get() { Some(_value) => {} _ => {}} } { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` //~| 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 776b34fdbea..241fb897fce 100644 --- a/tests/ui/drop/lint-if-let-rescope.rs +++ b/tests/ui/drop/lint-if-let-rescope.rs @@ -27,73 +27,45 @@ fn main() { if let Some(_value) = droppy().get() { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 // do something } else { //~^ HELP: the value is now dropped here in Edition 2024 - //~| HELP: rewrite this `if let` into `match` // do something else } - //~^ HELP: rewrite this `if let` into `match` if let Some(_value) = droppy().get() { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 // do something } else if let Some(_value) = droppy().get() { //~^ HELP: the value is now dropped here in Edition 2024 - //~| ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` // do something else } - //~^ HELP: rewrite this `if let` into `match` - //~| HELP: the value is now dropped here in Edition 2024 + //~^ HELP: the value is now dropped here in Edition 2024 if droppy().get().is_some() { // Should not lint } else if let Some(_value) = droppy().get() { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 } else if droppy().get().is_none() { //~^ HELP: the value is now dropped here in Edition 2024 - //~| HELP: rewrite this `if let` into `match` } - //~^ HELP: rewrite this `if let` into `match` if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` //~| 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() {} } { //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 - //~| ERROR a `match` with a single arm can preserve the drop order up to Edition 2021 //~| WARN: this changes meaning in Rust 2024 - //~| WARN: this changes meaning in Rust 2024 - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` - //~| HELP: rewrite this `if let` into `match` //~| 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 a08d95e1138..25ca3cf1ca8 100644 --- a/tests/ui/drop/lint-if-let-rescope.stderr +++ b/tests/ui/drop/lint-if-let-rescope.stderr @@ -9,7 +9,7 @@ LL | if let Some(_value) = droppy().get() { = 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:35:5 + --> $DIR/lint-if-let-rescope.rs:32:5 | LL | } else { | ^ @@ -18,53 +18,52 @@ note: the lint level is defined here | LL | #![deny(if_let_rescope)] | ^^^^^^^^^^^^^^ - -error: a `match` with a single arm can preserve the drop order up to Edition 2021 - --> $DIR/lint-if-let-rescope.rs:27:5 +help: a `match` with a single arm can preserve the drop order up to Edition 2021 | -LL | / if let Some(_value) = droppy().get() { -LL | | -LL | | -LL | | -... | -LL | | // do something else -LL | | } - | |_____^ +LL ~ match droppy().get() { Some(_value) => { +LL | +... +LL | // do something +LL ~ } _ => { +LL | +LL | // do something else +LL ~ }} | - = warning: this changes meaning in Rust 2024 - = note: for more information, see issue #124085 -help: rewrite this `if let` into `match` - | -LL | match droppy().get() { - | ~~~~~ -help: rewrite this `if let` into `match` - | -LL | if let Some(_value) = droppy().get() { Some(_value) => { - | +++++++++++++++++ -help: rewrite this `if let` into `match` - | -LL | }} - | + -help: rewrite this `if let` into `match` - | -LL | } _ => { - | ~~~~ error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:42:8 + --> $DIR/lint-if-let-rescope.rs:37:8 | LL | 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 +... +LL | } else 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:50:5 + --> $DIR/lint-if-let-rescope.rs:42:5 | LL | } else if let Some(_value) = droppy().get() { | ^ +help: the value is now dropped here in Edition 2024 + --> $DIR/lint-if-let-rescope.rs:45:5 + | +LL | } + | ^ +help: a `match` with a single arm can preserve the drop order up to Edition 2021 + | +LL ~ match droppy().get() { Some(_value) => { +LL | +... +LL | // do something +LL ~ } _ => { match droppy().get() { Some(_value) => { +LL | +LL | // do something else +LL ~ } _ => {}}}} + | error: `if let` assigns a shorter lifetime since Edition 2024 --> $DIR/lint-if-let-rescope.rs:50:15 @@ -77,100 +76,23 @@ LL | } else if let Some(_value) = droppy().get() { = 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:58:5 - | -LL | } - | ^ - -error: a `match` with a single arm can preserve the drop order up to Edition 2021 - --> $DIR/lint-if-let-rescope.rs:42:5 - | -LL | / if let Some(_value) = droppy().get() { -LL | | -LL | | -LL | | -... | -LL | | // do something else -LL | | } - | |_____^ - | - = warning: this changes meaning in Rust 2024 - = note: for more information, see issue #124085 -help: rewrite this `if let` into `match` - | -LL | match droppy().get() { - | ~~~~~ -help: rewrite this `if let` into `match` - | -LL | } else { match droppy().get() { - | ~~~~~~~ -help: rewrite this `if let` into `match` - | -LL | if let Some(_value) = droppy().get() { Some(_value) => { - | +++++++++++++++++ -help: rewrite this `if let` into `match` - | -LL | } else if let Some(_value) = droppy().get() { Some(_value) => { - | +++++++++++++++++ -help: rewrite this `if let` into `match` - | -LL | } _ => {}}}} - | ++++++++++ -help: rewrite this `if let` into `match` - | -LL | } _ => if let Some(_value) = droppy().get() { - | ~~~~ - -error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:64:15 - | -LL | } else 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:71:5 + --> $DIR/lint-if-let-rescope.rs:54:5 | LL | } else if droppy().get().is_none() { | ^ - -error: a `match` with a single arm can preserve the drop order up to Edition 2021 - --> $DIR/lint-if-let-rescope.rs:64:12 +help: a `match` with a single arm can preserve the drop order up to Edition 2021 | -LL | } else if let Some(_value) = droppy().get() { - | ____________^ -LL | | -LL | | -LL | | -... | -LL | | -LL | | } - | |_____^ +LL ~ } else { match droppy().get() { Some(_value) => { +LL | +LL | +LL | +LL ~ } _ => if droppy().get().is_none() { +LL | +LL ~ }}} | - = warning: this changes meaning in Rust 2024 - = note: for more information, see issue #124085 -help: rewrite this `if let` into `match` - | -LL | } else { match droppy().get() { - | ~~~~~~~ -help: rewrite this `if let` into `match` - | -LL | } else if let Some(_value) = droppy().get() { Some(_value) => { - | +++++++++++++++++ -help: rewrite this `if let` into `match` - | -LL | }}} - | ++ -help: rewrite this `if let` into `match` - | -LL | } _ => if droppy().get().is_none() { - | ~~~~ error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:77:27 + --> $DIR/lint-if-let-rescope.rs:58:27 | LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { | ^^^^^^^^^^^^^^^^^^^------^^^^^^ @@ -180,38 +102,17 @@ LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else = 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:77:69 + --> $DIR/lint-if-let-rescope.rs:58:69 | LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { | ^ - -error: a `match` with a single arm can preserve the drop order up to Edition 2021 - --> $DIR/lint-if-let-rescope.rs:77:24 +help: a `match` with a single arm can preserve the drop order up to Edition 2021 | -LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = warning: this changes meaning in Rust 2024 - = note: for more information, see issue #124085 -help: rewrite this `if let` into `match` - | -LL | if let Some(1) = { match Droppy.get() { Some(1) } else { None } } { - | ~~~~~ -help: rewrite this `if let` into `match` - | -LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(_value) => { Some(1) } else { None } } { - | +++++++++++++++++ -help: rewrite this `if let` into `match` - | -LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None }} } { - | + -help: rewrite this `if let` into `match` - | -LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } _ => { None } } { - | ~~~~ +LL | if let Some(1) = { match Droppy.get() { Some(_value) => { Some(1) } _ => { None }} } { + | ~~~~~ +++++++++++++++++ ~~~~ + error: `if let` assigns a shorter lifetime since Edition 2024 - --> $DIR/lint-if-let-rescope.rs:89:22 + --> $DIR/lint-if-let-rescope.rs:65:22 | LL | if let () = { if let Some(_value) = Droppy.get() {} } { | ^^^^^^^^^^^^^^^^^^^------^^^^^^ @@ -221,31 +122,14 @@ LL | if let () = { if let Some(_value) = Droppy.get() {} } { = 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:89:55 + --> $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: a `match` with a single arm can preserve the drop order up to Edition 2021 - --> $DIR/lint-if-let-rescope.rs:89:19 - | -LL | if let () = { if let Some(_value) = Droppy.get() {} } { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = warning: this changes meaning in Rust 2024 - = note: for more information, see issue #124085 -help: rewrite this `if let` into `match` - | -LL | if let () = { match Droppy.get() {} } { - | ~~~~~ -help: rewrite this `if let` into `match` - | -LL | if let () = { if let Some(_value) = Droppy.get() { Some(_value) => {} } { - | +++++++++++++++++ -help: rewrite this `if let` into `match` - | -LL | if let () = { if let Some(_value) = Droppy.get() {} _ => {}} } { - | ++++++++ - -error: aborting due to 11 previous errors +error: aborting due to 5 previous errors