Defer creating drop trees in MIR lowering until leaving that scope

Author: matthewjasper

src/librustc_mir/util/graphviz.rs

@@ -97,12 +97,17 @@ where
     write!(w, r#"<table border="0" cellborder="1" cellspacing="0">"#)?;
 
     // Basic block number at the top.
+    let (blk, color) = if data.is_cleanup {
+        (format!("{} (cleanup)", block.index()), "lightblue")
+    } else {
+        (format!("{}", block.index()), "gray")
+    };
     write!(
         w,
-        r#"<tr><td {attrs} colspan="{colspan}">{blk}</td></tr>"#,
-        attrs = r#"bgcolor="gray" align="center""#,
+        r#"<tr><td bgcolor="{color}" align="center" colspan="{colspan}">{blk}</td></tr>"#,
         colspan = num_cols,
-        blk = block.index()
+        blk = blk,
+        color = color
     )?;
 
     init(w)?;

src/librustc_mir_build/build/block.rs

@@ -26,14 +26,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         self.in_opt_scope(opt_destruction_scope.map(|de| (de, source_info)), move |this| {
             this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
                 if targeted_by_break {
-                    // This is a `break`-able block
-                    let exit_block = this.cfg.start_new_block();
-                    let block_exit =
-                        this.in_breakable_scope(None, exit_block, destination, |this| {
-                            this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
-                        });
-                    this.cfg.goto(unpack!(block_exit), source_info, exit_block);
-                    exit_block.unit()
+                    this.in_breakable_scope(None, destination, span, |this| {
+                        Some(this.ast_block_stmts(
+                            destination,
+                            block,
+                            span,
+                            stmts,
+                            expr,
+                            safety_mode,
+                        ))
+                    })
                 } else {
                     this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
                 }

src/librustc_mir_build/build/expr/into.rs

@@ -134,32 +134,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 // body, even when the exact code in the body cannot unwind
 
                 let loop_block = this.cfg.start_new_block();
-                let exit_block = this.cfg.start_new_block();
 
                 // Start the loop.
                 this.cfg.goto(block, source_info, loop_block);
 
-                this.in_breakable_scope(Some(loop_block), exit_block, destination, move |this| {
+                this.in_breakable_scope(Some(loop_block), destination, expr_span, move |this| {
                     // conduct the test, if necessary
                     let body_block = this.cfg.start_new_block();
-                    let diverge_cleanup = this.diverge_cleanup();
                     this.cfg.terminate(
                         loop_block,
                         source_info,
-                        TerminatorKind::FalseUnwind {
-                            real_target: body_block,
-                            unwind: Some(diverge_cleanup),
-                        },
+                        TerminatorKind::FalseUnwind { real_target: body_block, unwind: None },
                     );
+                    this.diverge_from(loop_block);
 
                     // The “return” value of the loop body must always be an unit. We therefore
                     // introduce a unit temporary as the destination for the loop body.
                     let tmp = this.get_unit_temp();
                     // Execute the body, branching back to the test.
                     let body_block_end = unpack!(this.into(tmp, body_block, body));
                     this.cfg.goto(body_block_end, source_info, loop_block);
-                });
-                exit_block.unit()
+
+                    // Loops are only exited by `break` expressions.
+                    None
+                })
             }
             ExprKind::Call { ty, fun, args, from_hir_call } => {
                 let intrinsic = match ty.kind {
@@ -201,7 +199,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         .collect();
 
                     let success = this.cfg.start_new_block();
-                    let cleanup = this.diverge_cleanup();
 
                     this.record_operands_moved(&args);
 
@@ -211,7 +208,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         TerminatorKind::Call {
                             func: fun,
                             args,
-                            cleanup: Some(cleanup),
+                            cleanup: None,
                             // FIXME(varkor): replace this with an uninhabitedness-based check.
                             // This requires getting access to the current module to call
                             // `tcx.is_ty_uninhabited_from`, which is currently tricky to do.
@@ -223,6 +220,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                             from_hir_call,
                         },
                     );
+                    this.diverge_from(block);
                     success.unit()
                 }
             }
@@ -358,12 +356,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let scope = this.local_scope();
                 let value = unpack!(block = this.as_operand(block, scope, value));
                 let resume = this.cfg.start_new_block();
-                let cleanup = this.generator_drop_cleanup();
                 this.cfg.terminate(
                     block,
                     source_info,
-                    TerminatorKind::Yield { value, resume, resume_arg: destination, drop: cleanup },
+                    TerminatorKind::Yield { value, resume, resume_arg: destination, drop: None },
                 );
+                this.generator_drop_cleanup(block);
                 resume.unit()
             }
 

src/librustc_mir_build/build/matches/mod.rs

@@ -225,8 +225,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         outer_source_info: SourceInfo,
         fake_borrow_temps: Vec<(Place<'tcx>, Local)>,
     ) -> BlockAnd<()> {
-        let match_scope = self.scopes.topmost();
-
         let arm_end_blocks: Vec<_> = arm_candidates
             .into_iter()
             .map(|(arm, candidate)| {
@@ -247,7 +245,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     let arm_block = this.bind_pattern(
                         outer_source_info,
                         candidate,
-                        arm.guard.as_ref().map(|g| (g, match_scope)),
+                        arm.guard.as_ref(),
                         &fake_borrow_temps,
                         scrutinee_span,
                         Some(arm.scope),
@@ -284,7 +282,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         &mut self,
         outer_source_info: SourceInfo,
         candidate: Candidate<'_, 'tcx>,
-        guard: Option<(&Guard<'tcx>, region::Scope)>,
+        guard: Option<&Guard<'tcx>>,
         fake_borrow_temps: &Vec<(Place<'tcx>, Local)>,
         scrutinee_span: Span,
         arm_scope: Option<region::Scope>,
@@ -1563,7 +1561,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         &mut self,
         candidate: Candidate<'pat, 'tcx>,
         parent_bindings: &[(Vec<Binding<'tcx>>, Vec<Ascription<'tcx>>)],
-        guard: Option<(&Guard<'tcx>, region::Scope)>,
+        guard: Option<&Guard<'tcx>>,
         fake_borrows: &Vec<(Place<'tcx>, Local)>,
         scrutinee_span: Span,
         schedule_drops: bool,
@@ -1675,7 +1673,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         //      the reference that we create for the arm.
         //    * So we eagerly create the reference for the arm and then take a
         //      reference to that.
-        if let Some((guard, region_scope)) = guard {
+        if let Some(guard) = guard {
             let tcx = self.hir.tcx();
             let bindings = parent_bindings
                 .iter()
@@ -1719,12 +1717,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 unreachable
             });
             let outside_scope = self.cfg.start_new_block();
-            self.exit_scope(
-                source_info.span,
-                region_scope,
-                otherwise_post_guard_block,
-                outside_scope,
-            );
+            self.exit_top_scope(otherwise_post_guard_block, outside_scope, source_info);
             self.false_edges(
                 outside_scope,
                 otherwise_block,

src/librustc_mir_build/build/matches/test.rs

@@ -423,7 +423,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let bool_ty = self.hir.bool_ty();
         let eq_result = self.temp(bool_ty, source_info.span);
         let eq_block = self.cfg.start_new_block();
-        let cleanup = self.diverge_cleanup();
         self.cfg.terminate(
             block,
             source_info,
@@ -440,11 +439,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     literal: method,
                 }),
                 args: vec![val, expect],
-                destination: Some((eq_result, eq_block)),
-                cleanup: Some(cleanup),
+                destination: Some((eq_result.clone(), eq_block)),
+                cleanup: None,
                 from_hir_call: false,
             },
         );
+        self.diverge_from(block);
 
         if let [success_block, fail_block] = *make_target_blocks(self) {
             // check the result

src/librustc_mir_build/build/mod.rs

@@ -327,11 +327,6 @@ struct Builder<'a, 'tcx> {
 
     var_debug_info: Vec<VarDebugInfo<'tcx>>,
 
-    /// Cached block with the `RESUME` terminator; this is created
-    /// when first set of cleanups are built.
-    cached_resume_block: Option<BasicBlock>,
-    /// Cached block with the `RETURN` terminator.
-    cached_return_block: Option<BasicBlock>,
     /// Cached block with the `UNREACHABLE` terminator.
     cached_unreachable_block: Option<BasicBlock>,
 }
@@ -590,50 +585,34 @@ where
         region::Scope { id: body.value.hir_id.local_id, data: region::ScopeData::CallSite };
     let arg_scope =
         region::Scope { id: body.value.hir_id.local_id, data: region::ScopeData::Arguments };
-    let mut block = START_BLOCK;
     let source_info = builder.source_info(span);
     let call_site_s = (call_site_scope, source_info);
-    unpack!(
-        block = builder.in_scope(call_site_s, LintLevel::Inherited, |builder| {
-            if should_abort_on_panic(tcx, fn_def_id, abi) {
-                builder.schedule_abort();
-            }
-
-            let arg_scope_s = (arg_scope, source_info);
-            // `return_block` is called when we evaluate a `return` expression, so
-            // we just use `START_BLOCK` here.
-            unpack!(
-                block = builder.in_breakable_scope(
-                    None,
-                    START_BLOCK,
-                    Place::return_place(),
-                    |builder| {
-                        builder.in_scope(arg_scope_s, LintLevel::Inherited, |builder| {
-                            builder.args_and_body(
-                                block,
-                                fn_def_id.to_def_id(),
-                                &arguments,
-                                arg_scope,
-                                &body.value,
-                            )
-                        })
-                    },
-                )
-            );
-            // Attribute epilogue to function's closing brace
-            let fn_end = span.shrink_to_hi();
-            let source_info = builder.source_info(fn_end);
-            let return_block = builder.return_block();
-            builder.cfg.goto(block, source_info, return_block);
-            builder.cfg.terminate(return_block, source_info, TerminatorKind::Return);
-            // Attribute any unreachable codepaths to the function's closing brace
-            if let Some(unreachable_block) = builder.cached_unreachable_block {
-                builder.cfg.terminate(unreachable_block, source_info, TerminatorKind::Unreachable);
-            }
-            return_block.unit()
-        })
-    );
-    assert_eq!(block, builder.return_block());
+    unpack!(builder.in_scope(call_site_s, LintLevel::Inherited, |builder| {
+        let arg_scope_s = (arg_scope, source_info);
+        // Attribute epilogue to function's closing brace
+        let fn_end = span.shrink_to_hi();
+        let return_block =
+            unpack!(builder.in_breakable_scope(None, Place::return_place(), fn_end, |builder| {
+                Some(builder.in_scope(arg_scope_s, LintLevel::Inherited, |builder| {
+                    builder.args_and_body(
+                        START_BLOCK,
+                        fn_def_id.to_def_id(),
+                        &arguments,
+                        arg_scope,
+                        &body.value,
+                    )
+                }))
+            }));
+        let source_info = builder.source_info(fn_end);
+        builder.cfg.terminate(return_block, source_info, TerminatorKind::Return);
+        let should_abort = should_abort_on_panic(tcx, fn_def_id, abi);
+        builder.build_drop_trees(should_abort);
+        // Attribute any unreachable codepaths to the function's closing brace
+        if let Some(unreachable_block) = builder.cached_unreachable_block {
+            builder.cfg.terminate(unreachable_block, source_info, TerminatorKind::Unreachable);
+        }
+        return_block.unit()
+    }));
 
     let spread_arg = if abi == Abi::RustCall {
         // RustCall pseudo-ABI untuples the last argument.
@@ -667,8 +646,7 @@ fn construct_const<'a, 'tcx>(
     let source_info = builder.source_info(span);
     builder.cfg.terminate(block, source_info, TerminatorKind::Return);
 
-    // Constants can't `return` so a return block should not be created.
-    assert_eq!(builder.cached_return_block, None);
+    builder.build_drop_trees(false);
 
     // Constants may be match expressions in which case an unreachable block may
     // be created, so terminate it properly.
@@ -735,7 +713,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             fn_span: span,
             arg_count,
             generator_kind,
-            scopes: Default::default(),
+            scopes: scope::Scopes::new(),
             block_context: BlockContext::new(),
             source_scopes: IndexVec::new(),
             source_scope: OUTERMOST_SOURCE_SCOPE,
@@ -748,8 +726,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             var_indices: Default::default(),
             unit_temp: None,
             var_debug_info: vec![],
-            cached_resume_block: None,
-            cached_return_block: None,
             cached_unreachable_block: None,
         };
 
@@ -981,17 +957,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
         }
     }
-
-    fn return_block(&mut self) -> BasicBlock {
-        match self.cached_return_block {
-            Some(rb) => rb,
-            None => {
-                let rb = self.cfg.start_new_block();
-                self.cached_return_block = Some(rb);
-                rb
-            }
-        }
-    }
 }
 
 ///////////////////////////////////////////////////////////////////////////