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

Avoid duplicating StorageLive in let-else #101894

Merged
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
3 changes: 2 additions & 1 deletion compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pattern,
UserTypeProjections::none(),
&mut |this, _, _, _, node, span, _, _| {
this.storage_live_binding(block, node, span, OutsideGuard, false);
this.storage_live_binding(block, node, span, OutsideGuard, true);
this.schedule_drop_for_binding(node, span, OutsideGuard);
},
);
let failure = unpack!(
Expand Down
39 changes: 31 additions & 8 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Some(arm.span),
Some(arm.scope),
Some(match_scope),
false,
);

if let Some(source_scope) = scope {
Expand Down Expand Up @@ -416,6 +417,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
arm_span: Option<Span>,
arm_scope: Option<region::Scope>,
match_scope: Option<region::Scope>,
storages_alive: bool,
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
) -> BasicBlock {
if candidate.subcandidates.is_empty() {
// Avoid generating another `BasicBlock` when we only have one
Expand All @@ -429,6 +431,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
arm_span,
match_scope,
true,
storages_alive,
)
} else {
// It's helpful to avoid scheduling drops multiple times to save
Expand Down Expand Up @@ -466,6 +469,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
arm_span,
match_scope,
schedule_drops,
storages_alive,
);
if arm_scope.is_none() {
schedule_drops = false;
Expand Down Expand Up @@ -641,6 +645,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
None,
None,
None,
false,
)
.unit()
}
Expand Down Expand Up @@ -1813,6 +1818,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
None,
None,
None,
false,
);

post_guard_block.unit()
Expand All @@ -1836,6 +1842,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
arm_span: Option<Span>,
match_scope: Option<region::Scope>,
schedule_drops: bool,
storages_alive: bool,
) -> BasicBlock {
debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate);

Expand Down Expand Up @@ -2051,7 +2058,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.push_fake_read(post_guard_block, guard_end, cause, Place::from(local_id));
}
assert!(schedule_drops, "patterns with guards must schedule drops");
self.bind_matched_candidate_for_arm_body(post_guard_block, true, by_value_bindings);
self.bind_matched_candidate_for_arm_body(
post_guard_block,
true,
by_value_bindings,
storages_alive,
);

post_guard_block
} else {
Expand All @@ -2065,6 +2077,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.iter()
.flat_map(|(bindings, _)| bindings)
.chain(&candidate.bindings),
storages_alive,
);
block
}
Expand Down Expand Up @@ -2154,6 +2167,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block: BasicBlock,
schedule_drops: bool,
bindings: impl IntoIterator<Item = &'b Binding<'tcx>>,
storages_alive: bool,
) where
'tcx: 'b,
{
Expand All @@ -2163,13 +2177,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Assign each of the bindings. This may trigger moves out of the candidate.
for binding in bindings {
let source_info = self.source_info(binding.span);
let local = self.storage_live_binding(
block,
binding.var_id,
binding.span,
OutsideGuard,
schedule_drops,
);
let local = if storages_alive {
// Here storages are already alive, probably because this is a binding
// from let-else.
// We just need to schedule drop for the value.
self.var_local_id(binding.var_id, OutsideGuard).into()
} else {
self.storage_live_binding(
block,
binding.var_id,
binding.span,
OutsideGuard,
schedule_drops,
)
};
if schedule_drops {
self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard);
}
Expand Down Expand Up @@ -2300,6 +2321,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
None,
None,
None,
true,
);
// This block is for the failure case
let failure = this.bind_pattern(
Expand All @@ -2311,6 +2333,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
None,
None,
None,
true,
);
this.break_for_else(failure, *let_else_scope, this.source_info(initializer_span));
matching.unit()
Expand Down
9 changes: 9 additions & 0 deletions src/test/mir-opt/issue-101867.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![cfg_attr(bootstrap, feature(let_else))]

// EMIT_MIR issue_101867.main.mir_map.0.mir
fn main() {
let x: Option<u8> = Some(1);
let Some(y) = x else {
panic!();
};
}
75 changes: 75 additions & 0 deletions src/test/mir-opt/issue_101867.main.mir_map.0.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// MIR for `main` 0 mir_map

| User Type Annotations
| 0: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option<u8>) }, span: $DIR/issue-101867.rs:5:12: 5:22, inferred_ty: std::option::Option<u8>
| 1: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option<u8>) }, span: $DIR/issue-101867.rs:5:12: 5:22, inferred_ty: std::option::Option<u8>
|
fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/issue-101867.rs:+0:11: +0:11
let _1: std::option::Option<u8> as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 0 at $DIR/issue-101867.rs:+1:9: +1:10
let mut _2: !; // in scope 0 at $DIR/issue-101867.rs:+2:26: +4:6
let _3: (); // in scope 0 at $SRC_DIR/std/src/panic.rs:LL:COL
let mut _4: !; // in scope 0 at $SRC_DIR/std/src/panic.rs:LL:COL
let mut _6: isize; // in scope 0 at $DIR/issue-101867.rs:+2:9: +2:16
scope 1 {
debug x => _1; // in scope 1 at $DIR/issue-101867.rs:+1:9: +1:10
let _5: u8; // in scope 1 at $DIR/issue-101867.rs:+2:14: +2:15
scope 2 {
debug y => _5; // in scope 2 at $DIR/issue-101867.rs:+2:14: +2:15
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/issue-101867.rs:+1:9: +1:10
_1 = Option::<u8>::Some(const 1_u8); // scope 0 at $DIR/issue-101867.rs:+1:25: +1:32
FakeRead(ForLet(None), _1); // scope 0 at $DIR/issue-101867.rs:+1:9: +1:10
AscribeUserType(_1, o, UserTypeProjection { base: UserType(1), projs: [] }); // scope 0 at $DIR/issue-101867.rs:+1:12: +1:22
StorageLive(_5); // scope 1 at $DIR/issue-101867.rs:+2:14: +2:15
FakeRead(ForMatchedPlace(None), _1); // scope 1 at $DIR/issue-101867.rs:+2:19: +2:20
_6 = discriminant(_1); // scope 1 at $DIR/issue-101867.rs:+2:19: +2:20
switchInt(move _6) -> [1_isize: bb4, otherwise: bb3]; // scope 1 at $DIR/issue-101867.rs:+2:9: +2:16
}

bb1: {
StorageLive(_3); // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL
StorageLive(_4); // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL
_4 = begin_panic::<&str>(const "explicit panic") -> bb7; // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/std/src/panic.rs:LL:COL
// + literal: Const { ty: fn(&str) -> ! {begin_panic::<&str>}, val: Value(<ZST>) }
// mir::Constant
// + span: $SRC_DIR/std/src/panic.rs:LL:COL
// + literal: Const { ty: &str, val: Value(Slice(..)) }
}

bb2: {
StorageDead(_4); // scope 1 at $SRC_DIR/std/src/panic.rs:LL:COL
StorageDead(_3); // scope 1 at $DIR/issue-101867.rs:+3:16: +3:17
unreachable; // scope 1 at $DIR/issue-101867.rs:+2:26: +4:6
}

bb3: {
goto -> bb6; // scope 1 at $DIR/issue-101867.rs:+2:19: +2:20
}

bb4: {
falseEdge -> [real: bb5, imaginary: bb3]; // scope 1 at $DIR/issue-101867.rs:+2:9: +2:16
}

bb5: {
_5 = ((_1 as Some).0: u8); // scope 1 at $DIR/issue-101867.rs:+2:14: +2:15
_0 = const (); // scope 0 at $DIR/issue-101867.rs:+0:11: +5:2
StorageDead(_5); // scope 1 at $DIR/issue-101867.rs:+5:1: +5:2
StorageDead(_1); // scope 0 at $DIR/issue-101867.rs:+5:1: +5:2
return; // scope 0 at $DIR/issue-101867.rs:+5:2: +5:2
}

bb6: {
StorageDead(_5); // scope 1 at $DIR/issue-101867.rs:+5:1: +5:2
goto -> bb1; // scope 0 at $DIR/issue-101867.rs:+0:11: +5:2
}

bb7 (cleanup): {
resume; // scope 0 at $DIR/issue-101867.rs:+0:1: +5:2
}
}
19 changes: 19 additions & 0 deletions src/test/ui/let-else/const-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// run-pass
// issue #101932

#![cfg_attr(bootstrap, feature(let_else))]

const fn foo(a: Option<i32>) -> i32 {
let Some(a) = a else {
return 42
};

a + 1
}

fn main() {
const A: i32 = foo(None);
const B: i32 = foo(Some(1));

println!("{} {}", A, B);
}