Skip to content

Commit

Permalink
fix dynamic drop for unions
Browse files Browse the repository at this point in the history
Moving out of a union is now treated like moving out of its parent type.

Fixes #36246
  • Loading branch information
arielb1 authored and Ariel Ben-Yehuda committed Sep 16, 2016
1 parent 7b25e88 commit eeedc14
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 23 deletions.
52 changes: 33 additions & 19 deletions src/librustc_borrowck/borrowck/mir/gather_moves.rs
Expand Up @@ -12,7 +12,6 @@
use rustc::ty::{self, TyCtxt, ParameterEnvironment};
use rustc::mir::repr::*;
use rustc::util::nodemap::FnvHashMap;
use rustc::util::common::ErrorReported;
use rustc_data_structures::indexed_vec::{IndexVec};

use syntax::codemap::DUMMY_SP;
Expand Down Expand Up @@ -198,6 +197,11 @@ struct MoveDataBuilder<'a, 'tcx: 'a> {
data: MoveData<'tcx>,
}

pub enum MovePathError {
IllegalMove,
UnionMove { path: MovePathIndex },
}

impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
fn new(mir: &'a Mir<'tcx>,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
Expand Down Expand Up @@ -256,23 +260,23 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
move_path
}

/// This creates a MovePath for a given lvalue, returning an `ErrorReported`
/// This creates a MovePath for a given lvalue, returning an `MovePathError`
/// if that lvalue can't be moved from.
///
/// NOTE: lvalues behind references *do not* get a move path, which is
/// problematic for borrowck.
///
/// Maybe we should have seperate "borrowck" and "moveck" modes.
fn move_path_for(&mut self, lval: &Lvalue<'tcx>)
-> Result<MovePathIndex, ErrorReported>
-> Result<MovePathIndex, MovePathError>
{
debug!("lookup({:?})", lval);
match *lval {
Lvalue::Var(var) => Ok(self.data.rev_lookup.vars[var]),
Lvalue::Arg(arg) => Ok(self.data.rev_lookup.args[arg]),
Lvalue::Temp(temp) => Ok(self.data.rev_lookup.temps[temp]),
// error: can't move out of a static
Lvalue::Static(..) => Err(ErrorReported),
Lvalue::Static(..) => Err(MovePathError::IllegalMove),
Lvalue::ReturnPointer => match self.data.rev_lookup.return_ptr {
Some(ptr) => Ok(ptr),
ref mut ptr @ None => {
Expand Down Expand Up @@ -300,21 +304,28 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
fn move_path_for_projection(&mut self,
lval: &Lvalue<'tcx>,
proj: &LvalueProjection<'tcx>)
-> Result<MovePathIndex, ErrorReported>
-> Result<MovePathIndex, MovePathError>
{
let base = try!(self.move_path_for(&proj.base));
let lv_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
match lv_ty.sty {
// error: can't move out of borrowed content
ty::TyRef(..) | ty::TyRawPtr(..) => return Err(ErrorReported),
ty::TyRef(..) | ty::TyRawPtr(..) => return Err(MovePathError::IllegalMove),
// error: can't move out of struct with destructor
ty::TyStruct(adt, _) | ty::TyEnum(adt, _) if adt.has_dtor() =>
return Err(ErrorReported),

ty::TyArray(..) | ty::TySlice(..) => match proj.elem {
ty::TyAdt(adt, _) if adt.has_dtor() =>
return Err(MovePathError::IllegalMove),
// move out of union - always move the entire union
ty::TyAdt(adt, _) if adt.is_union() =>
return Err(MovePathError::UnionMove { path: base }),
// error: can't move out of a slice
ty::TySlice(..) =>
return Err(MovePathError::IllegalMove),
ty::TyArray(..) => match proj.elem {
// error: can't move out of an array
ProjectionElem::Index(..) => return Err(ErrorReported),
_ => {}
ProjectionElem::Index(..) => return Err(MovePathError::IllegalMove),
_ => {
// FIXME: still badly broken
}
},
_ => {}
};
Expand Down Expand Up @@ -521,13 +532,16 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
return
}

let path = self.move_path_for(lval).unwrap_or_else(|_| {
// Moving out of a bad path. Eventually, this should be a MIR
// borrowck error instead of a bug.
span_bug!(self.mir.span,
"Broken MIR: moving out of lvalue {:?}: {:?} at {:?}",
lval, lv_ty, loc);
});
let path = match self.move_path_for(lval) {
Ok(path) | Err(MovePathError::UnionMove { path }) => path,
Err(MovePathError::IllegalMove) => {
// Moving out of a bad path. Eventually, this should be a MIR
// borrowck error instead of a bug.
span_bug!(self.mir.span,
"Broken MIR: moving out of lvalue {:?}: {:?} at {:?}",
lval, lv_ty, loc);
}
};
let move_out = self.data.moves.push(MoveOut { path: path, source: loc });

debug!("gather_move({:?}, {:?}): adding move {:?} of {:?}",
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_borrowck/borrowck/mir/mod.rs
Expand Up @@ -256,12 +256,12 @@ fn lvalue_contents_drop_state_cannot_differ<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx
let ty = lv.ty(mir, tcx).to_ty(tcx);
match ty.sty {
ty::TyArray(..) | ty::TySlice(..) | ty::TyRef(..) | ty::TyRawPtr(..) => {
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} refd => false",
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} refd => true",
lv, ty);
true
}
ty::TyAdt(def, _) if def.has_dtor() => {
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} Drop => false",
ty::TyAdt(def, _) if def.has_dtor() || def.is_union() => {
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} Drop => true",
lv, ty);
true
}
Expand Down
25 changes: 24 additions & 1 deletion src/test/run-pass/dynamic-drop.rs
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(rustc_attrs)]
#![feature(untagged_unions)]

use std::cell::{Cell, RefCell};
use std::panic;
Expand Down Expand Up @@ -111,6 +111,20 @@ fn assignment1(a: &Allocator, c0: bool) {
_v = _w;
}

#[allow(unions_with_drop_fields)]
union Boxy<T> {
a: T,
b: T,
}

fn union1(a: &Allocator) {
unsafe {
let mut u = Boxy { a: a.alloc() };
u.b = a.alloc();
drop(u.a);
}
}

fn run_test<F>(mut f: F)
where F: FnMut(&Allocator)
{
Expand All @@ -136,6 +150,13 @@ fn run_test<F>(mut f: F)
}
}

fn run_test_nopanic<F>(mut f: F)
where F: FnMut(&Allocator)
{
let first_alloc = Allocator::new(usize::MAX);
f(&first_alloc);
}

fn main() {
run_test(|a| dynamic_init(a, false));
run_test(|a| dynamic_init(a, true));
Expand All @@ -149,4 +170,6 @@ fn main() {

run_test(|a| assignment1(a, false));
run_test(|a| assignment1(a, true));

run_test_nopanic(|a| union1(a));
}

0 comments on commit eeedc14

Please sign in to comment.