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

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

Merged
merged 3 commits into from
Jul 30, 2022
Merged
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
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!();
}
}