Skip to content

Commit

Permalink
Rollup merge of #99518 - dingxiangfei2009:let-else-additional-tests, …
Browse files Browse the repository at this point in the history
…r=oli-obk

Let-else: break out scopes when a let-else pattern fails to match

This PR will commit to a new behavior so that values from initializer expressions are dropped earlier when a let-else pattern fails to match.

Fix #98672.
Close #93951.
cc `@camsteffen` `@est31`
  • Loading branch information
JohnTitor authored Jul 29, 2022
2 parents 36ab4ec + 60be2de commit 955091b
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 42 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
initializer_span,
else_block,
visibility_scope,
*remainder_scope,
remainder_span,
pattern,
)
Expand Down
86 changes: 46 additions & 40 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2282,49 +2282,55 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
initializer_span: Span,
else_block: &Block,
visibility_scope: Option<SourceScope>,
remainder_scope: region::Scope,
remainder_span: Span,
pattern: &Pat<'tcx>,
) -> BlockAnd<()> {
let scrutinee = unpack!(block = self.lower_scrutinee(block, init, initializer_span));
let pat = Pat { ty: init.ty, span: else_block.span, kind: Box::new(PatKind::Wild) };
let mut wildcard = Candidate::new(scrutinee.clone(), &pat, false);
self.declare_bindings(
visibility_scope,
remainder_span,
pattern,
ArmHasGuard(false),
Some((None, initializer_span)),
);
let mut candidate = Candidate::new(scrutinee.clone(), pattern, false);
let fake_borrow_temps = self.lower_match_tree(
block,
initializer_span,
pattern.span,
false,
&mut [&mut candidate, &mut wildcard],
);
// This block is for the matching case
let matching = self.bind_pattern(
self.source_info(pattern.span),
candidate,
None,
&fake_borrow_temps,
initializer_span,
None,
None,
None,
);
// This block is for the failure case
let failure = self.bind_pattern(
self.source_info(else_block.span),
wildcard,
None,
&fake_borrow_temps,
initializer_span,
None,
None,
None,
);
let (matching, failure) = self.in_if_then_scope(remainder_scope, |this| {
let scrutinee = unpack!(block = this.lower_scrutinee(block, init, initializer_span));
let pat = Pat { ty: init.ty, span: else_block.span, kind: Box::new(PatKind::Wild) };
let mut wildcard = Candidate::new(scrutinee.clone(), &pat, false);
this.declare_bindings(
visibility_scope,
remainder_span,
pattern,
ArmHasGuard(false),
Some((None, initializer_span)),
);
let mut candidate = Candidate::new(scrutinee.clone(), pattern, false);
let fake_borrow_temps = this.lower_match_tree(
block,
initializer_span,
pattern.span,
false,
&mut [&mut candidate, &mut wildcard],
);
// This block is for the matching case
let matching = this.bind_pattern(
this.source_info(pattern.span),
candidate,
None,
&fake_borrow_temps,
initializer_span,
None,
None,
None,
);
// This block is for the failure case
let failure = this.bind_pattern(
this.source_info(else_block.span),
wildcard,
None,
&fake_borrow_temps,
initializer_span,
None,
None,
None,
);
this.break_for_else(failure, remainder_scope, this.source_info(initializer_span));
matching.unit()
});

// This place is not really used because this destination place
// should never be used to take values at the end of the failure
// block.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
drops.add_entry(block, drop_idx);

// `build_drop_tree` doesn't have access to our source_info, so we
// `build_drop_trees` doesn't have access to our source_info, so we
// create a dummy terminator now. `TerminatorKind::Resume` is used
// because MIR type checking will panic if it hasn't been overwritten.
self.cfg.terminate(block, source_info, TerminatorKind::Resume);
Expand Down Expand Up @@ -722,7 +722,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
drops.add_entry(block, drop_idx);

// `build_drop_tree` doesn't have access to our source_info, so we
// `build_drop_trees` doesn't have access to our source_info, so we
// create a dummy terminator now. `TerminatorKind::Resume` is used
// because MIR type checking will panic if it hasn't been overwritten.
self.cfg.terminate(block, source_info, TerminatorKind::Resume);
Expand Down
26 changes: 26 additions & 0 deletions src/test/ui/let-else/let-else-temp-borrowck.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// run-pass
//
// from issue #93951, where borrowck complained the temporary that `foo(&x)` was stored in was to
// be dropped sometime after `x` was. It then suggested adding a semicolon that was already there.

#![feature(let_else)]
use std::fmt::Debug;

fn foo<'a>(x: &'a str) -> Result<impl Debug + 'a, ()> {
Ok(x)
}

fn let_else() {
let x = String::from("Hey");
let Ok(_) = foo(&x) else { return };
}

fn if_let() {
let x = String::from("Hey");
let _ = if let Ok(s) = foo(&x) { s } else { return };
}

fn main() {
let_else();
if_let();
}
63 changes: 63 additions & 0 deletions src/test/ui/let-else/let-else-temporary-lifetime.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// run-pass
#![feature(let_else)]

use std::fmt::Display;
use std::rc::Rc;
use std::sync::atomic::{AtomicU8, Ordering};

static TRACKER: AtomicU8 = AtomicU8::new(0);
Expand All @@ -17,9 +19,70 @@ impl Drop for Droppy {
}
}

fn foo<'a>(x: &'a str) -> Result<impl Display + 'a, ()> {
Ok(x)
}

fn main() {
assert_eq!(TRACKER.load(Ordering::Acquire), 0);
let 0 = Droppy::default().inner else { return };
assert_eq!(TRACKER.load(Ordering::Acquire), 1);
println!("Should have dropped 👆");

{
// cf. https://github.com/rust-lang/rust/pull/99518#issuecomment-1191520030
struct Foo<'a>(&'a mut u32);

impl<'a> Drop for Foo<'a> {
fn drop(&mut self) {
*self.0 = 0;
}
}
let mut foo = 0;
let Foo(0) = Foo(&mut foo) else {
*&mut foo = 1;
todo!()
};
}
{
let x = String::from("Hey");

let Ok(s) = foo(&x) else { panic!() };
assert_eq!(s.to_string(), x);
}
{
// test let-else drops temps after statement
let rc = Rc::new(0);
let 0 = *rc.clone() else { unreachable!() };
Rc::try_unwrap(rc).unwrap();
}
{
let mut rc = Rc::new(0);
let mut i = 0;
loop {
if i > 3 {
break;
}
let 1 = *rc.clone() else {
if let Ok(v) = Rc::try_unwrap(rc) {
rc = Rc::new(v);
} else {
panic!()
}
i += 1;
continue
};
}
}
{
// test let-else drops temps before else block
// NOTE: this test has to be the last block in the `main`
// body.
let rc = Rc::new(0);
let 1 = *rc.clone() else {
Rc::try_unwrap(rc).unwrap();
return;
};
unreachable!();
}
}

0 comments on commit 955091b

Please sign in to comment.