Skip to content

Commit

Permalink
Auto merge of #107251 - dingxiangfei2009:let-chain-rescope, r=jieyouxu
Browse files Browse the repository at this point in the history
Rescope temp lifetime in if-let into IfElse with migration lint

Tracking issue #124085

This PR shortens the temporary lifetime to cover only the pattern matching and consequent branch of a `if let`.

At the expression location, means that the lifetime is shortened from previously the deepest enclosing block or statement in Edition 2021. This warrants an Edition change.

Coming with the Edition change, this patch also implements an edition lint to warn about the change and a safe rewrite suggestion to preserve the 2021 semantics in most cases.

Related to #103108.
Related crater runs: #129466.
bors committed Sep 13, 2024
2 parents d3a8524 + b4b2b35 commit a5efa01
Showing 29 changed files with 1,393 additions and 35 deletions.
29 changes: 21 additions & 8 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
@@ -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,
110 changes: 101 additions & 9 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
@@ -30,7 +30,7 @@ use crate::{MirBorrowckCtxt, WriteKind};

#[derive(Debug)]
pub(crate) enum BorrowExplanation<'tcx> {
UsedLater(LaterUseKind, Span, Option<Span>),
UsedLater(Local, LaterUseKind, Span, Option<Span>),
UsedLaterInLoop(LaterUseKind, Span, Option<Span>),
UsedLaterWhenDropped {
drop_loc: Location,
@@ -99,17 +99,39 @@ 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",
LaterUseKind::Call => "used by call",
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 = <expr>` 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,
)
}
}

2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 12 additions & 2 deletions compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
@@ -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);
5 changes: 5 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
@@ -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 ->
430 changes: 430 additions & 0 deletions compiler/rustc_lint/src/if_let_rescope.rs

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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(),
]
]
);
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/middle/region.rs
Original file line number Diff line number Diff line change
@@ -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),
}
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
@@ -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.
10 changes: 9 additions & 1 deletion compiler/rustc_middle/src/ty/rvalue_scopes.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}

10 changes: 10 additions & 0 deletions compiler/rustc_mir_build/src/build/expr/as_temp.rs
Original file line number Diff line number Diff line change
@@ -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);
}
8 changes: 7 additions & 1 deletion compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
@@ -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),
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
@@ -1016,6 +1016,7 @@ symbols! {
ident,
if_let,
if_let_guard,
if_let_rescope,
if_while_or_patterns,
ignore,
impl_header_lifetime_elision,
22 changes: 22 additions & 0 deletions tests/ui/drop/drop_order.rs
Original file line number Diff line number Diff line change
@@ -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
122 changes: 122 additions & 0 deletions tests/ui/drop/drop_order_if_let_rescope.rs
Original file line number Diff line number Diff line change
@@ -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<Vec<u32>>);

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<LoudDrop> {
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();
}
33 changes: 33 additions & 0 deletions tests/ui/drop/if-let-rescope-borrowck-suggestions.rs
Original file line number Diff line number Diff line change
@@ -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>(_: &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
});
}
89 changes: 89 additions & 0 deletions tests/ui/drop/if-let-rescope-borrowck-suggestions.stderr
Original file line number Diff line number Diff line change
@@ -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`.
34 changes: 34 additions & 0 deletions tests/ui/drop/lint-if-let-rescope-gated.rs
Original file line number Diff line number Diff line change
@@ -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<u8> {
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
}
}
33 changes: 33 additions & 0 deletions tests/ui/drop/lint-if-let-rescope-gated.with_feature_gate.stderr
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/rust-lang/rust/issues/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

41 changes: 41 additions & 0 deletions tests/ui/drop/lint-if-let-rescope-with-macro.rs
Original file line number Diff line number Diff line change
@@ -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<u8> {
None
}
}

fn main() {
edition_2021_if_let! {
Some(_value),
droppy().get(),
{}
{}
};
}
39 changes: 39 additions & 0 deletions tests/ui/drop/lint-if-let-rescope-with-macro.stderr
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/rust-lang/rust/issues/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

71 changes: 71 additions & 0 deletions tests/ui/drop/lint-if-let-rescope.fixed
Original file line number Diff line number Diff line change
@@ -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<u8> {
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
}
}
71 changes: 71 additions & 0 deletions tests/ui/drop/lint-if-let-rescope.rs
Original file line number Diff line number Diff line change
@@ -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<u8> {
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
}
}
135 changes: 135 additions & 0 deletions tests/ui/drop/lint-if-let-rescope.stderr
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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

27 changes: 27 additions & 0 deletions tests/ui/feature-gates/feature-gate-if-let-rescope.rs
Original file line number Diff line number Diff line change
@@ -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<B<'_, Self>> {
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
};
}
18 changes: 18 additions & 0 deletions tests/ui/feature-gates/feature-gate-if-let-rescope.stderr
Original file line number Diff line number Diff line change
@@ -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<B<'_, A>>`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0499`.
41 changes: 29 additions & 12 deletions tests/ui/mir/mir_let_chains_drop_order.rs
Original file line number Diff line number Diff line change
@@ -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]);
}
Original file line number Diff line number Diff line change
@@ -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
19 changes: 18 additions & 1 deletion tests/ui/nll/issue-54556-niconii.rs
Original file line number Diff line number Diff line change
@@ -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,13 +27,22 @@ 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`
// still has a reference to `counter`.
//
// 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.
}

0 comments on commit a5efa01

Please sign in to comment.