schedule drops on bindings only after initializing them

This reduces the number of dynamic drops in libstd from 1141 to 899.
However, without this change, the next patch would have created much
more dynamic drops.

A basic merge unswitching hack reduced the number of dynamic drops to
644, with no effect on stack usage. I should be writing a more dedicated
drop unswitching pass.

No performance measurements.

Author: arielb1

src/librustc_borrowck/borrowck/mir/elaborate_drops.rs

@@ -161,6 +161,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
     fn create_drop_flag(&mut self, index: MovePathIndex) {
         let tcx = self.tcx;
         let patch = &mut self.patch;
+        debug!("create_drop_flag({:?})", self.mir.span);
         self.drop_flags.entry(index).or_insert_with(|| {
             patch.new_temp(tcx.types.bool)
         });

src/librustc_mir/build/block.rs

@@ -67,7 +67,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                                 this.expr_into_pattern(block, pattern, init)
                             }));
                         } else {
-                            this.storage_live_for_bindings(block, &pattern);
+                            this.visit_bindings(&pattern, &mut |this, _, _, node, span, _| {
+                                this.storage_live_binding(block, node, span);
+                                this.schedule_drop_for_binding(node, span);
+                            })
                         }
 
                         // Enter the visibility scope, after evaluating the initializer.

src/librustc_mir/build/matches/mod.rs

@@ -123,16 +123,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             PatternKind::Binding { mode: BindingMode::ByValue,
                                    var,
                                    subpattern: None, .. } => {
-                self.storage_live_for_bindings(block, &irrefutable_pat);
-                let lvalue = Lvalue::Local(self.var_indices[&var]);
-                return self.into(&lvalue, block, initializer);
+                let lvalue = self.storage_live_binding(block, var, irrefutable_pat.span);
+                unpack!(block = self.into(&lvalue, block, initializer));
+                self.schedule_drop_for_binding(var, irrefutable_pat.span);
+                block.unit()
+            }
+            _ => {
+                let lvalue = unpack!(block = self.as_lvalue(block, initializer));
+                self.lvalue_into_pattern(block, irrefutable_pat, &lvalue)
             }
-            _ => {}
         }
-        let lvalue = unpack!(block = self.as_lvalue(block, initializer));
-        self.lvalue_into_pattern(block,
-                                 irrefutable_pat,
-                                 &lvalue)
     }
 
     pub fn lvalue_into_pattern(&mut self,
@@ -174,79 +174,70 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                             scope_span: Span,
                             pattern: &Pattern<'tcx>)
                             -> Option<VisibilityScope> {
-        match *pattern.kind {
-            PatternKind::Binding { mutability, name, mode: _, var, ty, ref subpattern } => {
-                if var_scope.is_none() {
-                    var_scope = Some(self.new_visibility_scope(scope_span));
-                }
-                let source_info = SourceInfo {
-                    span: pattern.span,
-                    scope: var_scope.unwrap()
-                };
-                self.declare_binding(source_info, mutability, name, var, ty);
-                if let Some(subpattern) = subpattern.as_ref() {
-                    var_scope = self.declare_bindings(var_scope, scope_span, subpattern);
-                }
-            }
-            PatternKind::Array { ref prefix, ref slice, ref suffix } |
-            PatternKind::Slice { ref prefix, ref slice, ref suffix } => {
-                for subpattern in prefix.iter().chain(slice).chain(suffix) {
-                    var_scope = self.declare_bindings(var_scope, scope_span, subpattern);
-                }
-            }
-            PatternKind::Constant { .. } | PatternKind::Range { .. } | PatternKind::Wild => {
-            }
-            PatternKind::Deref { ref subpattern } => {
-                var_scope = self.declare_bindings(var_scope, scope_span, subpattern);
-            }
-            PatternKind::Leaf { ref subpatterns } |
-            PatternKind::Variant { ref subpatterns, .. } => {
-                for subpattern in subpatterns {
-                    var_scope = self.declare_bindings(var_scope, scope_span, &subpattern.pattern);
-                }
+        self.visit_bindings(pattern, &mut |this, mutability, name, var, span, ty| {
+            if var_scope.is_none() {
+                var_scope = Some(this.new_visibility_scope(scope_span));
             }
-        }
+            let source_info = SourceInfo {
+                span: span,
+                scope: var_scope.unwrap()
+            };
+            this.declare_binding(source_info, mutability, name, var, ty);
+        });
         var_scope
     }
 
-    /// Emit `StorageLive` for every binding in the pattern.
-    pub fn storage_live_for_bindings(&mut self,
-                                     block: BasicBlock,
-                                     pattern: &Pattern<'tcx>) {
-        match *pattern.kind {
-            PatternKind::Binding { var, ref subpattern, .. } => {
-                let lvalue = Lvalue::Local(self.var_indices[&var]);
-                let source_info = self.source_info(pattern.span);
-                self.cfg.push(block, Statement {
-                    source_info: source_info,
-                    kind: StatementKind::StorageLive(lvalue)
-                });
+    pub fn storage_live_binding(&mut self, block: BasicBlock, var: NodeId, span: Span)
+                            -> Lvalue<'tcx>
+    {
+        let local_id = self.var_indices[&var];
+        let source_info = self.source_info(span);
+        self.cfg.push(block, Statement {
+            source_info: source_info,
+            kind: StatementKind::StorageLive(Lvalue::Local(local_id))
+        });
+        Lvalue::Local(local_id)
+    }
 
+    pub fn schedule_drop_for_binding(&mut self, var: NodeId, span: Span) {
+        let local_id = self.var_indices[&var];
+        let var_ty = self.local_decls[local_id].ty;
+        let extent = self.hir.tcx().region_maps.var_scope(var);
+        self.schedule_drop(span, extent, &Lvalue::Local(local_id), var_ty);
+    }
+
+    pub fn visit_bindings<F>(&mut self, pattern: &Pattern<'tcx>, mut f: &mut F)
+        where F: FnMut(&mut Self, Mutability, Name, NodeId, Span, Ty<'tcx>)
+    {
+        match *pattern.kind {
+            PatternKind::Binding { mutability, name, var, ty, ref subpattern, .. } => {
+                f(self, mutability, name, var, pattern.span, ty);
                 if let Some(subpattern) = subpattern.as_ref() {
-                    self.storage_live_for_bindings(block, subpattern);
+                    self.visit_bindings(subpattern, f);
                 }
             }
             PatternKind::Array { ref prefix, ref slice, ref suffix } |
             PatternKind::Slice { ref prefix, ref slice, ref suffix } => {
                 for subpattern in prefix.iter().chain(slice).chain(suffix) {
-                    self.storage_live_for_bindings(block, subpattern);
+                    self.visit_bindings(subpattern, f);
                 }
             }
             PatternKind::Constant { .. } | PatternKind::Range { .. } | PatternKind::Wild => {
             }
             PatternKind::Deref { ref subpattern } => {
-                self.storage_live_for_bindings(block, subpattern);
+                self.visit_bindings(subpattern, f);
             }
             PatternKind::Leaf { ref subpatterns } |
             PatternKind::Variant { ref subpatterns, .. } => {
                 for subpattern in subpatterns {
-                    self.storage_live_for_bindings(block, &subpattern.pattern);
+                    self.visit_bindings(&subpattern.pattern, f);
                 }
             }
         }
     }
 }
 
+
 /// List of blocks for each arm (and potentially other metadata in the
 /// future).
 struct ArmBlocks {
@@ -691,25 +682,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
 
         // Assign each of the bindings. This may trigger moves out of the candidate.
         for binding in bindings {
-            // Find the variable for the `var_id` being bound. It
-            // should have been created by a previous call to
-            // `declare_bindings`.
-            let var_index = self.var_indices[&binding.var_id];
-
+            let source_info = self.source_info(binding.span);
+            let local = self.storage_live_binding(block, binding.var_id, binding.span);
+            self.schedule_drop_for_binding(binding.var_id, binding.span);
             let rvalue = match binding.binding_mode {
                 BindingMode::ByValue =>
                     Rvalue::Use(Operand::Consume(binding.source)),
                 BindingMode::ByRef(region, borrow_kind) =>
                     Rvalue::Ref(region, borrow_kind, binding.source),
             };
-
-            let source_info = self.source_info(binding.span);
-            self.cfg.push(block, Statement {
-                source_info: source_info,
-                kind: StatementKind::StorageLive(Lvalue::Local(var_index))
-            });
-            self.cfg.push_assign(block, source_info,
-                                 &Lvalue::Local(var_index), rvalue);
+            self.cfg.push_assign(block, source_info, &local, rvalue);
         }
     }
 
@@ -730,8 +712,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             name: Some(name),
             source_info: Some(source_info),
         });
-        let extent = self.hir.tcx().region_maps.var_scope(var_id);
-        self.schedule_drop(source_info.span, extent, &Lvalue::Local(var), var_ty);
         self.var_indices.insert(var_id, var);
 
         debug!("declare_binding: var={:?}", var);

src/test/mir-opt/storage_ranges.rs

@@ -31,8 +31,8 @@ fn main() {
 //         _3 = &_4;
 //         StorageDead(_5);
 //         _2 = ();
-//         StorageDead(_4);
 //         StorageDead(_3);
+//         StorageDead(_4);
 //         StorageLive(_6);
 //         _6 = const 1i32;
 //         _0 = ();

src/test/run-pass/mir_drop_order.rs

@@ -0,0 +1,42 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::cell::RefCell;
+
+pub struct DropLogger<'a> {
+    id: usize,
+    log: &'a RefCell<Vec<usize>>
+}
+
+impl<'a> Drop for DropLogger<'a> {
+    fn drop(&mut self) {
+        self.log.borrow_mut().push(self.id);
+    }
+}
+
+fn main() {
+    let log = RefCell::new(vec![]);
+    let d = |id| DropLogger { id: id, log: &log };
+    let get = || -> Vec<_> {
+        let mut m = log.borrow_mut();
+        let n = m.drain(..);
+        n.collect()
+    };
+
+    {
+        let _x = (d(0), &d(1), d(2), &d(3));
+        // all borrows are extended - nothing has been dropped yet
+        assert_eq!(get(), vec![]);
+    }
+    // in a let-statement, extended lvalues are dropped
+    // *after* the let result (tho they have the same scope
+    // as far as scope-based borrowck goes).
+    assert_eq!(get(), vec![0, 2, 3, 1]);
+}