Skip to content

Commit eeedc14

Browse files
arielb1Ariel Ben-Yehuda
authored and
Ariel Ben-Yehuda
committed
fix dynamic drop for unions
Moving out of a union is now treated like moving out of its parent type. Fixes #36246
1 parent 7b25e88 commit eeedc14

File tree

3 files changed

+60
-23
lines changed

3 files changed

+60
-23
lines changed

src/librustc_borrowck/borrowck/mir/gather_moves.rs

+33-19
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use rustc::ty::{self, TyCtxt, ParameterEnvironment};
1313
use rustc::mir::repr::*;
1414
use rustc::util::nodemap::FnvHashMap;
15-
use rustc::util::common::ErrorReported;
1615
use rustc_data_structures::indexed_vec::{IndexVec};
1716

1817
use syntax::codemap::DUMMY_SP;
@@ -198,6 +197,11 @@ struct MoveDataBuilder<'a, 'tcx: 'a> {
198197
data: MoveData<'tcx>,
199198
}
200199

200+
pub enum MovePathError {
201+
IllegalMove,
202+
UnionMove { path: MovePathIndex },
203+
}
204+
201205
impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
202206
fn new(mir: &'a Mir<'tcx>,
203207
tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -256,23 +260,23 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
256260
move_path
257261
}
258262

259-
/// This creates a MovePath for a given lvalue, returning an `ErrorReported`
263+
/// This creates a MovePath for a given lvalue, returning an `MovePathError`
260264
/// if that lvalue can't be moved from.
261265
///
262266
/// NOTE: lvalues behind references *do not* get a move path, which is
263267
/// problematic for borrowck.
264268
///
265269
/// Maybe we should have seperate "borrowck" and "moveck" modes.
266270
fn move_path_for(&mut self, lval: &Lvalue<'tcx>)
267-
-> Result<MovePathIndex, ErrorReported>
271+
-> Result<MovePathIndex, MovePathError>
268272
{
269273
debug!("lookup({:?})", lval);
270274
match *lval {
271275
Lvalue::Var(var) => Ok(self.data.rev_lookup.vars[var]),
272276
Lvalue::Arg(arg) => Ok(self.data.rev_lookup.args[arg]),
273277
Lvalue::Temp(temp) => Ok(self.data.rev_lookup.temps[temp]),
274278
// error: can't move out of a static
275-
Lvalue::Static(..) => Err(ErrorReported),
279+
Lvalue::Static(..) => Err(MovePathError::IllegalMove),
276280
Lvalue::ReturnPointer => match self.data.rev_lookup.return_ptr {
277281
Some(ptr) => Ok(ptr),
278282
ref mut ptr @ None => {
@@ -300,21 +304,28 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
300304
fn move_path_for_projection(&mut self,
301305
lval: &Lvalue<'tcx>,
302306
proj: &LvalueProjection<'tcx>)
303-
-> Result<MovePathIndex, ErrorReported>
307+
-> Result<MovePathIndex, MovePathError>
304308
{
305309
let base = try!(self.move_path_for(&proj.base));
306310
let lv_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
307311
match lv_ty.sty {
308312
// error: can't move out of borrowed content
309-
ty::TyRef(..) | ty::TyRawPtr(..) => return Err(ErrorReported),
313+
ty::TyRef(..) | ty::TyRawPtr(..) => return Err(MovePathError::IllegalMove),
310314
// error: can't move out of struct with destructor
311-
ty::TyStruct(adt, _) | ty::TyEnum(adt, _) if adt.has_dtor() =>
312-
return Err(ErrorReported),
313-
314-
ty::TyArray(..) | ty::TySlice(..) => match proj.elem {
315+
ty::TyAdt(adt, _) if adt.has_dtor() =>
316+
return Err(MovePathError::IllegalMove),
317+
// move out of union - always move the entire union
318+
ty::TyAdt(adt, _) if adt.is_union() =>
319+
return Err(MovePathError::UnionMove { path: base }),
320+
// error: can't move out of a slice
321+
ty::TySlice(..) =>
322+
return Err(MovePathError::IllegalMove),
323+
ty::TyArray(..) => match proj.elem {
315324
// error: can't move out of an array
316-
ProjectionElem::Index(..) => return Err(ErrorReported),
317-
_ => {}
325+
ProjectionElem::Index(..) => return Err(MovePathError::IllegalMove),
326+
_ => {
327+
// FIXME: still badly broken
328+
}
318329
},
319330
_ => {}
320331
};
@@ -521,13 +532,16 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
521532
return
522533
}
523534

524-
let path = self.move_path_for(lval).unwrap_or_else(|_| {
525-
// Moving out of a bad path. Eventually, this should be a MIR
526-
// borrowck error instead of a bug.
527-
span_bug!(self.mir.span,
528-
"Broken MIR: moving out of lvalue {:?}: {:?} at {:?}",
529-
lval, lv_ty, loc);
530-
});
535+
let path = match self.move_path_for(lval) {
536+
Ok(path) | Err(MovePathError::UnionMove { path }) => path,
537+
Err(MovePathError::IllegalMove) => {
538+
// Moving out of a bad path. Eventually, this should be a MIR
539+
// borrowck error instead of a bug.
540+
span_bug!(self.mir.span,
541+
"Broken MIR: moving out of lvalue {:?}: {:?} at {:?}",
542+
lval, lv_ty, loc);
543+
}
544+
};
531545
let move_out = self.data.moves.push(MoveOut { path: path, source: loc });
532546

533547
debug!("gather_move({:?}, {:?}): adding move {:?} of {:?}",

src/librustc_borrowck/borrowck/mir/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,12 @@ fn lvalue_contents_drop_state_cannot_differ<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx
256256
let ty = lv.ty(mir, tcx).to_ty(tcx);
257257
match ty.sty {
258258
ty::TyArray(..) | ty::TySlice(..) | ty::TyRef(..) | ty::TyRawPtr(..) => {
259-
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} refd => false",
259+
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} refd => true",
260260
lv, ty);
261261
true
262262
}
263-
ty::TyAdt(def, _) if def.has_dtor() => {
264-
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} Drop => false",
263+
ty::TyAdt(def, _) if def.has_dtor() || def.is_union() => {
264+
debug!("lvalue_contents_drop_state_cannot_differ lv: {:?} ty: {:?} Drop => true",
265265
lv, ty);
266266
true
267267
}

src/test/run-pass/dynamic-drop.rs

+24-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
#![feature(rustc_attrs)]
11+
#![feature(untagged_unions)]
1212

1313
use std::cell::{Cell, RefCell};
1414
use std::panic;
@@ -111,6 +111,20 @@ fn assignment1(a: &Allocator, c0: bool) {
111111
_v = _w;
112112
}
113113

114+
#[allow(unions_with_drop_fields)]
115+
union Boxy<T> {
116+
a: T,
117+
b: T,
118+
}
119+
120+
fn union1(a: &Allocator) {
121+
unsafe {
122+
let mut u = Boxy { a: a.alloc() };
123+
u.b = a.alloc();
124+
drop(u.a);
125+
}
126+
}
127+
114128
fn run_test<F>(mut f: F)
115129
where F: FnMut(&Allocator)
116130
{
@@ -136,6 +150,13 @@ fn run_test<F>(mut f: F)
136150
}
137151
}
138152

153+
fn run_test_nopanic<F>(mut f: F)
154+
where F: FnMut(&Allocator)
155+
{
156+
let first_alloc = Allocator::new(usize::MAX);
157+
f(&first_alloc);
158+
}
159+
139160
fn main() {
140161
run_test(|a| dynamic_init(a, false));
141162
run_test(|a| dynamic_init(a, true));
@@ -149,4 +170,6 @@ fn main() {
149170

150171
run_test(|a| assignment1(a, false));
151172
run_test(|a| assignment1(a, true));
173+
174+
run_test_nopanic(|a| union1(a));
152175
}

0 commit comments

Comments
 (0)