`alloc` should require `T: !Drop` or be `unsafe`

[Pr0methean]

To prevent users from unwittingly allocating structs that they aren't aware need to have their Drop methods run, the alloc* and try_alloc* methods should add the type constraint T: !Drop, and have pub unsafe fn *_unchecked versions for use when the user can't meet that constraint.

It might also be useful to have a wrapper which had a Vec or LinkedList<*mut dyn Any> droppables; then, the unsafe methods would be replaced with alloc_*_droppable methods that pushed the allocated pointer onto self.droppables, and then the wrapper's impl Drop would call self.droppables.into_iter().rev().for_each(mem::drop_in_place).

[bluurryy]

To prevent users from unwittingly allocating structs that they aren't aware need to have their Drop methods run, the alloc* and try_alloc* methods should add the type constraint T: !Drop, and have pub unsafe fn *_unchecked versions for use when the user can't meet that constraint.

There is no way to say !core::mem::needs_drop::<T> in a trait bound yet AFAIK (in stable). Best you could do is T: Copy. It would be strange making them unsafe when not-dropping is safe and is not marked as unsafe in the std library.

It might also be useful to have a wrapper which had a Vec or LinkedList<*mut dyn Any> droppables; then, the unsafe methods would be replaced with alloc_*_droppable methods that pushed the allocated pointer onto self.droppables, and then the wrapper's impl Drop would call self.droppables.into_iter().rev().for_each(mem::drop_in_place).

There is a crate like that called rodeo that does that. A neat thing about rodeo is that it detects whether the type needs drop and only adds it to the drop list if so.

[Pr0methean]

There is no way to say !core::mem::needs_drop:: in a trait bound yet AFAIK (in stable). Best you could do is T: Copy. It would be strange making them unsafe when not-dropping is safe and is not marked as unsafe in the std library.

I guess one way to achieve that conservatively would be to use the static_assertions crate:

if (core::mem::needs_drop::<T>()) {
    static_assertions::const_assert!(false, "This method isn't safe to use with types that need to be explicitly dropped");
}

[bluurryy]

It would work with a static assertion. But it would be implemented like so:

struct AssertNoDrop<T: ?Sized>(PhantomData<T>);
impl<T: ?Sized> AssertNoDrop<T> {
    const ASSERT_NO_DROP: () = assert!(!core::mem::needs_drop::<T>(), "T must not need drop");
}

const fn must_be_no_drop<T: ?Sized>() {
    let _ = AssertNoDrop::<T>::ASSERT_NO_DROP;
}

Unfortunately the developer experience of such assertions is really bad. The resulting error message only points at the assert!. Those errors are also only emitted when building the program. So cargo check does not tell you that there is an error.

[VorpalBlade]

std::mem::forget is safe, not running Drop is perfectly valid. So I really don't get the motivation for this issue.

[bluurryy]

So I really don't get the motivation for this issue.

I don't think its a bad idea to be more explicit that you're leaking memory if there is way to assert that you don't. So you don't accidentally leak.

I think if such a feature would be added it'd make the most sense like this:

fn alloc<T: NoDrop>(&self, value: T) -> &mut T;
fn alloc_leak<T>(&self, value: T)  -> &mut T;

(NoDrop does not exist, Copy works for the most part though)

EDIT: changed alloc_forget to alloc_leak

[fitzgen]

As @VorpalBlade pointed out, not running Drop is safe, and in fact sometimes it is fine and even desired to leak or what have you.

FWIW, I am working on consolidating some API breaking changes I've long wanted to make in a new branch to get ready for a new major release. I don't want to introduce a new leak vs no-leak on 3.* because that will be a whole 'nother combinatorial blow up of methods. I'm investigating using a builder-style API for non-default allocation options in the next major version, and that should be able to support this kind of static assertion. It will also support a drop list, primarily for use with pinning, but should work well enough for this kind of thing too.