Skip to content

Commit

Permalink
librustc: Make sure to run destructors in the right order when matchi…
Browse files Browse the repository at this point in the history
…ng on moved value.
  • Loading branch information
luqmana committed Jul 5, 2014
1 parent 5d5c206 commit 1af8663
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 8 deletions.
16 changes: 11 additions & 5 deletions src/librustc/middle/trans/_match.rs
Expand Up @@ -962,8 +962,8 @@ fn compare_values<'a>(
}
}

fn insert_lllocals<'a>(mut bcx: &'a Block<'a>,
bindings_map: &BindingsMap)
fn insert_lllocals<'a>(mut bcx: &'a Block<'a>, bindings_map: &BindingsMap,
cs: Option<cleanup::ScopeId>)
-> &'a Block<'a> {
/*!
* For each binding in `data.bindings_map`, adds an appropriate entry into
Expand All @@ -990,6 +990,10 @@ fn insert_lllocals<'a>(mut bcx: &'a Block<'a>,
};

let datum = Datum::new(llval, binding_info.ty, Lvalue);
match cs {
Some(cs) => bcx.fcx.schedule_drop_and_zero_mem(cs, llval, binding_info.ty),
_ => {}
}

debug!("binding {:?} to {}",
binding_info.id,
Expand Down Expand Up @@ -1021,7 +1025,7 @@ fn compile_guard<'a, 'b>(
vec_map_to_str(vals, |v| bcx.val_to_str(*v)));
let _indenter = indenter();

let mut bcx = insert_lllocals(bcx, &data.bindings_map);
let mut bcx = insert_lllocals(bcx, &data.bindings_map, None);

let val = unpack_datum!(bcx, expr::trans(bcx, guard_expr));
let val = val.to_llbool(bcx);
Expand Down Expand Up @@ -1475,9 +1479,11 @@ fn trans_match_inner<'a>(scope_cx: &'a Block<'a>,
for arm_data in arm_datas.iter() {
let mut bcx = arm_data.bodycx;

// insert bindings into the lllocals map
bcx = insert_lllocals(bcx, &arm_data.bindings_map);
// insert bindings into the lllocals map and add cleanups
let cs = fcx.push_custom_cleanup_scope();
bcx = insert_lllocals(bcx, &arm_data.bindings_map, Some(cleanup::CustomScope(cs)));
bcx = expr::trans_into(bcx, &*arm_data.arm.body, dest);
bcx = fcx.pop_and_trans_custom_cleanup_scope(bcx, cs);
arm_cxs.push(bcx);
}

Expand Down
44 changes: 41 additions & 3 deletions src/librustc/middle/trans/cleanup.rs
Expand Up @@ -240,7 +240,8 @@ impl<'a> CleanupMethods<'a> for FunctionContext<'a> {
is_immediate: false,
on_unwind: ty::type_needs_unwind_cleanup(self.ccx.tcx(), ty),
val: val,
ty: ty
ty: ty,
zero: false
};

debug!("schedule_drop_mem({:?}, val={}, ty={})",
Expand All @@ -251,6 +252,33 @@ impl<'a> CleanupMethods<'a> for FunctionContext<'a> {
self.schedule_clean(cleanup_scope, drop as Box<Cleanup>);
}

fn schedule_drop_and_zero_mem(&self,
cleanup_scope: ScopeId,
val: ValueRef,
ty: ty::t) {
/*!
* Schedules a (deep) drop and zero-ing of `val`, which is a pointer
* to an instance of `ty`
*/

if !ty::type_needs_drop(self.ccx.tcx(), ty) { return; }
let drop = box DropValue {
is_immediate: false,
on_unwind: ty::type_needs_unwind_cleanup(self.ccx.tcx(), ty),
val: val,
ty: ty,
zero: true
};

debug!("schedule_drop_and_zero_mem({:?}, val={}, ty={}, zero={})",
cleanup_scope,
self.ccx.tn.val_to_str(val),
ty.repr(self.ccx.tcx()),
true);

self.schedule_clean(cleanup_scope, drop as Box<Cleanup>);
}

fn schedule_drop_immediate(&self,
cleanup_scope: ScopeId,
val: ValueRef,
Expand All @@ -264,7 +292,8 @@ impl<'a> CleanupMethods<'a> for FunctionContext<'a> {
is_immediate: true,
on_unwind: ty::type_needs_unwind_cleanup(self.ccx.tcx(), ty),
val: val,
ty: ty
ty: ty,
zero: false
};

debug!("schedule_drop_immediate({:?}, val={}, ty={})",
Expand Down Expand Up @@ -824,6 +853,7 @@ pub struct DropValue {
on_unwind: bool,
val: ValueRef,
ty: ty::t,
zero: bool
}

impl Cleanup for DropValue {
Expand All @@ -832,11 +862,15 @@ impl Cleanup for DropValue {
}

fn trans<'a>(&self, bcx: &'a Block<'a>) -> &'a Block<'a> {
if self.is_immediate {
let bcx = if self.is_immediate {
glue::drop_ty_immediate(bcx, self.val, self.ty)
} else {
glue::drop_ty(bcx, self.val, self.ty)
};
if self.zero {
base::zero_mem(bcx, self.val, self.ty);
}
bcx
}
}

Expand Down Expand Up @@ -927,6 +961,10 @@ pub trait CleanupMethods<'a> {
cleanup_scope: ScopeId,
val: ValueRef,
ty: ty::t);
fn schedule_drop_and_zero_mem(&self,
cleanup_scope: ScopeId,
val: ValueRef,
ty: ty::t);
fn schedule_drop_immediate(&self,
cleanup_scope: ScopeId,
val: ValueRef,
Expand Down
64 changes: 64 additions & 0 deletions src/test/run-pass/order-drop-with-match.rs
@@ -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.


// Test to make sure the destructors run in the right order.
// Each destructor sets it's tag in the corresponding entry
// in ORDER matching up to when it ran.
// Correct order is: matched, inner, outer

static mut ORDER: [uint, ..3] = [0, 0, 0];
static mut INDEX: uint = 0;

struct A;
impl Drop for A {
fn drop(&mut self) {
unsafe {
ORDER[INDEX] = 1;
INDEX = INDEX + 1;
}
}
}

struct B;
impl Drop for B {
fn drop(&mut self) {
unsafe {
ORDER[INDEX] = 2;
INDEX = INDEX + 1;
}
}
}

struct C;
impl Drop for C {
fn drop(&mut self) {
unsafe {
ORDER[INDEX] = 3;
INDEX = INDEX + 1;
}
}
}

fn main() {
{
let matched = A;
let _outer = C;
{
match matched {
_s => {}
}
let _inner = B;
}
}
unsafe {
assert_eq!(&[1, 2, 3], ORDER.as_slice());
}
}

5 comments on commit 1af8663

@bors
Copy link
Contributor

@bors bors commented on 1af8663 Jul 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from pnkfelix
at luqmana@1af8663

@bors
Copy link
Contributor

@bors bors commented on 1af8663 Jul 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging luqmana/rust/odp = 1af8663 into auto

@bors
Copy link
Contributor

@bors bors commented on 1af8663 Jul 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

luqmana/rust/odp = 1af8663 merged ok, testing candidate = 342321d

@bors
Copy link
Contributor

@bors bors commented on 1af8663 Jul 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 342321d

Please sign in to comment.