From 4a2c1a12b662eb590dbe78f7f9d13c2f327d3bb6 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Wed, 28 Sep 2022 16:45:09 +0800 Subject: [PATCH 1/4] fix unwind drop glue for if-then scopes --- compiler/rustc_mir_build/src/build/block.rs | 5 -- .../rustc_mir_build/src/build/expr/into.rs | 4 +- .../rustc_mir_build/src/build/matches/mod.rs | 4 +- compiler/rustc_mir_build/src/build/scope.rs | 47 +++++++++++++++++-- src/test/ui/let-else/issue-102317.rs | 20 ++++++++ src/test/ui/mir/issue-99852.rs | 24 ++++++++++ 6 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/let-else/issue-102317.rs create mode 100644 src/test/ui/mir/issue-99852.rs diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index fc1b301402b73..183db56d7a08c 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -245,11 +245,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { OutsideGuard, true, ); - this.schedule_drop_for_binding( - node, - span, - OutsideGuard, - ); }, ); this.ast_let_else( diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 4b8c134b9d0a8..24ecd0a539949 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -74,7 +74,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.source_info(then_expr.span) }; let (then_block, else_block) = - this.in_if_then_scope(condition_scope, |this| { + this.in_if_then_scope(condition_scope, then_expr.span, |this| { let then_blk = unpack!(this.then_else_break( block, &this.thir[cond], @@ -107,7 +107,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } ExprKind::Let { expr, ref pat } => { let scope = this.local_scope(); - let (true_block, false_block) = this.in_if_then_scope(scope, |this| { + let (true_block, false_block) = this.in_if_then_scope(scope, expr_span, |this| { this.lower_let_expr(block, &this.thir[expr], pat, scope, None, expr_span) }); diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 4fddc24301aba..3f813e0af0da3 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1986,7 +1986,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let mut guard_span = rustc_span::DUMMY_SP; let (post_guard_block, otherwise_post_guard_block) = - self.in_if_then_scope(match_scope, |this| match *guard { + self.in_if_then_scope(match_scope, guard_span, |this| match *guard { Guard::If(e) => { let e = &this.thir[e]; guard_span = e.span; @@ -2301,7 +2301,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pattern: &Pat<'tcx>, ) -> BlockAnd { let else_block_span = self.thir[else_block].span; - let (matching, failure) = self.in_if_then_scope(*let_else_scope, |this| { + let (matching, failure) = self.in_if_then_scope(*let_else_scope, else_block_span, |this| { let scrutinee = unpack!(block = this.lower_scrutinee(block, init, initializer_span)); let pat = Pat { ty: init.ty, span: else_block_span, kind: PatKind::Wild }; let mut wildcard = Candidate::new(scrutinee.clone(), &pat, false, this); diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 2c7d6a572f45b..2d29d0b6928b6 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -466,9 +466,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let normal_exit_block = f(self); let breakable_scope = self.scopes.breakable_scopes.pop().unwrap(); assert!(breakable_scope.region_scope == region_scope); - let break_block = self.build_exit_tree(breakable_scope.break_drops, None); + let break_block = + self.build_exit_tree(breakable_scope.break_drops, region_scope, span, None); if let Some(drops) = breakable_scope.continue_drops { - self.build_exit_tree(drops, loop_block); + self.build_exit_tree(drops, region_scope, span, loop_block); } match (normal_exit_block, break_block) { (Some(block), None) | (None, Some(block)) => block, @@ -510,6 +511,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn in_if_then_scope( &mut self, region_scope: region::Scope, + span: Span, f: F, ) -> (BasicBlock, BasicBlock) where @@ -524,7 +526,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { assert!(if_then_scope.region_scope == region_scope); let else_block = self - .build_exit_tree(if_then_scope.else_drops, None) + .build_exit_tree(if_then_scope.else_drops, region_scope, span, None) .map_or_else(|| self.cfg.start_new_block(), |else_block_and| unpack!(else_block_and)); (then_block, else_block) @@ -1021,6 +1023,38 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { cached_drop } + /// This is similar to [diverge_cleanup_target] except its target is set to + /// some ancestor scope instead of the current scope. + /// It is possible to unwind to some ancestor scope if some drop panics as + /// the program breaks out of a if-then scope. + fn diverge_cleanup_target(&mut self, target_scope: region::Scope, span: Span) -> DropIdx { + let target = self.scopes.scope_index(target_scope, span); + let (uncached_scope, mut cached_drop) = self.scopes.scopes[..=target] + .iter() + .enumerate() + .rev() + .find_map(|(scope_idx, scope)| { + scope.cached_unwind_block.map(|cached_block| (scope_idx + 1, cached_block)) + }) + .unwrap_or((0, ROOT_NODE)); + + if uncached_scope > target { + return cached_drop; + } + + let is_generator = self.generator_kind.is_some(); + for scope in &mut self.scopes.scopes[uncached_scope..=target] { + for drop in &scope.drops { + if is_generator || drop.kind == DropKind::Value { + cached_drop = self.scopes.unwind_drops.add_drop(*drop, cached_drop); + } + } + scope.cached_unwind_block = Some(cached_drop); + } + + cached_drop + } + /// Prepares to create a path that performs all required cleanup for a /// terminator that can unwind at the given basic block. /// @@ -1222,21 +1256,24 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { fn build_exit_tree( &mut self, mut drops: DropTree, + else_scope: region::Scope, + span: Span, continue_block: Option, ) -> Option> { let mut blocks = IndexVec::from_elem(None, &drops.drops); blocks[ROOT_NODE] = continue_block; drops.build_mir::(&mut self.cfg, &mut blocks); + let is_generator = self.generator_kind.is_some(); // Link the exit drop tree to unwind drop tree. if drops.drops.iter().any(|(drop, _)| drop.kind == DropKind::Value) { - let unwind_target = self.diverge_cleanup(); + let unwind_target = self.diverge_cleanup_target(else_scope, span); let mut unwind_indices = IndexVec::from_elem_n(unwind_target, 1); for (drop_idx, drop_data) in drops.drops.iter_enumerated().skip(1) { match drop_data.0.kind { DropKind::Storage => { - if self.generator_kind.is_some() { + if is_generator { let unwind_drop = self .scopes .unwind_drops diff --git a/src/test/ui/let-else/issue-102317.rs b/src/test/ui/let-else/issue-102317.rs new file mode 100644 index 0000000000000..7dfaf1358ae7d --- /dev/null +++ b/src/test/ui/let-else/issue-102317.rs @@ -0,0 +1,20 @@ +// issue #102317 +// run-pass +// compile-flags: --edition 2021 -C opt-level=3 -Zvalidate-mir + +struct SegmentJob; + +impl Drop for SegmentJob { + fn drop(&mut self) {} +} + +pub async fn run() -> Result<(), ()> { + let jobs = Vec::::new(); + let Some(_job) = jobs.into_iter().next() else { + return Ok(()) + }; + + Ok(()) +} + +fn main() {} diff --git a/src/test/ui/mir/issue-99852.rs b/src/test/ui/mir/issue-99852.rs new file mode 100644 index 0000000000000..1c675788ee933 --- /dev/null +++ b/src/test/ui/mir/issue-99852.rs @@ -0,0 +1,24 @@ +// check-pass +// compile-flags: -Z validate-mir +#![feature(let_chains)] + +fn lambda() -> U +where + T: Default, + U: Default, +{ + let foo: Result = Ok(T::default()); + let baz: U = U::default(); + + if let Ok(foo) = foo && let Ok(bar) = transform(foo) { + bar + } else { + baz + } +} + +fn transform(input: T) -> Result { + todo!() +} + +fn main() {} From fb52dc7c3b7cc5170d5096931827a0169fd65eb5 Mon Sep 17 00:00:00 2001 From: X <6884440+dingxiangfei2009@users.noreply.github.com> Date: Fri, 30 Sep 2022 21:03:08 +0800 Subject: [PATCH 2/4] apply suggestion Co-authored-by: SafariMonkey --- compiler/rustc_mir_build/src/build/scope.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 2d29d0b6928b6..edace55f572d0 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1023,7 +1023,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { cached_drop } - /// This is similar to [diverge_cleanup_target] except its target is set to + /// This is similar to [diverge_cleanup] except its target is set to /// some ancestor scope instead of the current scope. /// It is possible to unwind to some ancestor scope if some drop panics as /// the program breaks out of a if-then scope. From 5131e9db07fb6cd158cf5bf2aa9fefbad3af1d2b Mon Sep 17 00:00:00 2001 From: X <6884440+dingxiangfei2009@users.noreply.github.com> Date: Fri, 30 Sep 2022 21:04:03 +0800 Subject: [PATCH 3/4] use build-pass for the test --- src/test/ui/let-else/issue-102317.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/let-else/issue-102317.rs b/src/test/ui/let-else/issue-102317.rs index 7dfaf1358ae7d..7369b4938eed4 100644 --- a/src/test/ui/let-else/issue-102317.rs +++ b/src/test/ui/let-else/issue-102317.rs @@ -1,5 +1,5 @@ // issue #102317 -// run-pass +// build-pass // compile-flags: --edition 2021 -C opt-level=3 -Zvalidate-mir struct SegmentJob; From 565c35aa5c3c39626fcd332bafbd8936b70ed989 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Wed, 5 Oct 2022 22:24:12 +0800 Subject: [PATCH 4/4] fix doc and dedup diverge_cleanup --- compiler/rustc_mir_build/src/build/scope.rs | 27 +++------------------ 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index edace55f572d0..3cebd5ebed660 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -999,31 +999,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// Returns the [DropIdx] for the innermost drop if the function unwound at /// this point. The `DropIdx` will be created if it doesn't already exist. fn diverge_cleanup(&mut self) -> DropIdx { - let is_generator = self.generator_kind.is_some(); - let (uncached_scope, mut cached_drop) = self - .scopes - .scopes - .iter() - .enumerate() - .rev() - .find_map(|(scope_idx, scope)| { - scope.cached_unwind_block.map(|cached_block| (scope_idx + 1, cached_block)) - }) - .unwrap_or((0, ROOT_NODE)); - - for scope in &mut self.scopes.scopes[uncached_scope..] { - for drop in &scope.drops { - if is_generator || drop.kind == DropKind::Value { - cached_drop = self.scopes.unwind_drops.add_drop(*drop, cached_drop); - } - } - scope.cached_unwind_block = Some(cached_drop); - } - - cached_drop + // It is okay to use dummy span because the getting scope index on the topmost scope + // must always succeed. + self.diverge_cleanup_target(self.scopes.topmost(), DUMMY_SP) } - /// This is similar to [diverge_cleanup] except its target is set to + /// This is similar to [diverge_cleanup](Self::diverge_cleanup) except its target is set to /// some ancestor scope instead of the current scope. /// It is possible to unwind to some ancestor scope if some drop panics as /// the program breaks out of a if-then scope.