Skip to content

expose needs_drop under mem #41892

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

Merged
merged 3 commits into from
May 21, 2017
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
7 changes: 7 additions & 0 deletions src/doc/unstable-book/src/library-features/needs-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# `needs_drop`

The tracking issue for this feature is: [#41890]

[#41890]: https://github.com/rust-lang/rust/issues/41890

------------------------
9 changes: 4 additions & 5 deletions src/libarena/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#![feature(core_intrinsics)]
#![feature(dropck_eyepatch)]
#![feature(generic_param_attrs)]
#![feature(needs_drop)]
#![cfg_attr(stage0, feature(staged_api))]
#![cfg_attr(test, feature(test))]

Expand Down Expand Up @@ -82,7 +83,7 @@ impl<T> TypedArenaChunk<T> {
unsafe fn destroy(&mut self, len: usize) {
// The branch on needs_drop() is an -O1 performance optimization.
// Without the branch, dropping TypedArena<u8> takes linear time.
if intrinsics::needs_drop::<T>() {
if mem::needs_drop::<T>() {
let mut start = self.start();
// Destroy all allocated objects.
for _ in 0..len {
Expand Down Expand Up @@ -350,7 +351,7 @@ impl DroplessArena {
#[inline]
pub fn alloc<T>(&self, object: T) -> &mut T {
unsafe {
assert!(!intrinsics::needs_drop::<T>());
assert!(!mem::needs_drop::<T>());
assert!(mem::size_of::<T>() != 0);

self.align_for::<T>();
Expand Down Expand Up @@ -379,9 +380,7 @@ impl DroplessArena {
#[inline]
pub fn alloc_slice<T>(&self, slice: &[T]) -> &mut [T]
where T: Copy {
unsafe {
assert!(!intrinsics::needs_drop::<T>());
}
assert!(!mem::needs_drop::<T>());
assert!(mem::size_of::<T>() != 0);
assert!(slice.len() != 0);
self.align_for::<T>();
Expand Down
52 changes: 52 additions & 0 deletions src/libcore/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,58 @@ pub fn align_of_val<T: ?Sized>(val: &T) -> usize {
unsafe { intrinsics::min_align_of_val(val) }
}

/// Returns whether dropping values of type `T` matters.
///
/// This is purely an optimization hint, and may be implemented conservatively.
/// For instance, always returning `true` would be a valid implementation of
/// this function.
Copy link
Member

Choose a reason for hiding this comment

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

So there's some backstory here: basically this was never implemented in the compiler in a principled way, to account for associated types. I've made the Copy check recursive recently but I don't believe that's relevant here.
What's left is to normalize associated type projections in the recursion and/or make an auto trait and rely on the trait system to handle the structural recursion involved here. cc @nikomatsakis

Copy link
Contributor

@gnzlbg gnzlbg Sep 13, 2019

Choose a reason for hiding this comment

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

@eddyb Is this a bug in the current implementation, and if so, is there an issue tracking it? If not, could you maybe expand on why needs_drop cannot be accurate?

///
/// Low level implementations of things like collections, which need to manually
/// drop their data, should use this function to avoid unnecessarily
/// trying to drop all their contents when they are destroyed. This might not
/// make a difference in release builds (where a loop that has no side-effects
/// is easily detected and eliminated), but is often a big win for debug builds.
///
/// Note that `ptr::drop_in_place` already performs this check, so if your workload
/// can be reduced to some small number of drop_in_place calls, using this is
/// unnecessary. In particular note that you can drop_in_place a slice, and that
/// will do a single needs_drop check for all the values.
///
/// Types like Vec therefore just `drop_in_place(&mut self[..])` without using
/// needs_drop explicitly. Types like HashMap, on the other hand, have to drop
/// values one at a time and should use this API.
///
///
/// # Examples
///
/// Here's an example of how a collection might make use of needs_drop:
///
/// ```ignore
/// #![feature(needs_drop)]
/// use std::{mem, ptr};
///
/// pub struct MyCollection<T> { /* ... */ }
///
/// impl<T> Drop for MyCollection<T> {
/// fn drop(&mut self) {
/// unsafe {
/// // drop the data
/// if mem::needs_drop::<T>() {
/// for x in self.iter_mut() {
/// ptr::drop_in_place(x);
/// }
/// }
/// self.free_buffer();
/// }
/// }
/// }
/// ```
#[inline]
#[unstable(feature = "needs_drop", issue = "41890")]
pub fn needs_drop<T>() -> bool {
unsafe { intrinsics::needs_drop::<T>() }
}

/// Creates a value whose bytes are all zero.
///
/// This has the same effect as allocating space with
Expand Down
3 changes: 1 addition & 2 deletions src/libstd/collections/hash/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ use alloc::heap::{allocate, deallocate};

use cmp;
use hash::{BuildHasher, Hash, Hasher};
use intrinsics::needs_drop;
use marker;
use mem::{align_of, size_of};
use mem::{align_of, size_of, needs_drop};
use mem;
use ops::{Deref, DerefMut};
use ptr::{self, Unique, Shared};
Expand Down
1 change: 1 addition & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@
#![feature(linkage)]
#![feature(macro_reexport)]
#![feature(needs_panic_runtime)]
#![feature(needs_drop)]
#![feature(never_type)]
#![feature(num_bits_bytes)]
#![feature(old_wrapping)]
Expand Down
7 changes: 4 additions & 3 deletions src/libstd/sys/redox/fast_thread_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
#![unstable(feature = "thread_local_internals", issue = "0")]

use cell::{Cell, UnsafeCell};
use intrinsics;
use mem;
use ptr;


pub struct Key<T> {
inner: UnsafeCell<Option<T>>,

Expand All @@ -37,7 +38,7 @@ impl<T> Key<T> {

pub fn get(&'static self) -> Option<&'static UnsafeCell<Option<T>>> {
unsafe {
if intrinsics::needs_drop::<T>() && self.dtor_running.get() {
if mem::needs_drop::<T>() && self.dtor_running.get() {
return None
}
self.register_dtor();
Expand All @@ -46,7 +47,7 @@ impl<T> Key<T> {
}

unsafe fn register_dtor(&self) {
if !intrinsics::needs_drop::<T>() || self.dtor_registered.get() {
if !mem::needs_drop::<T>() || self.dtor_registered.get() {
return
}

Expand Down
6 changes: 3 additions & 3 deletions src/libstd/sys/unix/fast_thread_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

use cell::{Cell, UnsafeCell};
use fmt;
use intrinsics;
use mem;
use ptr;

pub struct Key<T> {
Expand Down Expand Up @@ -44,7 +44,7 @@ impl<T> Key<T> {

pub fn get(&'static self) -> Option<&'static UnsafeCell<Option<T>>> {
unsafe {
if intrinsics::needs_drop::<T>() && self.dtor_running.get() {
if mem::needs_drop::<T>() && self.dtor_running.get() {
return None
}
self.register_dtor();
Expand All @@ -53,7 +53,7 @@ impl<T> Key<T> {
}

unsafe fn register_dtor(&self) {
if !intrinsics::needs_drop::<T>() || self.dtor_registered.get() {
if !mem::needs_drop::<T>() || self.dtor_registered.get() {
return
}

Expand Down