diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 5dec019126a..4a31aee5c96 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 { @@ -2859,7 +2872,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..d9b5fd8b5c1 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(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)) { 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(tcx, expr, &pat, init, conseq, alt, err); } } } @@ -390,6 +428,53 @@ impl<'tcx> BorrowExplanation<'tcx> { } } +fn suggest_rewrite_if_let( + tcx: TyCtxt<'_>, + expr: &hir::Expr<'_>, + pat: &str, + init: &hir::Expr<'_>, + conseq: &hir::Expr<'_>, + alt: Option<&hir::Expr<'_>>, + err: &mut Diag<'_>, +) { + let source_map = tcx.sess.source_map(); + err.span_note( + 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), + 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), " _ => ".into())); + expr_end_code = "}".to_string(); + } else { + 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::MaybeIncorrect, + ); + } +} + impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> { fn free_region_constraint_info( &self, @@ -465,14 +550,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 007b8753cfa..6ed5ea9f8bb 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -495,6 +495,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..4aa86944a0b 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 = 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 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..5b65541bea6 --- /dev/null +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -0,0 +1,430 @@ +use std::iter::repeat; +use std::ops::ControlFlow; + +use hir::intravisit::Visitor; +use rustc_ast::Recovered; +use rustc_errors::{ + Applicability, Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic, SuggestionStyle, +}; +use rustc_hir::{self as hir, HirIdSet}; +use rustc_macros::LintDiagnostic; +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; + +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 ", + }; +} + +/// 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 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_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); + // 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 + { + 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) + { + 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 { + // This is the end of the `if .. else ..` cascade. + // We can stop here. + emit_suggestion(None); + 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 { + expr = alt; + } else { + break; + } + } + if let Some((span, hir_id)) = first_if_to_lint { + tcx.emit_node_span_lint( + IF_LET_RESCOPE, + hir_id, + span, + 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, + }), + }, + ); + } + } +} + +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_2024() || !cx.tcx.features().if_let_rescope { + return; + } + if let (Level::Allow, _) = cx.tcx.lint_level_at_node(IF_LET_RESCOPE, expr.hir_id) { + return; + } + if expr_parent_is_stmt(cx.tcx, expr.hir_id) + && matches!(expr.kind, hir::ExprKind::If(_cond, _conseq, None)) + { + // `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 IfLetRescopeLint { + #[label] + significant_droppers: Vec, + #[help] + lifetime_ends: Vec, + #[subdiagnostic] + rewrite: Option, +} + +// #[derive(Subdiagnostic)] +struct IfLetRescopeRewrite { + match_heads: Vec, + consequent_heads: Vec, + closing_brackets: ClosingBrackets, + alt_heads: Vec, +} + +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); + +struct ConsequentRewrite { + span: Span, + pat: String, +} + +struct ClosingBrackets { + span: Span, + count: usize, + empty_alt: bool, +} +enum SingleArmMatchBegin { + WithOpenBracket(Span), + WithoutOpenBracket(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..0c32157758c 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::default(), ] ] ); 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 26addb1e357..e312e65cc21 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 f1f362ac3e8..cbe2bafef21 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1016,6 +1016,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.rs b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs new file mode 100644 index 00000000000..2476f7cf258 --- /dev/null +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.rs @@ -0,0 +1,33 @@ +//@ edition: 2024 +//@ compile-flags: -Z validate-mir -Zunstable-options + +#![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 + 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 new file mode 100644 index 00000000000..0c6f1ea28d2 --- /dev/null +++ b/tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr @@ -0,0 +1,89 @@ +error[E0716]: temporary value dropped while borrowed + --> $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: 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; +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[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`. 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..cef5de5a8fe --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope-gated.rs @@ -0,0 +1,34 @@ +// 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]~| 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 new file mode 100644 index 00000000000..48b7f3e11a6 --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr @@ -0,0 +1,33 @@ +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:31:5 + | +LL | } else { + | ^ +note: the lint level is defined here + --> $DIR/lint-if-let-rescope-gated.rs:11:9 + | +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: 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..f228783f88b --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope.fixed @@ -0,0 +1,71 @@ +//@ run-rustfix + +#![deny(if_let_rescope)] +#![feature(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() { + // 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: 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 + // do something else + }} + + match droppy().get() { Some(_value) => { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 + // do something + } _ => { match droppy().get() { Some(_value) => { + //~^ HELP: the value is now dropped here in Edition 2024 + // do something else + } _ => {}}}} + //~^ 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 + //~| WARN: this changes meaning in Rust 2024 + //~| 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 + }}} + + 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: 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 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: the value is now dropped here in Edition 2024 + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 + } +} diff --git a/tests/ui/drop/lint-if-let-rescope.rs b/tests/ui/drop/lint-if-let-rescope.rs new file mode 100644 index 00000000000..241fb897fce --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope.rs @@ -0,0 +1,71 @@ +//@ run-rustfix + +#![deny(if_let_rescope)] +#![feature(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() { + // 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: 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 + // do something else + } + + if let Some(_value) = droppy().get() { + //~^ ERROR: `if let` assigns a shorter lifetime since Edition 2024 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 + // do something + } else if let Some(_value) = droppy().get() { + //~^ HELP: the value is now dropped here in Edition 2024 + // do something else + } + //~^ 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 + //~| WARN: this changes meaning in Rust 2024 + //~| 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 + } + + 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: 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 + //~| WARN: this changes meaning in Rust 2024 + //~| HELP: the value is now dropped here in Edition 2024 + //~| HELP: a `match` with a single arm can preserve the drop order up to Edition 2021 + } +} diff --git a/tests/ui/drop/lint-if-let-rescope.stderr b/tests/ui/drop/lint-if-let-rescope.stderr new file mode 100644 index 00000000000..25ca3cf1ca8 --- /dev/null +++ b/tests/ui/drop/lint-if-let-rescope.stderr @@ -0,0 +1,135 @@ +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.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.rs:32:5 + | +LL | } else { + | ^ +note: the lint level is defined here + --> $DIR/lint-if-let-rescope.rs:3:9 + | +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 | // 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: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: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 + | +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:54:5 + | +LL | } else if droppy().get().is_none() { + | ^ +help: a `match` with a single arm can preserve the drop order up to Edition 2021 + | +LL ~ } else { match droppy().get() { Some(_value) => { +LL | +LL | +LL | +LL ~ } _ => if droppy().get().is_none() { +LL | +LL ~ }}} + | + +error: `if let` assigns a shorter lifetime since Edition 2024 + --> $DIR/lint-if-let-rescope.rs:58: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:58:69 + | +LL | if let Some(1) = { if let Some(_value) = Droppy.get() { Some(1) } else { None } } { + | ^ +help: a `match` with a single arm can 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:65:22 + | +LL | if let () = { if let Some(_value) = Droppy.get() {} } { + | ^^^^^^^^^^^^^^^^^^^------^^^^^^ + | | + | this value has a significant drop implementation which may observe a major change in drop order and requires your discretion + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #124085 +help: the value is now dropped here in Edition 2024 + --> $DIR/lint-if-let-rescope.rs:65:55 + | +LL | if let () = { if let Some(_value) = Droppy.get() {} } { + | ^ +help: a `match` with a single arm can preserve the drop order up to Edition 2021 + | +LL | if let () = { match Droppy.get() { Some(_value) => {} _ => {}} } { + | ~~~~~ +++++++++++++++++ ++++++++ + +error: aborting due to 5 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. }