Dataflow changes and associated borrowck fix.

Revise rustc::middle::dataflow: one must select kill-kind when calling
add_kill. The current kill-kinds are (1.) kills associated with
ends-of-scopes and (2.) kills associated with the actual action of the
expression/pattern.

Then, use this to fix borrowck analysis so that it will not treat a
break that pops through an assignment `x = { ... break; ... }` as a
kill of the "moved-out" bit for `x`.

Fix #24267.

Author: pnkfelix

src/librustc/middle/dataflow.rs

@@ -64,8 +64,12 @@ pub struct DataFlowContext<'a, 'tcx: 'a, O> {
     /// bits generated as we exit the cfg node. Updated by `add_gen()`.
     gens: Vec<usize>,
 
-    /// bits killed as we exit the cfg node. Updated by `add_kill()`.
-    kills: Vec<usize>,
+    /// bits killed as we exit the cfg node, or non-locally jump over
+    /// it. Updated by `add_kill(KillFrom::ScopeEnd)`.
+    scope_kills: Vec<usize>,
+
+    /// bits killed as we exit the cfg node directly. Updated by `add_kill(KillFrom::Execution)`.
+    action_kills: Vec<usize>,
 
     /// bits that are valid on entry to the cfg node. Updated by
     /// `propagate()`.
@@ -130,15 +134,23 @@ impl<'a, 'tcx, O:DataFlowOperator> pprust::PpAnn for DataFlowContext<'a, 'tcx, O
                 "".to_string()
             };
 
-            let kills = &self.kills[start .. end];
-            let kills_str = if kills.iter().any(|&u| u != 0) {
-                format!(" kill: {}", bits_to_string(kills))
+            let action_kills = &self.action_kills[start .. end];
+            let action_kills_str = if action_kills.iter().any(|&u| u != 0) {
+                format!(" action_kill: {}", bits_to_string(action_kills))
+            } else {
+                "".to_string()
+            };
+
+            let scope_kills = &self.scope_kills[start .. end];
+            let scope_kills_str = if scope_kills.iter().any(|&u| u != 0) {
+                format!(" scope_kill: {}", bits_to_string(scope_kills))
             } else {
                 "".to_string()
             };
 
-            try!(ps.synth_comment(format!("id {}: {}{}{}", id, entry_str,
-                                          gens_str, kills_str)));
+            try!(ps.synth_comment(
+                format!("id {}: {}{}{}{}", id, entry_str,
+                        gens_str, action_kills_str, scope_kills_str)));
             try!(pp::space(&mut ps.s));
         }
         Ok(())
@@ -187,6 +199,25 @@ fn build_nodeid_to_index(decl: Option<&ast::FnDecl>,
     }
 }
 
+/// Flag used by `add_kill` to indicate whether the provided kill
+/// takes effect only when control flows directly through the node in
+/// question, or if the kill's effect is associated with any
+/// control-flow directly through or indirectly over the node.
+#[derive(Copy, Clone, PartialEq, Debug)]
+pub enum KillFrom {
+    /// A `ScopeEnd` kill is one that takes effect when any control
+    /// flow goes over the node. A kill associated with the end of the
+    /// scope of a variable declaration `let x;` is an example of a
+    /// `ScopeEnd` kill.
+    ScopeEnd,
+
+    /// An `Execution` kill is one that takes effect only when controw
+    /// flow goes through the node to completion. A kill associated
+    /// with an assignment statement `x = expr;` is an example of an
+    /// `Execution` kill.
+    Execution,
+}
+
 impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
     pub fn new(tcx: &'a ty::ctxt<'tcx>,
                analysis_name: &'static str,
@@ -206,8 +237,10 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
 
         let entry = if oper.initial_value() { usize::MAX } else {0};
 
-        let gens: Vec<_> = repeat(0).take(num_nodes * words_per_id).collect();
-        let kills: Vec<_> = repeat(0).take(num_nodes * words_per_id).collect();
+        let zeroes: Vec<_> = repeat(0).take(num_nodes * words_per_id).collect();
+        let gens: Vec<_> = zeroes.clone();
+        let kills1: Vec<_> = zeroes.clone();
+        let kills2: Vec<_> = zeroes;
         let on_entry: Vec<_> = repeat(entry).take(num_nodes * words_per_id).collect();
 
         let nodeid_to_index = build_nodeid_to_index(decl, cfg);
@@ -220,7 +253,8 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
             bits_per_id: bits_per_id,
             oper: oper,
             gens: gens,
-            kills: kills,
+            action_kills: kills1,
+            scope_kills: kills2,
             on_entry: on_entry
         }
     }
@@ -240,7 +274,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
         }
     }
 
-    pub fn add_kill(&mut self, id: ast::NodeId, bit: usize) {
+    pub fn add_kill(&mut self, kind: KillFrom, id: ast::NodeId, bit: usize) {
         //! Indicates that `id` kills `bit`
         debug!("{} add_kill(id={}, bit={})",
                self.analysis_name, id, bit);
@@ -250,7 +284,10 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
         let indices = get_cfg_indices(id, &self.nodeid_to_index);
         for &cfgidx in indices {
             let (start, end) = self.compute_id_range(cfgidx);
-            let kills = &mut self.kills[start.. end];
+            let kills = match kind {
+                KillFrom::Execution => &mut self.action_kills[start.. end],
+                KillFrom::ScopeEnd =>  &mut self.scope_kills[start.. end],
+            };
             set_bit(kills, bit);
         }
     }
@@ -264,7 +301,9 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
         let (start, end) = self.compute_id_range(cfgidx);
         let gens = &self.gens[start.. end];
         bitwise(bits, gens, &Union);
-        let kills = &self.kills[start.. end];
+        let kills = &self.action_kills[start.. end];
+        bitwise(bits, kills, &Subtract);
+        let kills = &self.scope_kills[start.. end];
         bitwise(bits, kills, &Subtract);
 
         debug!("{} apply_gen_kill(cfgidx={:?}, bits={}) [after]",
@@ -278,7 +317,8 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
 
         assert!(start < self.gens.len());
         assert!(end <= self.gens.len());
-        assert!(self.gens.len() == self.kills.len());
+        assert!(self.gens.len() == self.action_kills.len());
+        assert!(self.gens.len() == self.scope_kills.len());
         assert!(self.gens.len() == self.on_entry.len());
 
         (start, end)
@@ -402,6 +442,11 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
         //!
         //! This is usually called (if it is called at all), after
         //! all add_gen and add_kill calls, but before propagate.
+        //!
+        //! The callback predicate `p(b, e)` is invoked on each node
+        //! `e` that has been registered as an `exiting_scope` for a
+        //! given control-breaking node `b`, to determine whether to
+        //! include the associated kill-effects or not.
 
         debug!("{} add_kills_from_flow_exits", self.analysis_name);
         if self.bits_per_id == 0 {
@@ -412,7 +457,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
         cfg.graph.each_edge(|_edge_index, edge| {
             let flow_exit = edge.source();
             let (start, end) = self.compute_id_range(flow_exit);
-            let mut orig_kills = self.kills[start.. end].to_vec();
+            let mut orig_kills = self.scope_kills[start.. end].to_vec();
 
             let mut changed = false;
             for &node_id in &edge.data.exiting_scopes {
@@ -421,8 +466,12 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
                     Some(indices) => {
                         for &cfg_idx in indices {
                             let (start, end) = self.compute_id_range(cfg_idx);
-                            let kills = &self.kills[start.. end];
+                            let kills = &self.scope_kills[start.. end];
                             if bitwise(&mut orig_kills, kills, &Union) {
+                                debug!("scope exits: scope id={} \
+                                        (node={:?} of {:?}) added killset: {}",
+                                       node_id, cfg_idx, indices,
+                                       bits_to_string(kills));
                                 changed = true;
                             }
                         }
@@ -436,7 +485,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
             }
 
             if changed {
-                let bits = &mut self.kills[start.. end];
+                let bits = &mut self.scope_kills[start.. end];
                 debug!("{} add_kills_from_flow_exits flow_exit={:?} bits={} [before]",
                        self.analysis_name, flow_exit, mut_bits_to_string(bits));
                 bits.clone_from_slice(&orig_kills[..]);

src/librustc_borrowck/borrowck/mod.rs

@@ -24,6 +24,7 @@ use rustc::middle::cfg;
 use rustc::middle::dataflow::DataFlowContext;
 use rustc::middle::dataflow::BitwiseOperator;
 use rustc::middle::dataflow::DataFlowOperator;
+use rustc::middle::dataflow::KillFrom;
 use rustc::middle::expr_use_visitor as euv;
 use rustc::middle::mem_categorization as mc;
 use rustc::middle::region;
@@ -167,7 +168,7 @@ fn build_borrowck_dataflow_data<'a, 'tcx>(this: &mut BorrowckCtxt<'a, 'tcx>,
                              all_loans.len());
     for (loan_idx, loan) in all_loans.iter().enumerate() {
         loan_dfcx.add_gen(loan.gen_scope.node_id(), loan_idx);
-        loan_dfcx.add_kill(loan.kill_scope.node_id(), loan_idx);
+        loan_dfcx.add_kill(KillFrom::ScopeEnd, loan.kill_scope.node_id(), loan_idx);
     }
     loan_dfcx.add_kills_from_flow_exits(cfg);
     loan_dfcx.propagate(cfg, body);

src/librustc_borrowck/borrowck/move_data.rs

@@ -18,6 +18,7 @@ use rustc::middle::cfg;
 use rustc::middle::dataflow::DataFlowContext;
 use rustc::middle::dataflow::BitwiseOperator;
 use rustc::middle::dataflow::DataFlowOperator;
+use rustc::middle::dataflow::KillFrom;
 use rustc::middle::expr_use_visitor as euv;
 use rustc::middle::ty;
 use rustc::util::nodemap::{FnvHashMap, NodeSet};
@@ -59,6 +60,9 @@ pub struct MoveData<'tcx> {
     /// Assignments to a variable or path, like `x = foo`, but not `x += foo`.
     pub assignee_ids: RefCell<NodeSet>,
 
+    /// AST node ids for move entries that have kind `Declared`.
+    pub declared_ids: RefCell<NodeSet>,
+
     /// Path-fragments from moves in to or out of parts of structured data.
     pub fragments: RefCell<fragments::FragmentSets>,
 }
@@ -216,6 +220,7 @@ impl<'tcx> MoveData<'tcx> {
             var_assignments: RefCell::new(Vec::new()),
             variant_matches: RefCell::new(Vec::new()),
             assignee_ids: RefCell::new(NodeSet()),
+            declared_ids: RefCell::new(NodeSet()),
             fragments: RefCell::new(fragments::FragmentSets::new()),
         }
     }
@@ -376,6 +381,10 @@ impl<'tcx> MoveData<'tcx> {
         let next_move = self.path_first_move(path_index);
         self.set_path_first_move(path_index, move_index);
 
+        if kind == MoveKind::Declared {
+            self.declared_ids.borrow_mut().insert(id);
+        }
+
         self.moves.borrow_mut().push(Move {
             path: path_index,
             id: id,
@@ -473,11 +482,13 @@ impl<'tcx> MoveData<'tcx> {
 
         for (i, assignment) in self.var_assignments.borrow().iter().enumerate() {
             dfcx_assign.add_gen(assignment.id, i);
-            self.kill_moves(assignment.path, assignment.id, dfcx_moves);
+            self.kill_moves(assignment.path, assignment.id,
+                            KillFrom::Execution, dfcx_moves);
         }
 
         for assignment in &*self.path_assignments.borrow() {
-            self.kill_moves(assignment.path, assignment.id, dfcx_moves);
+            self.kill_moves(assignment.path, assignment.id,
+                            KillFrom::Execution, dfcx_moves);
         }
 
         // Kill all moves related to a variable `x` when
@@ -487,7 +498,8 @@ impl<'tcx> MoveData<'tcx> {
                 LpVar(..) | LpUpvar(..) | LpDowncast(..) => {
                     let kill_scope = path.loan_path.kill_scope(tcx);
                     let path = *self.path_map.borrow().get(&path.loan_path).unwrap();
-                    self.kill_moves(path, kill_scope.node_id(), dfcx_moves);
+                    self.kill_moves(path, kill_scope.node_id(),
+                                    KillFrom::ScopeEnd, dfcx_moves);
                 }
                 LpExtend(..) => {}
             }
@@ -500,7 +512,9 @@ impl<'tcx> MoveData<'tcx> {
             match lp.kind {
                 LpVar(..) | LpUpvar(..) | LpDowncast(..) => {
                     let kill_scope = lp.kill_scope(tcx);
-                    dfcx_assign.add_kill(kill_scope.node_id(), assignment_index);
+                    dfcx_assign.add_kill(KillFrom::ScopeEnd,
+                                         kill_scope.node_id(),
+                                         assignment_index);
                 }
                 LpExtend(..) => {
                     tcx.sess.bug("var assignment for non var path");
@@ -568,6 +582,7 @@ impl<'tcx> MoveData<'tcx> {
     fn kill_moves(&self,
                   path: MovePathIndex,
                   kill_id: ast::NodeId,
+                  kill_kind: KillFrom,
                   dfcx_moves: &mut MoveDataFlow) {
         // We can only perform kills for paths that refer to a unique location,
         // since otherwise we may kill a move from one location with an
@@ -576,7 +591,9 @@ impl<'tcx> MoveData<'tcx> {
         let loan_path = self.path_loan_path(path);
         if loan_path_is_precise(&*loan_path) {
             self.each_applicable_move(path, |move_index| {
-                dfcx_moves.add_kill(kill_id, move_index.get());
+                debug!("kill_moves add_kill {:?} kill_id={} move_index={}",
+                       kill_kind, kill_id, move_index.get());
+                dfcx_moves.add_kill(kill_kind, kill_id, move_index.get());
                 true
             });
         }