Auto merge of #30481 - nagisa:mir-calls-2, r=nikomatsakis

r? @nikomatsakis

This is a pretty big PR conflating changes to a few different block terminators (Call, DivergingCall, Panic, Resume, Diverge), because they are somewhat closely related.

Each commit has a pretty good description on what is being changed in each commit. The end result is greatly simplified CFG and translation for calls (no success branch if the function is diverging, no cleanup branch if there’s nothing to cleanup etc).

Fixes #30480
Fixes #29767
Partialy solves #29575
Fixes #29573

Author: bors

Diff for: src/librustc/mir/repr.rs

@@ -51,9 +51,6 @@ pub const START_BLOCK: BasicBlock = BasicBlock(0);
 /// where execution ends, on normal return
 pub const END_BLOCK: BasicBlock = BasicBlock(1);
 
-/// where execution ends, on panic
-pub const DIVERGE_BLOCK: BasicBlock = BasicBlock(2);
-
 impl<'tcx> Mir<'tcx> {
     pub fn all_basic_blocks(&self) -> Vec<BasicBlock> {
         (0..self.basic_blocks.len())
@@ -194,7 +191,8 @@ impl Debug for BasicBlock {
 #[derive(Debug, RustcEncodable, RustcDecodable)]
 pub struct BasicBlockData<'tcx> {
     pub statements: Vec<Statement<'tcx>>,
-    pub terminator: Terminator<'tcx>,
+    pub terminator: Option<Terminator<'tcx>>,
+    pub is_cleanup: bool,
 }
 
 #[derive(RustcEncodable, RustcDecodable)]
@@ -204,12 +202,6 @@ pub enum Terminator<'tcx> {
         target: BasicBlock,
     },
 
-    /// block should initiate unwinding; should be one successor
-    /// that does cleanup and branches to DIVERGE_BLOCK
-    Panic {
-        target: BasicBlock,
-    },
-
     /// jump to branch 0 if this lvalue evaluates to true
     If {
         cond: Operand<'tcx>,
@@ -243,77 +235,125 @@ pub enum Terminator<'tcx> {
         targets: Vec<BasicBlock>,
     },
 
-    /// Indicates that the last statement in the block panics, aborts,
-    /// etc. No successors. This terminator appears on exactly one
-    /// basic block which we create in advance. However, during
-    /// construction, we use this value as a sentinel for "terminator
-    /// not yet assigned", and assert at the end that only the
-    /// well-known diverging block actually diverges.
-    Diverge,
+    /// Indicates that the landing pad is finished and unwinding should
+    /// continue. Emitted by build::scope::diverge_cleanup.
+    Resume,
 
     /// Indicates a normal return. The ReturnPointer lvalue should
     /// have been filled in by now. This should only occur in the
     /// `END_BLOCK`.
     Return,
 
-    /// block ends with a call; it should have two successors. The
-    /// first successor indicates normal return. The second indicates
-    /// unwinding.
+    /// Block ends with a call of a converging function
     Call {
-        data: CallData<'tcx>,
-        targets: (BasicBlock, BasicBlock),
+        /// The function that’s being called
+        func: Operand<'tcx>,
+        /// Arguments the function is called with
+        args: Vec<Operand<'tcx>>,
+        /// The kind of call with associated information
+        kind: CallKind<'tcx>,
     },
 }
 
+#[derive(Clone, RustcEncodable, RustcDecodable)]
+pub enum CallKind<'tcx> {
+    /// Diverging function without associated cleanup
+    Diverging,
+    /// Diverging function with associated cleanup
+    DivergingCleanup(BasicBlock),
+    /// Converging function without associated cleanup
+    Converging {
+        /// Destination where the call result is written
+        destination: Lvalue<'tcx>,
+        /// Block to branch into on successful return
+        target: BasicBlock,
+    },
+    ConvergingCleanup {
+        /// Destination where the call result is written
+        destination: Lvalue<'tcx>,
+        /// First target is branched to on successful return.
+        /// Second block contains the cleanups to do on unwind.
+        targets: (BasicBlock, BasicBlock)
+    }
+}
+
+impl<'tcx> CallKind<'tcx> {
+    pub fn successors(&self) -> &[BasicBlock] {
+        match *self {
+            CallKind::Diverging => &[],
+            CallKind::DivergingCleanup(ref b) |
+            CallKind::Converging { target: ref b, .. } => slice::ref_slice(b),
+            CallKind::ConvergingCleanup { ref targets, .. } => targets.as_slice(),
+        }
+    }
+
+    pub fn successors_mut(&mut self) -> &mut [BasicBlock] {
+        match *self {
+            CallKind::Diverging => &mut [],
+            CallKind::DivergingCleanup(ref mut b) |
+            CallKind::Converging { target: ref mut b, .. } => slice::mut_ref_slice(b),
+            CallKind::ConvergingCleanup { ref mut targets, .. } => targets.as_mut_slice(),
+        }
+    }
+
+    pub fn destination(&self) -> Option<Lvalue<'tcx>> {
+        match *self {
+            CallKind::Converging { ref destination, .. } |
+            CallKind::ConvergingCleanup { ref destination, .. } => Some(destination.clone()),
+            CallKind::Diverging |
+            CallKind::DivergingCleanup(_) => None
+        }
+    }
+}
+
 impl<'tcx> Terminator<'tcx> {
     pub fn successors(&self) -> &[BasicBlock] {
         use self::Terminator::*;
         match *self {
             Goto { target: ref b } => slice::ref_slice(b),
-            Panic { target: ref b } => slice::ref_slice(b),
-            If { cond: _, targets: ref b } => b.as_slice(),
+            If { targets: ref b, .. } => b.as_slice(),
             Switch { targets: ref b, .. } => b,
             SwitchInt { targets: ref b, .. } => b,
-            Diverge => &[],
+            Resume => &[],
             Return => &[],
-            Call { data: _, targets: ref b } => b.as_slice(),
+            Call { ref kind, .. } => kind.successors(),
         }
     }
 
     pub fn successors_mut(&mut self) -> &mut [BasicBlock] {
         use self::Terminator::*;
         match *self {
             Goto { target: ref mut b } => slice::mut_ref_slice(b),
-            Panic { target: ref mut b } => slice::mut_ref_slice(b),
-            If { cond: _, targets: ref mut b } => b.as_mut_slice(),
+            If { targets: ref mut b, .. } => b.as_mut_slice(),
             Switch { targets: ref mut b, .. } => b,
             SwitchInt { targets: ref mut b, .. } => b,
-            Diverge => &mut [],
+            Resume => &mut [],
             Return => &mut [],
-            Call { data: _, targets: ref mut b } => b.as_mut_slice(),
+            Call { ref mut kind, .. } => kind.successors_mut(),
         }
     }
 }
 
-#[derive(Debug, RustcEncodable, RustcDecodable)]
-pub struct CallData<'tcx> {
-    /// where the return value is written to
-    pub destination: Lvalue<'tcx>,
-
-    /// the fn being called
-    pub func: Operand<'tcx>,
-
-    /// the arguments
-    pub args: Vec<Operand<'tcx>>,
-}
-
 impl<'tcx> BasicBlockData<'tcx> {
-    pub fn new(terminator: Terminator<'tcx>) -> BasicBlockData<'tcx> {
+    pub fn new(terminator: Option<Terminator<'tcx>>) -> BasicBlockData<'tcx> {
         BasicBlockData {
             statements: vec![],
             terminator: terminator,
+            is_cleanup: false,
         }
     }
+
+    /// Accessor for terminator.
+    ///
+    /// Terminator may not be None after construction of the basic block is complete. This accessor
+    /// provides a convenience way to reach the terminator.
+    pub fn terminator(&self) -> &Terminator<'tcx> {
+        self.terminator.as_ref().expect("invalid terminator state")
+    }
+
+    pub fn terminator_mut(&mut self) -> &mut Terminator<'tcx> {
+        self.terminator.as_mut().expect("invalid terminator state")
+    }
 }
 
 impl<'tcx> Debug for Terminator<'tcx> {
@@ -351,15 +391,17 @@ impl<'tcx> Terminator<'tcx> {
         use self::Terminator::*;
         match *self {
             Goto { .. } => write!(fmt, "goto"),
-            Panic { .. } => write!(fmt, "panic"),
             If { cond: ref lv, .. } => write!(fmt, "if({:?})", lv),
             Switch { discr: ref lv, .. } => write!(fmt, "switch({:?})", lv),
             SwitchInt { discr: ref lv, .. } => write!(fmt, "switchInt({:?})", lv),
-            Diverge => write!(fmt, "diverge"),
             Return => write!(fmt, "return"),
-            Call { data: ref c, .. } => {
-                try!(write!(fmt, "{:?} = {:?}(", c.destination, c.func));
-                for (index, arg) in c.args.iter().enumerate() {
+            Resume => write!(fmt, "resume"),
+            Call { ref kind, ref func, ref args } => {
+                if let Some(destination) = kind.destination() {
+                    try!(write!(fmt, "{:?} = ", destination));
+                }
+                try!(write!(fmt, "{:?}(", func));
+                for (index, arg) in args.iter().enumerate() {
                     if index > 0 {
                         try!(write!(fmt, ", "));
                     }
@@ -374,10 +416,9 @@ impl<'tcx> Terminator<'tcx> {
     pub fn fmt_successor_labels(&self) -> Vec<Cow<'static, str>> {
         use self::Terminator::*;
         match *self {
-            Diverge | Return => vec![],
-            Goto { .. } | Panic { .. } => vec!["".into_cow()],
+            Return | Resume => vec![],
+            Goto { .. } => vec!["".into_cow()],
             If { .. } => vec!["true".into_cow(), "false".into_cow()],
-            Call { .. } => vec!["return".into_cow(), "unwind".into_cow()],
             Switch { ref adt_def, .. } => {
                 adt_def.variants
                        .iter()
@@ -394,6 +435,16 @@ impl<'tcx> Terminator<'tcx> {
                       .chain(iter::once(String::from("otherwise").into_cow()))
                       .collect()
             }
+            Call { ref kind, .. } => match *kind {
+                CallKind::Diverging =>
+                    vec![],
+                CallKind::DivergingCleanup(..) =>
+                    vec!["unwind".into_cow()],
+                CallKind::Converging { .. } =>
+                    vec!["return".into_cow()],
+                CallKind::ConvergingCleanup { .. } =>
+                    vec!["return".into_cow(), "unwind".into_cow()],
+            },
         }
     }
 }

Diff for: src/librustc/mir/visit.rs

@@ -84,7 +84,7 @@ pub trait Visitor<'tcx> {
         for statement in &data.statements {
             self.visit_statement(block, statement);
         }
-        self.visit_terminator(block, &data.terminator);
+        data.terminator.as_ref().map(|r| self.visit_terminator(block, r));
     }
 
     fn super_statement(&mut self, block: BasicBlock, statement: &Statement<'tcx>) {
@@ -107,8 +107,7 @@ pub trait Visitor<'tcx> {
 
     fn super_terminator(&mut self, block: BasicBlock, terminator: &Terminator<'tcx>) {
         match *terminator {
-            Terminator::Goto { target } |
-            Terminator::Panic { target } => {
+            Terminator::Goto { target } => {
                 self.visit_branch(block, target);
             }
 
@@ -133,17 +132,19 @@ pub trait Visitor<'tcx> {
                 }
             }
 
-            Terminator::Diverge |
+            Terminator::Resume |
             Terminator::Return => {
             }
 
-            Terminator::Call { ref data, ref targets } => {
-                self.visit_lvalue(&data.destination, LvalueContext::Store);
-                self.visit_operand(&data.func);
-                for arg in &data.args {
+            Terminator::Call { ref func, ref args, ref kind } => {
+                if let Some(ref destination) = kind.destination() {
+                    self.visit_lvalue(destination, LvalueContext::Store);
+                }
+                self.visit_operand(func);
+                for arg in args {
                     self.visit_operand(arg);
                 }
-                for &target in targets.as_slice() {
+                for &target in kind.successors() {
                     self.visit_branch(block, target);
                 }
             }
@@ -364,7 +365,7 @@ pub trait MutVisitor<'tcx> {
         for statement in &mut data.statements {
             self.visit_statement(block, statement);
         }
-        self.visit_terminator(block, &mut data.terminator);
+        data.terminator.as_mut().map(|r| self.visit_terminator(block, r));
     }
 
     fn super_statement(&mut self,
@@ -394,8 +395,7 @@ pub trait MutVisitor<'tcx> {
                         block: BasicBlock,
                         terminator: &mut Terminator<'tcx>) {
         match *terminator {
-            Terminator::Goto { target } |
-            Terminator::Panic { target } => {
+            Terminator::Goto { target } => {
                 self.visit_branch(block, target);
             }
 
@@ -420,17 +420,19 @@ pub trait MutVisitor<'tcx> {
                 }
             }
 
-            Terminator::Diverge |
+            Terminator::Resume |
             Terminator::Return => {
             }
 
-            Terminator::Call { ref mut data, ref mut targets } => {
-                self.visit_lvalue(&mut data.destination, LvalueContext::Store);
-                self.visit_operand(&mut data.func);
-                for arg in &mut data.args {
+            Terminator::Call { ref mut func, ref mut args, ref mut kind } => {
+                if let Some(ref mut destination) = kind.destination() {
+                    self.visit_lvalue(destination, LvalueContext::Store);
+                }
+                self.visit_operand(func);
+                for arg in args {
                     self.visit_operand(arg);
                 }
-                for &target in targets.as_slice() {
+                for &target in kind.successors() {
                     self.visit_branch(block, target);
                 }
             }

Diff for: src/librustc_mir/build/cfg.rs

@@ -28,7 +28,7 @@ impl<'tcx> CFG<'tcx> {
 
     pub fn start_new_block(&mut self) -> BasicBlock {
         let node_index = self.basic_blocks.len();
-        self.basic_blocks.push(BasicBlockData::new(Terminator::Diverge));
+        self.basic_blocks.push(BasicBlockData::new(None));
         BasicBlock::new(node_index)
     }
 
@@ -67,15 +67,9 @@ impl<'tcx> CFG<'tcx> {
     pub fn terminate(&mut self,
                      block: BasicBlock,
                      terminator: Terminator<'tcx>) {
-        // Check whether this block has already been terminated. For
-        // this, we rely on the fact that the initial state is to have
-        // a Diverge terminator and an empty list of targets (which
-        // is not a valid state).
-        debug_assert!(match self.block_data(block).terminator { Terminator::Diverge => true,
-                                                                _ => false },
+        debug_assert!(self.block_data(block).terminator.is_none(),
                       "terminate: block {:?} already has a terminator set", block);
-
-        self.block_data_mut(block).terminator = terminator;
+        self.block_data_mut(block).terminator = Some(terminator);
     }
 }
 

Diff for: src/librustc_mir/build/expr/as_lvalue.rs

@@ -63,15 +63,15 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                 this.cfg.push_assign(block, expr_span, // lt = idx < len
                                      &lt, Rvalue::BinaryOp(BinOp::Lt,
                                                            idx.clone(),
-                                                           Operand::Consume(len)));
+                                                           Operand::Consume(len.clone())));
 
                 let (success, failure) = (this.cfg.start_new_block(), this.cfg.start_new_block());
                 this.cfg.terminate(block,
                                    Terminator::If {
                                        cond: Operand::Consume(lt),
                                        targets: (success, failure),
                                    });
-                this.panic(failure);
+                this.panic_bounds_check(failure, idx.clone(), Operand::Consume(len), expr_span);
                 success.and(slice.index(idx))
             }
             ExprKind::SelfRef => {