Edition idea: Should any type that has drop glue be considered to impl Drop?

#1

After upgrading rustc I got this warning:

Drop bounds do not really accomplish anything. A type may have compiler-generated drop glue without implementing the Drop trait itself. The Drop trait also only has one method, Drop::drop , and that function is by fiat not callable in user code. So there is really no use case for using Drop in trait bounds.`

However, a reliable Drop bound would actually be super useful for containers to be able to have smarter implementations based on whether they have to worry about Drop. There is precedent for this in C++ -- the type trait is_trivially_destructible exists exactly for this purpose. If a C++ type doesn't have an explicitly defined destructor, but has members that have explicit destructors, is_trivially_destructible is false.

AFAICT this inconsistency only exists because types that don't explicitly impl Drop are not considered to impl Drop, even if they have members that are impl Drop. Is there any reason to not just consider any type that needs code to execute when it drops to be impl Drop? I believe this would be a edition'able change since using Drop as a bound currently has no effect. rustfix would just need to remove all Drop bounds when porting code from old to new editions.

Rust seems to have the workaround function std::mem::needs_drop::<T> -- but this will still cause a runtime branch in debug builds. It's also inconsistent with how the language deals with all other traits, e.g. you don't write std::is_clone::<T>(), std::is_default::<T>(), etc. And finally what lead me down this road is lack of addressing this at the type level prevents you from being able to write stack containers that are either Copy or Drop depending on the type they contain. AFAICT I will need to fork my container code, once for use with types where I can be sure memcpying the container is safe and one not.

1 Like
#2

Previously

And I know this was discussed in the context of const Trait (as we would like const Drop to be useful to mean "can be dropped in a const context," even including drop glue), but I can't find where it was discussed.

The lint for using Drop as a trait bound does seem a really good candidate for upgrading to an error in edition 2021 in preparation for "fixing" the bound.

Unfortunately I don't think Drop meaning "has nontrivial drop glue" is going to ever be the case, because we want const Drop to be "has const drop glue," which includes "has no drop glue."

1 Like
#3

what about having auto trait NonTrivialDrop, which is implemented for every type that has either Drop or contains a type with Drop to allow specialization based on Drop?

#4

Hmm... I can buy the idea of that every type should implement Drop since should be able to call drop on any object. In that case maybe NonTrivialDrop should be a separate trait, that is automatically implemented by any type that has an explicit impl for Drop or contains a member that does. That would be orthogonal from const Trait which I imagine would just mean "a type that has an implementation of the trait where all of the trait methods are const."

Edit: @Mokuz beat me to it by seconds :wink:

2 Likes
#5

Can you elaborate on this? Without specialization or negative trait bounds, a 'reliable Drop bound' would only allow you to require that a type have drop glue. That is, T: ReliableDrop would hold for String and MyWrapper(String), but not for u8.

Surprisingly, there is actually a (quite obscure) use for Drop bounds currently:

trait PreventExplicitDropImpl {}
impl<T: Drop> PreventExplicitDropImpl for T {}

struct Foo;
impl PreventExplicitDropImpl for Foo {}

impl Drop for Foo {
    fn drop(&mut self) {}
}

This code causes a compile error if there is an explicit Drop impl for Foo, but does not error if Foo just has drop glue.

This is actually used by the pin-project crate, which needs to be able to prevent users from writing a manual Drop impl (pin-project does not need to prevent drop glue).

Thus, changing the behavior of Drop would be a breaking change.

If you want to be able to express std::mem::needs_drop<T> via the trait system (with all the limitations that currently entails), it's actually possible on Nightly via const-generics (playgroud):

#![feature(const_generics)]
#![feature(const_evaluatable_checked)]

trait NeedsDrop {}

impl<T> NeedsDrop for T where Bool<{std::mem::needs_drop::<T>()}>: True {}

struct Bool<const B: bool>;
trait True {}
impl True for Bool<true> {}

fn assert_needs_drop<T: NeedsDrop>() where Bool<{std::mem::needs_drop::<T>()}>: True {}

struct WithDropGlue {
    val: String
}

fn main() {
    assert_needs_drop::<String>();
    assert_needs_drop::<WithDropGlue>();
    assert_needs_drop::<u8>();
}

It currently requires a duplicated where clause on assert_needs_drop, but I believe that should be addressed in coming changes to const-generics.

8 Likes
#6

FWIW, it's worth noting that mem::needs_drop is sadly allowed to have false positives (but no false negatives!), as mentioned in its docs.

Personally, the most intuitive definition for this bound would be something along the lines of:

unsafe auto trait NoDropGlue {}
impl<T : Drop> !NoDropGlue for T {}

but generic negative impls being as they are (let's say they're weird), this doesn't work :disappointed:

#7

(Off topic: why doesn't it just always emit a Drop impl? That seems like it would accomplish the same goal, and it has to for pinned drop anyway.)

Dah, that's unfortunate that you can observe this for negative reasoning. All the more reason to make the warning an error in the 2021 edition, IMHO. (If everything works as intended, then the proc macro "should" emit tokens with edition2018 hygiene and get the current behavior still.)

#8

The presence of a Drop impl prevents moving out of fields. If the user didn't want a Drop impl, then providing one would unnecessarily prevent moving out of fields, making the struct much less ergonmic.

I strongly disagree. This would require pin-project to permanently emit tokens using the 2018 edition (which AFAIK is only possible by doing an awful hack involving a separate crate compiled with the 2018 edition).

Additionally, I am unconvinced that changing a Drop bound to mean 'has drop glue' is desirable. While Drop is certainly a 'special' trait, T: Trait currently always implies that there's actually an impl of Trait for T. I believe that special-casing Drop in this way woud make the trait system even harder to understand.

Except for dropping things in const contexts, I believe that the NeedsDrop trait I demonstrated is exactly equivalent to the behavior you're proposing for Drop bounds. While it does rely on nightly features, I believe that there's active work on #![feature(const_evaluatable_checked)]

2 Likes
#9

I actually have, as an implementation-detail, in my WIP rust implementation lccc, the negative of that, TrivialDestruction, as an unstable auto trait. In theory, with OIBIT, you could do the same thing, like so

pub unsafe auto trait TrivialDestruction{}
impl<'a,T: ?Sized> TrivialDestruction for &'a T{}
impl<'a,T: ?Sized> TrivialDestruction for &'a mut T{}
impl<T: ?Sized> TrivialDestruction for *mut T{}
impl<T: ?Sized> TrivialDestruction for *const T{}
impl<T: Drop> !TrivialDestruction for T{}
impl<T> TrivialDestruction for ManuallyDrop<T>{}

With the appropriate guards to prevent user implementations (like my stdlib does for ManuallyDrop, since ManuallyDrop does, in fact, have TrivialDestruction).

If we had addative auto traits (such that it is auto-implemented if at least one field implements the trait), then you could also very easily write the inverse case (Disclaimer: I am not proposing addative auto traits here). Otherwise, would negative trait bounds work here with a negative auto trait?

#10

(... How is moving fields out of a pin-projected type sound? Oh, actually, I suppose if it's not been pinned yet, and you can't move out behind a pin anyway.)

To be clear, I'm for T: Drop always holds, because you can always drop T, not for it to mean the type has nontrivial drop glue, or that it directly implements Drop. I realize the issues that has with existing semantics, though.

I'll be sad if this means we can't drop generic types by value in const code, though. (Since const Drop would then necessarily be "has a direct impl of const Drop and not "has const drop glue," so T: const Drop wouldn't even imply const drop glue)

#11

That's right. For example, if I have #[pin_project] MyStruct { future: #[pin] MyFuture }, I should be able to move out of MyStruct.future when I have ownership of MyStruct (without a wrapping Pin).

I agree that we should be able to drop generic types in const code. However, I don't think that changing the meaning of a Drop bound is the right way to go about accomplishing that.

#12

My experience in C++ is that is_trivially_destructible is less useful than you might expect, because in most cases the compiler can easily optimise away calls.

In general (and obviously I don't have proof of this, but I did write a lot of C++ code back in the day), my feeling is back in the C++03 days there was a temptation to try hard to stop compilers having to instansiate functions that wouldn't do anything, but it turned out there are just too many cases (and it's easy to get wrong), so nowadays most people just trust the compiler to optimise them away.

#13

I think in Rust it's more of an issue because Drop and Copy are mutually exclusive, so this bubbles up to the semantic level and actually prevents you from expressing things, rather than just being a performance issue.

#14

I just realized that we don't want to expand Drop to include drop glue types (without including all types, always) for one really big reason nobody brought up yet: it's a (new) encapsulation violation, the same way Send/Sync are, but much more likely to trip and be an accidental breaking change.

Technically, all OIBITS (auto traits, and I probably got that acronym wrong) are encapsulation breaking. The presence of a field impacts the public API of your type in an observable manner.

If you are currently Sync and add a field that isn't Sync, you're no longer Sync, and that's a breaking change. That's something we live with because it's relatively uncommon enough and easy enough to reason about.

If Drop includes drop glue and not types with trivial drop, then removing a field that has drop glue, even as an unneeded bit of legacy implementation details. Plus, if you have a single member with a noöp implementation of Drop, does the compiler optimize your type to have a trivial drop and no drop glue? I sure hope it's allowed to!

Beyond that, IIUC it's generally considered that switching from an explicit Drop impl to just owning members that own and drop their resources is both nonbreaking and a positive change. @Aaron1011 shows that apparently this change is observable, and thus it's a breaking change to remove an explicit Drop implementation :frowning_face:

trait A {
    fn uhoh() {}
}

impl<T: Drop> A for T {}

fn main() {
//  regex::Regex::uhoh(); // error: Regex doesnt directly impl Drop
    Box::<()>::uhoh(); // not an error: Box directly impls Drop
}
#15

that's the mentioned:

And when I mentioned weird it's the issue that in order for the trait solver to prove that something is NoDropGlue / TrivialDestruction, it needs to prove / find a "non-impl" of T : Drop. And a lack of such impl is not a proof of "non-impl" (that's one of the core things about coherence ~ that adding trait impls should not cause breakage at a distance; for the case of Drop however, it already causes breakage when at least a field is pub, and the Drop impl cannot be specialized, so special-casing that one could be fine)

1 Like
#16

I don't see why negative trait bounds need to come into it. I approve of @InfernoDeity's approach where there is a regular trait (well, an OIBIT) that indicates that the type has no drop glue; this trait does not have to interact with the Drop trait at all, unless we want the trait system to know that TrivialDrop + Drop implies false, and I think that we don't, since as others have mentioned Drop should be implemented for all types since drop can be called on anything. The presence of drop glue would then be indicated by !TrivialDrop.

Now as for where the impls come from, I think the normal auto trait rules get it right, but the compiler already has a dropck pass so it can just provide magical impls if there are edge cases. (Unless this causes a cycle between trait solving and dropck?)

#17

I don't understand that part: the moment there is an impl Drop on some type, that type (and any containing type!) ceases to feature "trivial drop" and now has drop glue, so it can't keep implementing TrivialDrop / NoDropGlue.

While that impl !NoDropGlue could be done for each and every case of impl Drop by the compiler, this is a situation where the usual approach is to be generic over that last case, hence the negative_impls usage (that is, the whole thing can be implemented differently with special compiler / lang support, but that's only used when the current abstractions fail to express that).

  • But, as I mentioned, the negative_impl approach does not work:

    unsafe auto trait NoDropGlue {}
impl<T : Drop> !NoDropGlue for T {}

fn impls_NoDropGlue<T : ?Sized>() where
    T : NoDropGlue,
{}

const _: () = {
    // ERROR: the trait bound `u8: NoDropGlue` is not satisfied
    let _ = impls_NoDropGlue::<u8>;
};

So we need some form of language support to handle that part (I suspect that making the Drop trait #[fundamental] would suffice to appease the proof obligations, but that would be way too overkill and problematic).

This last point is interesting: if users get the power to have <T : NoDropGlue> generics, this means that drop glue becomes a type-level observable part of the API of types, even for types having only private types! This means that adding drop glue to a type would be a breaking change, which means the std lib, for instance, would have to add many preemptive dummy Drop impls / drop glues so as to keep the option open.

From there, we observe we have, yet again, hit the common problem of auto-traits (vs., for instance, derive(Copy)).

So I think that type-level observability of NoDropGlue ought to be opt-in (we could see it as a weaker version of Copy).

All in all, I realize the current design with a mem::needs_drop() function is actually really good :slightly_smiling_face:

#18

If it's a magical impl, then that's fine, the determination of whether TrivialDrop is implemented depends on drop check which depends on the nonexistence of an impl Drop. You are right that auto traits alone don't make this work correctly. Or maybe drop check is not the right term - it should be as simple as the conjunction of "all fields are TrivialDrop" and "this type does not impl Drop", although the latter proof obligation may be literally a negative impl, in particular if you have funny conditional drop impls like impl<T: Foo> Drop for Bar<T> where T: Foo is being supplied by a downstream crate. It would be nice to be conservative here and say that TrivialDrop is disabled if there are any Drop impls at all, including conditional impls, but maybe that has unforeseen issues.

One solution for that issue is to allow impl !TrivialDrop for T as with other auto traits, to explicitly opt out of signalling trivial drop even if there is no drop glue.

I can see the argument for making this opt-in, though. I don't have a good sense for when you would want to use this outside unsafe code that is messing with ManuallyDrop types.

1 Like
#19

This is already the case for const-constructable types, as const contexts cannot run drop glue. A type being const-destructable is currently exactly that it has a trivial drop.

1 Like
#20

It's a lot more useful nowadays I'd say in conjunction with the significantly improved C++17 / C++20 constexpr and consteval, as it can be directly used to have entirely different (and not necessarily uniformly compatible) branches for handling things that are and are not trivially destructible, where only the "currently true" branch is evaluated by the compiler at all,