librustc: Don't use the same alloca for match binding which we reassign to in arm body.

Author: luqmana

src/librustc/middle/trans/_match.rs

@@ -189,7 +189,9 @@
 #![allow(non_camel_case_types)]
 
 use back::abi;
+use mc = middle::mem_categorization;
 use driver::config::FullDebugInfo;
+use euv = middle::expr_use_visitor;
 use llvm;
 use llvm::{ValueRef, BasicBlockRef};
 use middle::const_eval;
@@ -1292,13 +1294,55 @@ pub fn trans_match<'a>(
     trans_match_inner(bcx, match_expr.id, discr_expr, arms, dest)
 }
 
-fn create_bindings_map(bcx: &Block, pat: Gc<ast::Pat>) -> BindingsMap {
+/// Checks whether the binding in `discr` is assigned to anywhere in the expression `body`
+fn is_discr_reassigned(bcx: &Block, discr: &ast::Expr, body: &ast::Expr) -> bool {
+    match discr.node {
+        ast::ExprPath(..) => match bcx.def(discr.id) {
+            def::DefLocal(vid, _) | def::DefBinding(vid, _) => {
+                let mut rc = ReassignmentChecker {
+                    node: vid,
+                    reassigned: false
+                };
+                {
+                    let mut visitor = euv::ExprUseVisitor::new(&mut rc, bcx);
+                    visitor.walk_expr(body);
+                }
+                rc.reassigned
+            }
+            _ => false
+        },
+        _ => false
+    }
+}
+
+struct ReassignmentChecker {
+    node: ast::NodeId,
+    reassigned: bool
+}
+
+impl euv::Delegate for ReassignmentChecker {
+    fn consume(&mut self, _: ast::NodeId, _: Span, _: mc::cmt, _: euv::ConsumeMode) {}
+    fn consume_pat(&mut self, _: &ast::Pat, _: mc::cmt, _: euv::ConsumeMode) {}
+    fn borrow(&mut self, _: ast::NodeId, _: Span, _: mc::cmt, _: ty::Region,
+              _: ty::BorrowKind, _: euv::LoanCause) {}
+    fn decl_without_init(&mut self, _: ast::NodeId, _: Span) {}
+    fn mutate(&mut self, _: ast::NodeId, _: Span, cmt: mc::cmt, _: euv::MutateMode) {
+        match cmt.cat {
+            mc::cat_local(vid) => self.reassigned = self.node == vid,
+            _ => {}
+        }
+    }
+}
+
+fn create_bindings_map(bcx: &Block, pat: Gc<ast::Pat>,
+                      discr: &ast::Expr, body: &ast::Expr) -> BindingsMap {
     // Create the bindings map, which is a mapping from each binding name
     // to an alloca() that will be the value for that local variable.
     // Note that we use the names because each binding will have many ids
     // from the various alternatives.
     let ccx = bcx.ccx();
     let tcx = bcx.tcx();
+    let reassigned = is_discr_reassigned(bcx, discr, body);
     let mut bindings_map = HashMap::new();
     pat_bindings(&tcx.def_map, &*pat, |bm, p_id, span, path1| {
         let ident = path1.node;
@@ -1310,7 +1354,7 @@ fn create_bindings_map(bcx: &Block, pat: Gc<ast::Pat>) -> BindingsMap {
         let trmode;
         match bm {
             ast::BindByValue(_)
-                if !ty::type_moves_by_default(tcx, variable_ty) => {
+                if !ty::type_moves_by_default(tcx, variable_ty) || reassigned => {
                 llmatch = alloca_no_lifetime(bcx,
                                  llvariable_ty.ptr_to(),
                                  "__llmatch");
@@ -1371,7 +1415,7 @@ fn trans_match_inner<'a>(scope_cx: &'a Block<'a>,
     let arm_datas: Vec<ArmData> = arms.iter().map(|arm| ArmData {
         bodycx: fcx.new_id_block("case_body", arm.body.id),
         arm: arm,
-        bindings_map: create_bindings_map(bcx, *arm.pats.get(0))
+        bindings_map: create_bindings_map(bcx, *arm.pats.get(0), discr_expr, &*arm.body)
     }).collect();
 
     let mut static_inliner = StaticInliner { tcx: scope_cx.tcx() };

src/librustc/middle/trans/common.rs

@@ -16,6 +16,7 @@ use driver::session::Session;
 use llvm;
 use llvm::{ValueRef, BasicBlockRef, BuilderRef};
 use llvm::{True, False, Bool};
+use mc = middle::mem_categorization;
 use middle::def;
 use middle::lang_items::LangItem;
 use middle::subst;
@@ -481,6 +482,36 @@ impl<'a> Block<'a> {
     }
 }
 
+impl<'a> mc::Typer for Block<'a> {
+    fn tcx<'a>(&'a self) -> &'a ty::ctxt {
+        self.tcx()
+    }
+
+    fn node_ty(&self, id: ast::NodeId) -> mc::McResult<ty::t> {
+        Ok(node_id_type(self, id))
+    }
+
+    fn node_method_ty(&self, method_call: typeck::MethodCall) -> Option<ty::t> {
+        self.tcx().method_map.borrow().find(&method_call).map(|method| method.ty)
+    }
+
+    fn adjustments<'a>(&'a self) -> &'a RefCell<NodeMap<ty::AutoAdjustment>> {
+        &self.tcx().adjustments
+    }
+
+    fn is_method_call(&self, id: ast::NodeId) -> bool {
+        self.tcx().method_map.borrow().contains_key(&typeck::MethodCall::expr(id))
+    }
+
+    fn temporary_scope(&self, rvalue_id: ast::NodeId) -> Option<ast::NodeId> {
+        self.tcx().region_maps.temporary_scope(rvalue_id)
+    }
+
+    fn upvar_borrow(&self, upvar_id: ty::UpvarId) -> ty::UpvarBorrow {
+        self.tcx().upvar_borrow_map.borrow().get_copy(&upvar_id)
+    }
+}
+
 pub struct Result<'a> {
     pub bcx: &'a Block<'a>,
     pub val: ValueRef

src/test/run-pass/issue-15571.rs

@@ -0,0 +1,20 @@
+// Copyright 2014 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.
+
+fn main() {
+    let mut foo = Some(box 5i);
+    match foo {
+        None => {},
+        Some(x) => {
+            foo = Some(x);
+        }
+    };
+    println!("'{}'", foo.unwrap());
+}

src/test/run-pass/issue-16151.rs

@@ -0,0 +1,38 @@
+// Copyright 2014 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::mem;
+
+static mut DROP_COUNT: uint = 0;
+
+struct Fragment;
+
+impl Drop for Fragment {
+    fn drop(&mut self) {
+        unsafe {
+            DROP_COUNT += 1;
+        }
+    }
+}
+
+fn main() {
+    {
+        let mut fragments = vec![Fragment, Fragment, Fragment];
+        let _new_fragments: Vec<Fragment> = mem::replace(&mut fragments, vec![])
+            .move_iter()
+            .skip_while(|_fragment| {
+                true
+            }).collect();
+    }
+    unsafe {
+        assert_eq!(DROP_COUNT, 3);
+    }
+}
+