From d7c0f7d1c07a060b6d06bdd60b24c78bd2c9a6c3 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Fri, 1 Aug 2014 19:38:10 -0700 Subject: [PATCH] librustc: Don't use the same alloca for match binding which we reassign to in arm body. --- src/librustc/middle/trans/_match.rs | 50 +++++++++++++++++++++++++++-- src/librustc/middle/trans/common.rs | 31 ++++++++++++++++++ src/test/run-pass/issue-15571.rs | 20 ++++++++++++ src/test/run-pass/issue-16151.rs | 38 ++++++++++++++++++++++ 4 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 src/test/run-pass/issue-15571.rs create mode 100644 src/test/run-pass/issue-16151.rs diff --git a/src/librustc/middle/trans/_match.rs b/src/librustc/middle/trans/_match.rs index 5334205aa523..d8cbf5a2341e 100644 --- a/src/librustc/middle/trans/_match.rs +++ b/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) -> 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, + 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) -> 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 = 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() }; diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index f06415f26ad3..84f380e862a4 100644 --- a/src/librustc/middle/trans/common.rs +++ b/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 { + Ok(node_id_type(self, id)) + } + + fn node_method_ty(&self, method_call: typeck::MethodCall) -> Option { + self.tcx().method_map.borrow().find(&method_call).map(|method| method.ty) + } + + fn adjustments<'a>(&'a self) -> &'a RefCell> { + &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 { + 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 diff --git a/src/test/run-pass/issue-15571.rs b/src/test/run-pass/issue-15571.rs new file mode 100644 index 000000000000..1a1618fbc48f --- /dev/null +++ b/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 or the MIT license +// , 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()); +} diff --git a/src/test/run-pass/issue-16151.rs b/src/test/run-pass/issue-16151.rs new file mode 100644 index 000000000000..9cc571c7819c --- /dev/null +++ b/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 or the MIT license +// , 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 = mem::replace(&mut fragments, vec![]) + .move_iter() + .skip_while(|_fragment| { + true + }).collect(); + } + unsafe { + assert_eq!(DROP_COUNT, 3); + } +} +