Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Aug 10, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/librustc/metadata/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,10 @@ pub enum astencode_tag { // Reserves 0x40 -- 0x5f
tag_table_moves_map = 0x52,
tag_table_capture_map = 0x53,
tag_table_unboxed_closure_type = 0x54,
tag_table_upvar_borrow_map = 0x55,
}
static first_astencode_tag: uint = tag_ast as uint;
static last_astencode_tag: uint = tag_table_unboxed_closure_type as uint;
static last_astencode_tag: uint = tag_table_upvar_borrow_map as uint;
impl astencode_tag {
pub fn from_uint(value : uint) -> Option<astencode_tag> {
let is_a_tag = first_astencode_tag <= value && value <= last_astencode_tag;
Expand Down
43 changes: 42 additions & 1 deletion src/librustc/middle/astencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use driver::session::Session;
use metadata::decoder;
use middle::def;
use e = metadata::encoder;
use middle::freevars;
use middle::freevars::freevar_entry;
use middle::region;
use metadata::tydecode;
Expand Down Expand Up @@ -551,6 +552,15 @@ impl tr for freevar_entry {
}
}

impl tr for ty::UpvarBorrow {
fn tr(&self, xcx: &ExtendedDecodeContext) -> ty::UpvarBorrow {
ty::UpvarBorrow {
kind: self.kind,
region: self.region.tr(xcx)
}
}
}

// ______________________________________________________________________
// Encoding and decoding of MethodCallee

Expand Down Expand Up @@ -1061,7 +1071,29 @@ fn encode_side_tables_for_id(ecx: &e::EncodeContext,
Ok(encode_freevar_entry(rbml_w, fv_entry))
});
})
})
});

for freevar in fv.iter() {
match freevars::get_capture_mode(tcx, id) {
freevars::CaptureByRef => {
rbml_w.tag(c::tag_table_upvar_borrow_map, |rbml_w| {
rbml_w.id(id);
rbml_w.tag(c::tag_table_val, |rbml_w| {
let var_id = freevar.def.def_id().node;
let upvar_id = ty::UpvarId {
var_id: var_id,
closure_expr_id: id
};
let upvar_borrow = tcx.upvar_borrow_map.borrow()
.get_copy(&upvar_id);
var_id.encode(rbml_w);
upvar_borrow.encode(rbml_w);
})
})
}
_ => {}
}
}
}

let lid = ast::DefId { krate: ast::LOCAL_CRATE, node: id };
Expand Down Expand Up @@ -1468,6 +1500,15 @@ fn decode_side_tables(xcx: &ExtendedDecodeContext,
}).unwrap().move_iter().collect();
dcx.tcx.freevars.borrow_mut().insert(id, fv_info);
}
c::tag_table_upvar_borrow_map => {
let var_id: ast::NodeId = Decodable::decode(val_dsr).unwrap();
let upvar_id = ty::UpvarId {
var_id: xcx.tr_id(var_id),
closure_expr_id: id
};
let ub: ty::UpvarBorrow = Decodable::decode(val_dsr).unwrap();
dcx.tcx.upvar_borrow_map.borrow_mut().insert(upvar_id, ub.tr(xcx));
}
c::tag_table_tcache => {
let pty = val_dsr.read_polytype(xcx);
let lid = ast::DefId { krate: ast::LOCAL_CRATE, node: id };
Expand Down
8 changes: 3 additions & 5 deletions src/librustc/middle/freevars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#![allow(non_camel_case_types)]

use middle::def;
use middle::mem_categorization::Typer;
use middle::resolve;
use middle::ty;
use util::nodemap::{DefIdSet, NodeMap, NodeSet};
Expand Down Expand Up @@ -147,11 +148,8 @@ pub fn with_freevars<T>(tcx: &ty::ctxt, fid: ast::NodeId, f: |&[freevar_entry]|
}
}

pub fn get_capture_mode(tcx: &ty::ctxt,
closure_expr_id: ast::NodeId)
-> CaptureMode
{
let fn_ty = ty::node_id_to_type(tcx, closure_expr_id);
pub fn get_capture_mode<T: Typer>(tcx: &T, closure_expr_id: ast::NodeId) -> CaptureMode {
let fn_ty = tcx.node_ty(closure_expr_id).ok().expect("couldn't find closure ty?");
match ty::ty_closure_store(fn_ty) {
ty::RegionTraitStore(..) => CaptureByRef,
ty::UniqTraitStore => CaptureByValue
Expand Down
53 changes: 50 additions & 3 deletions src/librustc/middle/trans/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1292,13 +1294,58 @@ 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::DefArg(vid, _) | def::DefBinding(vid, _) |
def::DefLocal(vid, _) | def::DefUpvar(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_copied_upvar(mc::CopiedUpvar { upvar_id: vid, .. }) |
mc::cat_arg(vid) | 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;
Expand All @@ -1310,7 +1357,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");
Expand Down Expand Up @@ -1371,7 +1418,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() };
Expand Down
31 changes: 31 additions & 0 deletions src/librustc/middle/trans/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ pub struct UpvarId {
pub closure_expr_id: ast::NodeId,
}

#[deriving(Clone, PartialEq, Eq, Hash, Show)]
#[deriving(Clone, PartialEq, Eq, Hash, Show, Encodable, Decodable)]
pub enum BorrowKind {
/// Data must be immutable and is aliasable.
ImmBorrow,
Expand Down Expand Up @@ -634,7 +634,7 @@ pub enum BorrowKind {
* the closure, so sometimes it is necessary for them to be larger
* than the closure lifetime itself.
*/
#[deriving(PartialEq, Clone)]
#[deriving(PartialEq, Clone, Encodable, Decodable)]
pub struct UpvarBorrow {
pub kind: BorrowKind,
pub region: ty::Region,
Expand Down
64 changes: 64 additions & 0 deletions src/test/run-pass/issue-15571.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// 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 match_on_local() {
let mut foo = Some(box 5i);
match foo {
None => {},
Some(x) => {
foo = Some(x);
}
}
println!("'{}'", foo.unwrap());
}

fn match_on_arg(mut foo: Option<Box<int>>) {
match foo {
None => {}
Some(x) => {
foo = Some(x);
}
}
println!("'{}'", foo.unwrap());
}

fn match_on_binding() {
match Some(box 7i) {
mut foo => {
match foo {
None => {},
Some(x) => {
foo = Some(x);
}
}
println!("'{}'", foo.unwrap());
}
}
}

fn match_on_upvar() {
let mut foo = Some(box 8i);
(proc() {
match foo {
None => {},
Some(x) => {
foo = Some(x);
}
}
println!("'{}'", foo.unwrap());
})();
}

fn main() {
match_on_local();
match_on_arg(Some(box 6i));
match_on_binding();
match_on_upvar();
}
38 changes: 38 additions & 0 deletions src/test/run-pass/issue-16151.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}