Skip to content

Commit c97d02c

Browse files
committed
Auto merge of #102394 - dingxiangfei2009:issue-102317, r=oli-obk
Fix unwind drop glue for if-then scopes cc `@est31` Fix #102317 Fix #99852 This PR fixes the drop glue for unwinding from a panic originated in a drop while breaking out for the else block in an `if-then` scope. MIR validation does not fail for the synchronous versions of the test program, because `StorageDead` statements are skipped over in the unwinding process. It is only becoming a problem when it is inside a generator where `StorageDead` must be kept around.
2 parents 75ada3a + 565c35a commit c97d02c

File tree

6 files changed

+76
-19
lines changed

6 files changed

+76
-19
lines changed

compiler/rustc_mir_build/src/build/block.rs

-5
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
245245
OutsideGuard,
246246
true,
247247
);
248-
this.schedule_drop_for_binding(
249-
node,
250-
span,
251-
OutsideGuard,
252-
);
253248
},
254249
);
255250
this.ast_let_else(

compiler/rustc_mir_build/src/build/expr/into.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
7474
this.source_info(then_expr.span)
7575
};
7676
let (then_block, else_block) =
77-
this.in_if_then_scope(condition_scope, |this| {
77+
this.in_if_then_scope(condition_scope, then_expr.span, |this| {
7878
let then_blk = unpack!(this.then_else_break(
7979
block,
8080
&this.thir[cond],
@@ -107,7 +107,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
107107
}
108108
ExprKind::Let { expr, ref pat } => {
109109
let scope = this.local_scope();
110-
let (true_block, false_block) = this.in_if_then_scope(scope, |this| {
110+
let (true_block, false_block) = this.in_if_then_scope(scope, expr_span, |this| {
111111
this.lower_let_expr(block, &this.thir[expr], pat, scope, None, expr_span)
112112
});
113113

compiler/rustc_mir_build/src/build/matches/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1986,7 +1986,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
19861986
let mut guard_span = rustc_span::DUMMY_SP;
19871987

19881988
let (post_guard_block, otherwise_post_guard_block) =
1989-
self.in_if_then_scope(match_scope, |this| match *guard {
1989+
self.in_if_then_scope(match_scope, guard_span, |this| match *guard {
19901990
Guard::If(e) => {
19911991
let e = &this.thir[e];
19921992
guard_span = e.span;
@@ -2301,7 +2301,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
23012301
pattern: &Pat<'tcx>,
23022302
) -> BlockAnd<BasicBlock> {
23032303
let else_block_span = self.thir[else_block].span;
2304-
let (matching, failure) = self.in_if_then_scope(*let_else_scope, |this| {
2304+
let (matching, failure) = self.in_if_then_scope(*let_else_scope, else_block_span, |this| {
23052305
let scrutinee = unpack!(block = this.lower_scrutinee(block, init, initializer_span));
23062306
let pat = Pat { ty: init.ty, span: else_block_span, kind: PatKind::Wild };
23072307
let mut wildcard = Candidate::new(scrutinee.clone(), &pat, false, this);

compiler/rustc_mir_build/src/build/scope.rs

+28-10
Original file line numberDiff line numberDiff line change
@@ -466,9 +466,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
466466
let normal_exit_block = f(self);
467467
let breakable_scope = self.scopes.breakable_scopes.pop().unwrap();
468468
assert!(breakable_scope.region_scope == region_scope);
469-
let break_block = self.build_exit_tree(breakable_scope.break_drops, None);
469+
let break_block =
470+
self.build_exit_tree(breakable_scope.break_drops, region_scope, span, None);
470471
if let Some(drops) = breakable_scope.continue_drops {
471-
self.build_exit_tree(drops, loop_block);
472+
self.build_exit_tree(drops, region_scope, span, loop_block);
472473
}
473474
match (normal_exit_block, break_block) {
474475
(Some(block), None) | (None, Some(block)) => block,
@@ -510,6 +511,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
510511
pub(crate) fn in_if_then_scope<F>(
511512
&mut self,
512513
region_scope: region::Scope,
514+
span: Span,
513515
f: F,
514516
) -> (BasicBlock, BasicBlock)
515517
where
@@ -524,7 +526,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
524526
assert!(if_then_scope.region_scope == region_scope);
525527

526528
let else_block = self
527-
.build_exit_tree(if_then_scope.else_drops, None)
529+
.build_exit_tree(if_then_scope.else_drops, region_scope, span, None)
528530
.map_or_else(|| self.cfg.start_new_block(), |else_block_and| unpack!(else_block_and));
529531

530532
(then_block, else_block)
@@ -997,10 +999,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
997999
/// Returns the [DropIdx] for the innermost drop if the function unwound at
9981000
/// this point. The `DropIdx` will be created if it doesn't already exist.
9991001
fn diverge_cleanup(&mut self) -> DropIdx {
1000-
let is_generator = self.generator_kind.is_some();
1001-
let (uncached_scope, mut cached_drop) = self
1002-
.scopes
1003-
.scopes
1002+
// It is okay to use dummy span because the getting scope index on the topmost scope
1003+
// must always succeed.
1004+
self.diverge_cleanup_target(self.scopes.topmost(), DUMMY_SP)
1005+
}
1006+
1007+
/// This is similar to [diverge_cleanup](Self::diverge_cleanup) except its target is set to
1008+
/// some ancestor scope instead of the current scope.
1009+
/// It is possible to unwind to some ancestor scope if some drop panics as
1010+
/// the program breaks out of a if-then scope.
1011+
fn diverge_cleanup_target(&mut self, target_scope: region::Scope, span: Span) -> DropIdx {
1012+
let target = self.scopes.scope_index(target_scope, span);
1013+
let (uncached_scope, mut cached_drop) = self.scopes.scopes[..=target]
10041014
.iter()
10051015
.enumerate()
10061016
.rev()
@@ -1009,7 +1019,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
10091019
})
10101020
.unwrap_or((0, ROOT_NODE));
10111021

1012-
for scope in &mut self.scopes.scopes[uncached_scope..] {
1022+
if uncached_scope > target {
1023+
return cached_drop;
1024+
}
1025+
1026+
let is_generator = self.generator_kind.is_some();
1027+
for scope in &mut self.scopes.scopes[uncached_scope..=target] {
10131028
for drop in &scope.drops {
10141029
if is_generator || drop.kind == DropKind::Value {
10151030
cached_drop = self.scopes.unwind_drops.add_drop(*drop, cached_drop);
@@ -1222,21 +1237,24 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> {
12221237
fn build_exit_tree(
12231238
&mut self,
12241239
mut drops: DropTree,
1240+
else_scope: region::Scope,
1241+
span: Span,
12251242
continue_block: Option<BasicBlock>,
12261243
) -> Option<BlockAnd<()>> {
12271244
let mut blocks = IndexVec::from_elem(None, &drops.drops);
12281245
blocks[ROOT_NODE] = continue_block;
12291246

12301247
drops.build_mir::<ExitScopes>(&mut self.cfg, &mut blocks);
1248+
let is_generator = self.generator_kind.is_some();
12311249

12321250
// Link the exit drop tree to unwind drop tree.
12331251
if drops.drops.iter().any(|(drop, _)| drop.kind == DropKind::Value) {
1234-
let unwind_target = self.diverge_cleanup();
1252+
let unwind_target = self.diverge_cleanup_target(else_scope, span);
12351253
let mut unwind_indices = IndexVec::from_elem_n(unwind_target, 1);
12361254
for (drop_idx, drop_data) in drops.drops.iter_enumerated().skip(1) {
12371255
match drop_data.0.kind {
12381256
DropKind::Storage => {
1239-
if self.generator_kind.is_some() {
1257+
if is_generator {
12401258
let unwind_drop = self
12411259
.scopes
12421260
.unwind_drops

src/test/ui/let-else/issue-102317.rs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// issue #102317
2+
// build-pass
3+
// compile-flags: --edition 2021 -C opt-level=3 -Zvalidate-mir
4+
5+
struct SegmentJob;
6+
7+
impl Drop for SegmentJob {
8+
fn drop(&mut self) {}
9+
}
10+
11+
pub async fn run() -> Result<(), ()> {
12+
let jobs = Vec::<SegmentJob>::new();
13+
let Some(_job) = jobs.into_iter().next() else {
14+
return Ok(())
15+
};
16+
17+
Ok(())
18+
}
19+
20+
fn main() {}

src/test/ui/mir/issue-99852.rs

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// check-pass
2+
// compile-flags: -Z validate-mir
3+
#![feature(let_chains)]
4+
5+
fn lambda<T, U>() -> U
6+
where
7+
T: Default,
8+
U: Default,
9+
{
10+
let foo: Result<T, ()> = Ok(T::default());
11+
let baz: U = U::default();
12+
13+
if let Ok(foo) = foo && let Ok(bar) = transform(foo) {
14+
bar
15+
} else {
16+
baz
17+
}
18+
}
19+
20+
fn transform<T, U>(input: T) -> Result<U, ()> {
21+
todo!()
22+
}
23+
24+
fn main() {}

0 commit comments

Comments
 (0)