Address review feedback

Author: nbdd0121

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -147,7 +147,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
     }
 
     /// Call `fn_ptr` of `fn_abi` with the arguments `llargs`, the optional
-    /// return destination `destination` and the cleanup function `cleanup`.
+    /// return destination `destination` and the unwind action `unwind`.
     fn do_call<Bx: BuilderMethods<'a, 'tcx>>(
         &self,
         fx: &mut FunctionCx<'a, 'tcx, Bx>,
@@ -234,7 +234,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
         }
     }
 
-    /// Generates inline assembly with optional `destination` and `cleanup`.
+    /// Generates inline assembly with optional `destination` and `unwind`.
     fn do_inlineasm<Bx: BuilderMethods<'a, 'tcx>>(
         &self,
         fx: &mut FunctionCx<'a, 'tcx, Bx>,

compiler/rustc_const_eval/src/transform/validate.rs

@@ -232,6 +232,24 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
         }
     }
 
+    fn check_unwind_edge(&mut self, location: Location, unwind: UnwindAction) {
+        let is_cleanup = self.body.basic_blocks[location.block].is_cleanup;
+        match unwind {
+            UnwindAction::Cleanup(unwind) => {
+                if is_cleanup {
+                    self.fail(location, "unwind on cleanup block");
+                }
+                self.check_edge(location, unwind, EdgeKind::Unwind);
+            }
+            UnwindAction::Continue => {
+                if is_cleanup {
+                    self.fail(location, "unwind on cleanup block");
+                }
+            }
+            UnwindAction::Unreachable | UnwindAction::Terminate => (),
+        }
+    }
+
     /// Check if src can be assigned into dest.
     /// This is not precise, it will accept some incorrect assignments.
     fn mir_assign_valid_types(&self, src: Ty<'tcx>, dest: Ty<'tcx>) -> bool {
@@ -902,9 +920,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
             }
             TerminatorKind::Drop { target, unwind, .. } => {
                 self.check_edge(location, *target, EdgeKind::Normal);
-                if let UnwindAction::Cleanup(unwind) = unwind {
-                    self.check_edge(location, *unwind, EdgeKind::Unwind);
-                }
+                self.check_unwind_edge(location, *unwind);
             }
             TerminatorKind::Call { func, args, destination, target, unwind, .. } => {
                 let func_ty = func.ty(&self.body.local_decls, self.tcx);
@@ -918,9 +934,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 if let Some(target) = target {
                     self.check_edge(location, *target, EdgeKind::Normal);
                 }
-                if let UnwindAction::Cleanup(cleanup) = unwind {
-                    self.check_edge(location, *cleanup, EdgeKind::Unwind);
-                }
+                self.check_unwind_edge(location, *unwind);
 
                 // The call destination place and Operand::Move place used as an argument might be
                 // passed by a reference to the callee. Consequently they must be non-overlapping.
@@ -958,9 +972,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     );
                 }
                 self.check_edge(location, *target, EdgeKind::Normal);
-                if let UnwindAction::Cleanup(cleanup) = unwind {
-                    self.check_edge(location, *cleanup, EdgeKind::Unwind);
-                }
+                self.check_unwind_edge(location, *unwind);
             }
             TerminatorKind::Yield { resume, drop, .. } => {
                 if self.body.generator.is_none() {
@@ -992,17 +1004,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     );
                 }
                 self.check_edge(location, *real_target, EdgeKind::Normal);
-                if let UnwindAction::Cleanup(unwind) = unwind {
-                    self.check_edge(location, *unwind, EdgeKind::Unwind);
-                }
+                self.check_unwind_edge(location, *unwind);
             }
             TerminatorKind::InlineAsm { destination, unwind, .. } => {
                 if let Some(destination) = destination {
                     self.check_edge(location, *destination, EdgeKind::Normal);
                 }
-                if let UnwindAction::Cleanup(cleanup) = unwind {
-                    self.check_edge(location, *cleanup, EdgeKind::Unwind);
-                }
+                self.check_unwind_edge(location, *unwind);
             }
             TerminatorKind::GeneratorDrop => {
                 if self.body.generator.is_none() {

compiler/rustc_middle/src/mir/patch.rs

@@ -13,7 +13,7 @@ pub struct MirPatch<'tcx> {
     new_locals: Vec<LocalDecl<'tcx>>,
     resume_block: Option<BasicBlock>,
     // Only for unreachable in cleanup path.
-    unreachable_block: Option<BasicBlock>,
+    unreachable_cleanup_block: Option<BasicBlock>,
     terminate_block: Option<BasicBlock>,
     body_span: Span,
     next_local: usize,
@@ -28,7 +28,7 @@ impl<'tcx> MirPatch<'tcx> {
             new_locals: vec![],
             next_local: body.local_decls.len(),
             resume_block: None,
-            unreachable_block: None,
+            unreachable_cleanup_block: None,
             terminate_block: None,
             body_span: body.span,
         };
@@ -41,8 +41,11 @@ impl<'tcx> MirPatch<'tcx> {
             }
 
             // Check if we already have an unreachable block
-            if let TerminatorKind::Unreachable = block.terminator().kind && block.statements.is_empty() {
-                result.unreachable_block = Some(bb);
+            if let TerminatorKind::Unreachable = block.terminator().kind
+                && block.statements.is_empty()
+                && block.is_cleanup
+            {
+                result.unreachable_cleanup_block = Some(bb);
                 continue;
             }
 
@@ -73,8 +76,8 @@ impl<'tcx> MirPatch<'tcx> {
         bb
     }
 
-    pub fn unreachable_block(&mut self) -> BasicBlock {
-        if let Some(bb) = self.unreachable_block {
+    pub fn unreachable_cleanup_block(&mut self) -> BasicBlock {
+        if let Some(bb) = self.unreachable_cleanup_block {
             return bb;
         }
 
@@ -86,7 +89,7 @@ impl<'tcx> MirPatch<'tcx> {
             }),
             is_cleanup: true,
         });
-        self.unreachable_block = Some(bb);
+        self.unreachable_cleanup_block = Some(bb);
         bb
     }
 

compiler/rustc_middle/src/mir/syntax.rs

@@ -523,7 +523,7 @@ pub struct CopyNonOverlapping<'tcx> {
 /// The basic block pointed to by a `Cleanup` unwind action must have its `cleanup` flag set.
 /// `cleanup` basic blocks have a couple restrictions:
 ///  1. All `unwind` fields in them must be `UnwindAction::Terminate` or `UnwindAction::Unreachable`.
-///  2. `Return` terminators are not allowed in them. `Terminate` and `Unwind` terminators are.
+///  2. `Return` terminators are not allowed in them. `Terminate` and `Resume` terminators are.
 ///  3. All other basic blocks (in the current body) that are reachable from `cleanup` basic blocks
 ///     must also be `cleanup`. This is a part of the type system and checked statically, so it is
 ///     still an error to have such an edge in the CFG even if it's known that it won't be taken at
@@ -721,8 +721,6 @@ pub enum TerminatorKind<'tcx> {
         /// consider it in borrowck. We don't want to accept programs which
         /// pass borrowck only when `panic=abort` or some assertions are disabled
         /// due to release vs. debug mode builds.
-        /// This field does not necessary have to be `UnwindAction::Cleanup` because
-        /// of the `remove_noop_landing_pads` and `abort_unwinding_calls` passes.
         unwind: UnwindAction,
     },
 

compiler/rustc_mir_transform/src/elaborate_drops.rs

@@ -415,7 +415,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
                                     UnwindAction::Cleanup(cleanup) => Unwind::To(cleanup),
                                     UnwindAction::Continue => Unwind::To(self.patch.resume_block()),
                                     UnwindAction::Unreachable => {
-                                        Unwind::To(self.patch.unreachable_block())
+                                        Unwind::To(self.patch.unreachable_cleanup_block())
                                     }
                                     UnwindAction::Terminate => {
                                         Unwind::To(self.patch.terminate_block())

compiler/rustc_mir_transform/src/generator.rs

@@ -1063,7 +1063,7 @@ fn elaborate_generator_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
             Unwind::To(match *unwind {
                 UnwindAction::Cleanup(tgt) => tgt,
                 UnwindAction::Continue => elaborator.patch.resume_block(),
-                UnwindAction::Unreachable => elaborator.patch.unreachable_block(),
+                UnwindAction::Unreachable => elaborator.patch.unreachable_cleanup_block(),
                 UnwindAction::Terminate => elaborator.patch.terminate_block(),
             })
         };

compiler/rustc_mir_transform/src/shim.rs

@@ -543,7 +543,7 @@ impl<'tcx> CloneShimBuilder<'tcx> {
                 TerminatorKind::Drop {
                     place: dest_field,
                     target: unwind,
-                    unwind: UnwindAction::Continue,
+                    unwind: UnwindAction::Terminate,
                 },
                 true,
             );
@@ -814,7 +814,7 @@ fn build_call_shim<'tcx>(
             TerminatorKind::Drop {
                 place: rcvr_place(),
                 target: BasicBlock::new(4),
-                unwind: UnwindAction::Continue,
+                unwind: UnwindAction::Terminate,
             },
             true,
         );