Skip to content

Commit

Permalink
fixes for Box<[T]>
Browse files Browse the repository at this point in the history
The pointer in the slice must not be null, because enum representations
make that assumption. The `exchange_malloc` function returns a non-null
sentinel for the zero size case, and it must not be passed to the
`exchange_free` lang item.

Since the length is always equal to the true capacity, a branch on the
length is enough for most types. Slices of zero size types are
statically special cased to never attempt deallocation. This is the same
implementation as `Vec<T>`.

Closes #14395
  • Loading branch information
thestinger committed Sep 9, 2014
1 parent b625d43 commit 9639caf
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 37 deletions.
6 changes: 0 additions & 6 deletions src/liballoc/heap.rs
Expand Up @@ -12,7 +12,6 @@
// FIXME: #13996: mark the `allocate` and `reallocate` return value as `noalias`
// and `nonnull`

use core::ptr::RawPtr;
#[cfg(not(test))] use core::raw;
#[cfg(stage0, not(test))] use util;

Expand Down Expand Up @@ -70,11 +69,6 @@ pub unsafe fn reallocate_inplace(ptr: *mut u8, size: uint, align: uint,
/// the value returned by `usable_size` for the requested size.
#[inline]
pub unsafe fn deallocate(ptr: *mut u8, size: uint, align: uint) {
// FIXME(14395) This is only required for DST ~[T], it should be removed once
// we fix that representation to not use null pointers.
if ptr.is_null() {
return;
}
imp::deallocate(ptr, size, align)
}

Expand Down
28 changes: 5 additions & 23 deletions src/librustc/middle/trans/expr.rs
Expand Up @@ -412,29 +412,11 @@ fn apply_adjustments<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
let vec_ty = ty::mk_uniq(tcx, ty::mk_vec(tcx, unit_ty, None));
let scratch = rvalue_scratch_datum(bcx, vec_ty, "__unsize_unique");

if len == 0 {
Store(bcx,
C_null(type_of::type_of(bcx.ccx(), unit_ty).ptr_to()),
get_dataptr(bcx, scratch.val));
} else {
// Box<[(), ..n]> will not allocate, but ~[()] expects an
// allocation of n bytes, so we must allocate here (yuck).
let llty = type_of::type_of(bcx.ccx(), unit_ty);
if llsize_of_alloc(bcx.ccx(), llty) == 0 {
let ptr_unit_ty = type_of::type_of(bcx.ccx(), unit_ty).ptr_to();
let align = C_uint(bcx.ccx(), 8);
let alloc_result = malloc_raw_dyn(bcx, ptr_unit_ty, vec_ty, ll_len, align);
bcx = alloc_result.bcx;
let base = get_dataptr(bcx, scratch.val);
Store(bcx, alloc_result.val, base);
} else {
let base = get_dataptr(bcx, scratch.val);
let base = PointerCast(bcx,
base,
type_of::type_of(bcx.ccx(), datum_ty).ptr_to());
bcx = lval.store_to(bcx, base);
}
}
let base = get_dataptr(bcx, scratch.val);
let base = PointerCast(bcx,
base,
type_of::type_of(bcx.ccx(), datum_ty).ptr_to());
bcx = lval.store_to(bcx, base);

Store(bcx, ll_len, get_len(bcx, scratch.val));
DatumBlock::new(bcx, scratch.to_expr_datum())
Expand Down
21 changes: 13 additions & 8 deletions src/librustc/middle/trans/tvec.rs
Expand Up @@ -72,14 +72,19 @@ pub fn make_drop_glue_unboxed<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
};

if should_deallocate {
let not_null = IsNotNull(bcx, dataptr);
with_cond(bcx, not_null, |bcx| {
let llty = type_of::type_of(ccx, unit_ty);
let llsize = machine::llsize_of(ccx, llty);
let llalign = C_uint(ccx, machine::llalign_of_min(ccx, llty) as uint);
let size = Mul(bcx, llsize, get_len(bcx, vptr));
glue::trans_exchange_free_dyn(bcx, dataptr, size, llalign)
})
let llty = type_of::type_of(ccx, unit_ty);
let unit_size = llsize_of_alloc(ccx, llty);
if unit_size != 0 {
let len = get_len(bcx, vptr);
let not_empty = ICmp(bcx, llvm::IntNE, len, C_uint(ccx, 0));
with_cond(bcx, not_empty, |bcx| {
let llalign = C_uint(ccx, machine::llalign_of_min(ccx, llty) as uint);
let size = Mul(bcx, C_uint(ccx, unit_size as uint), len);
glue::trans_exchange_free_dyn(bcx, dataptr, size, llalign)
})
} else {
bcx
}
} else {
bcx
}
Expand Down
6 changes: 6 additions & 0 deletions src/test/run-pass/empty-allocation-non-null.rs
Expand Up @@ -11,6 +11,12 @@
pub fn main() {
assert!(Some(box() ()).is_some());

let xs: Box<[()]> = box [];
assert!(Some(xs).is_some());

struct Foo;
assert!(Some(box Foo).is_some());

let ys: Box<[Foo]> = box [];
assert!(Some(ys).is_some());
}

5 comments on commit 9639caf

@bors
Copy link
Contributor

@bors bors commented on 9639caf Sep 11, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 9639caf Sep 11, 2014

Choose a reason for hiding this comment

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

merging thestinger/rust/dst = 9639caf into auto

@bors
Copy link
Contributor

@bors bors commented on 9639caf Sep 11, 2014

Choose a reason for hiding this comment

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

thestinger/rust/dst = 9639caf merged ok, testing candidate = 1f4117f

@bors
Copy link
Contributor

@bors bors commented on 9639caf Sep 11, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 9639caf Sep 11, 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 = 1f4117f

Please sign in to comment.