Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rescope temp lifetime in let-chain into IfElse #107251

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,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, "1.78.0", 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.
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,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::IfThen2024
} 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);
Expand All @@ -481,7 +486,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::IfThen2024
} 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);
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,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::IfThen2024 => write!(fmt, "IfThen[edition2024]({:?})", self.id),
ScopeData::Remainder(fsi) => write!(
fmt,
"Remainder {{ block: {:?}, first_statement_index: {}}}",
Expand Down Expand Up @@ -125,6 +126,11 @@ pub enum ScopeData {
/// Used for variables introduced in an if-let expression.
IfThen,

/// Edition 2024: 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.
IfThen2024,

/// Scope following a `let id = expr;` binding in a block.
Remainder(FirstStatementIndex),
}
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_middle/src/ty/rvalue_scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,15 @@ impl RvalueScopes {
debug!("temporary_scope({expr_id:?}) = {id:?} [enclosing]");
return Some(id);
}
_ => id = p,
ScopeData::IfThen2024 => {
debug!("temporary_scope({expr_id:?}) = {p:?} [enclosing]");
return Some(p);
}
ScopeData::Node
| ScopeData::CallSite
| ScopeData::Arguments
| ScopeData::IfThen
| ScopeData::Remainder(_) => id = p,
}
}

Expand Down
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
Expand Up @@ -712,7 +712,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::IfThen2024
} else {
region::ScopeData::IfThen
}
},
},
cond: self.mirror_expr(cond),
then: self.mirror_expr(then),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,7 @@ symbols! {
ident,
if_let,
if_let_guard,
if_let_rescope,
if_while_or_patterns,
ignore,
impl_header_lifetime_elision,
Expand Down
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 --cfg edition2024
//@ [edition2024] edition: 2024
#![feature(let_chains)]
#![cfg_attr(edition2024, feature(if_let_rescope))]

use std::cell::RefCell;
use std::convert::TryInto;
Expand Down Expand Up @@ -55,11 +60,18 @@ impl DropOrderCollector {
}

fn if_let(&self) {
#[cfg(not(edition2024))]
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);
Expand Down Expand Up @@ -194,6 +206,7 @@ impl DropOrderCollector {
self.print(3); // 3
}

#[cfg(not(edition2024))]
// take the "else" branch
if self.option_loud_drop(5).is_some() // 1
&& self.option_loud_drop(6).is_some() // 2
Expand All @@ -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
Expand Down
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();
}
25 changes: 25 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,25 @@
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:22: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`.
Loading
Loading