Skip to content

Commit

Permalink
rescope temp lifetime in let-chain into IfElse
Browse files Browse the repository at this point in the history
  • Loading branch information
dingxiangfei2009 committed May 15, 2024
1 parent ade234d commit c13eb60
Show file tree
Hide file tree
Showing 12 changed files with 260 additions and 31 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,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
3 changes: 2 additions & 1 deletion compiler/rustc_hir_typeck/src/rvalue_scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ pub fn resolve_rvalue_scopes<'a, 'tcx>(
def_id: DefId,
) -> RvalueScopes {
let tcx = &fcx.tcx;
let mut rvalue_scopes = RvalueScopes::new();
let mut rvalue_scopes =
RvalueScopes::new(tcx.features().if_let_rescope && tcx.sess.at_least_rust_2024());
debug!("start resolving rvalue scopes, def_id={def_id:?}");
debug!("rvalue_scope: rvalue_candidates={:?}", scope_tree.rvalue_candidates);
for (&hir_id, candidate) in &scope_tree.rvalue_candidates {
Expand Down
17 changes: 14 additions & 3 deletions compiler/rustc_middle/src/ty/rvalue_scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ use rustc_macros::{HashStable, TyDecodable, TyEncodable};
/// rules laid out in `rustc_hir_analysis::check::rvalue_scopes`.
#[derive(TyEncodable, TyDecodable, Clone, Debug, Default, Eq, PartialEq, HashStable)]
pub struct RvalueScopes {
rescope_if_let: bool,
map: ItemLocalMap<Option<Scope>>,
}

impl RvalueScopes {
pub fn new() -> Self {
Self { map: <_>::default() }
pub fn new(rescope_if_let: bool) -> Self {
Self { rescope_if_let, map: <_>::default() }
}

/// Returns the scope when the temp created by `expr_id` will be cleaned up.
Expand All @@ -39,7 +40,17 @@ impl RvalueScopes {
debug!("temporary_scope({expr_id:?}) = {id:?} [enclosing]");
return Some(id);
}
_ => id = p,
ScopeData::IfThen => {
if self.rescope_if_let {
debug!("temporary_scope({expr_id:?}) = {p:?} [enclosing]");
return Some(p);
}
id = p;
}
ScopeData::Node
| ScopeData::CallSite
| ScopeData::Arguments
| ScopeData::Remainder(_) => id = p,
}
}

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 @@ -978,6 +978,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`.
11 changes: 11 additions & 0 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 --cfg edition2024
//@ [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;
Expand Down Expand Up @@ -57,7 +62,10 @@ fn main() {
d(10, None)
},
);
#[cfg(not(edition2024))]
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]);

Expand Down Expand Up @@ -89,5 +97,8 @@ fn main() {
panic::panic_any(InjectedFailure)
);
});
#[cfg(not(edition2024))]
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]);
}
40 changes: 40 additions & 0 deletions tests/ui/mir/mir_let_chains_drop_order.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
warning: unexpected `cfg` condition name: `edition2024`
--> $DIR/mir_let_chains_drop_order.rs:99:15
|
LL | #[cfg(not(edition2024))]
| ^^^^^^^^^^^
|
= help: expected names are: `FALSE`, `clippy`, `debug_assertions`, `doc`, `doctest`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `rustfmt`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
= help: to expect this configuration use `--check-cfg=cfg(edition2024)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name: `edition2024`
--> $DIR/mir_let_chains_drop_order.rs:101:15
|
LL | #[cfg(not(edition2024))]
| ^^^^^^^^^^^
|
= help: to expect this configuration use `--check-cfg=cfg(edition2024)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration

warning: unexpected `cfg` condition name: `edition2024`
--> $DIR/mir_let_chains_drop_order.rs:64:19
|
LL | #[cfg(not(edition2024))]
| ^^^^^^^^^^^
|
= help: to expect this configuration use `--check-cfg=cfg(edition2024)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration

warning: unexpected `cfg` condition name: `edition2024`
--> $DIR/mir_let_chains_drop_order.rs:66:15
|
LL | #[cfg(edition2024)]
| ^^^^^^^^^^^
|
= help: to expect this configuration use `--check-cfg=cfg(edition2024)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration

warning: 4 warnings emitted

4 changes: 3 additions & 1 deletion tests/ui/nll/issue-54556-niconii.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@ check-pass
// This is a reduction of a concrete test illustrating a case that was
// annoying to Rust developer niconii (see comment thread on #21114).
//
Expand All @@ -19,7 +20,7 @@ impl Mutex {
fn main() {
let counter = Mutex;

if let Ok(_) = counter.lock() { } //~ ERROR does not live long enough
if let Ok(_) = counter.lock() { }

// With this code as written, the dynamic semantics here implies
// that `Mutex::drop` for `counter` runs *before*
Expand All @@ -28,4 +29,5 @@ fn main() {
//
// The goal of #54556 is to explain that within a compiler
// diagnostic.
()
}
Loading

0 comments on commit c13eb60

Please sign in to comment.