Downstream crates may implement `Copy` for `Box<_>`

[CAD97]

(This maybe could fit on urlo as well, but this is more about the language implications of "fixing" this error.)

The code generating this error has approximately this shape:

trait Op: Copy {
    fn op(self) -> u32;
}

impl<O> Op for &'_ O
where
    O: Op + Copy,
{
    fn op(self) -> u32 {
        O::op(*self)
    }
}

impl<'a, T: ?Sized> Op for &'a Box<T>
where
    &'a T: Op,
{
    fn op(self) -> u32 {
        <&'a T>::op(&**self)
    }
}

If we were allowed to rely on the fact that Box<_>: !Copy, then I believe these would be coherent implementations.

So, what reason is there that we can't rely on Box<_>: !Copy? What would it take to be able to rely on this? This is, of course, complicated by the fact that Box is #[fundamental]; doing the same thing for e.g. Arc and Rc works fine.

Currently, Box cannot be Copy because it implements Drop. Obviously, the standard library isn't going to remove the Drop impl for Box, but does the coherence checker know that? (In other words, is it a breaking or compatible change to remove an explicit Drop impl?)

Should we have T: Drop imply T: !Copy for coherence purposes? Alternatively, as we eventually get the ability to provide explicit "never impl" clauses (impl<T> !Copy for Box<T>), should we provide them for (how many) standard library Drop types?

(Yes, autoderef handles all of these impls when doing .op(), but not when taking impl Op.)

[CAD97]

I wrote a bit more in context on the issue tracker for the impacted crate.

[cuviper]

Downstream crates may implement Copy for Box<_>

This subject made me panic -- I thought you were going to report a way that you did implement Copy for Box in a downstream crate. But this is just an error message from rustc allowing the possibility...

[CAD97]

@cuviper

This subject made me panic

Whoops, sorry! Didn't mean to clickbait that hard.

[PoignardAzur]

@cuviper

This subject made me panic

I hope your brain is exception-safe!

[dhm]

It looks like the soundness hole of Pin started moving things in the direction of stdlib's explicit negative impls:

@nikomatsakis

I am not sure how far we will want to go, but I think at least the first three bullets. In particular, I would like for us to be able to do things like:

impl<T> !Copy for Box<T> { }

which in turn would mean you could do things like:

trait MyTrait { }
impl<T: Copy> MyTrait for T { }
impl<T> MyTrait for Box<T> { }

---

Tangentially, I recently stumbled upon a related error which is also quite annoying:

trait PrivateTrait {}

pub(whatever)
trait MyTrait { ... }

impl<T : PrivateTrait> MyTrait for T { ... }
impl<Referee : ?Sized> MyTrait for &'_ Referee { ... }

failing with downstream crates may implement PrivateTrait for &'_ _!

[PoignardAzur]

@CAD97

The code generating this error has approximately this shape :

trait Op: Copy {
    fn op(self) -> u32;
}

impl<O> Op for &'_ O
where
    O: Op + Copy,
{
    fn op(self) -> u32 {
        O::op(*self)
    }
}

impl<'a, T: ?Sized> Op for &'a Box<T>
where
    &'a T: Op,
{
    fn op(self) -> u32 {
        <&'a T>::op(&**self)
    }
}

As an aside, this code feels really really boilerplate-ish. I wonder if there shouldn't be a rule that goes something like "All traits should always automatically be implemented for & / Box / Rc / whatever, when possible, using their Deref type".

[Ixrec]

@PoignardAzur

I wonder if there shouldn't be a rule that goes something like "All traits should always automatically be implemented for & / Box / Rc / whatever, when possible, using their Deref type".

I believe &, Box and Rc are all #[fundamental] types for similar reasons, so in practice each trait author gets to decide which of those wrappers automatically get impl'd too.

[cuviper]

@PoignardAzur

I hope your brain is exception-safe!

I think so, but I may have safely forgotten otherwise...

[CAD97]

@PoignardAzur

As an aside, this code feels really really boilerplate-ish. I wonder if there shouldn't be a rule that goes something like "All traits should always automatically be implemented for & / Box / Rc / whatever, when possible, using their Deref type".

These impls aren't usually necessary; Deref coersion means that you could do x.op() for any of these types without an explicit forwarding impl. These impls are only required because I want a function taking impl Op by value to be able to take &_, &Box<_>, etc.

Note also that I'm not implementing the traits for Box<_>, but &Box<_> which makes things more fun. This is definitely a messy corner of the type system.

All of these impls can actually be made generic, though, via something along the lines of impl<D> Op for &D where D: Deref, for<'a> &'a D::Target: Op. This just leads to horrible error messages when trying to prove Op for a type that isn't, though.

---

The root question here is:

Is removing an explicit Drop impl (and relying on implicit Drop glue) a semver-compatible change?

I think the answer should be no, and then coherence can count impl Drop for T as also providing impl !Copy for T (if/when said impl is actually considered by coherence).

Note that gaining/losing drop glue would still not be a semver-incompatible change, just removing an explicit Drop impl.

(Side note: that fact, which I'm fairly certain is true today, makes the “make Drop bounds useful (by making it mean "has drop glue"” proposal problematic, because it makes "has drop glue" a public implementation detail of the type separate from explicit Copy or Drop impls.)

[steffahn]

@CAD97

implicit Drop glue

What exactly are you referring to? Is that a technical term?

[Ixrec]

Unfortunately this doesn't appear to be formally documented anywhere that I can find, but I believe "drop glue" refers to the code the compiler generates to invoke the right drop() calls in the right places. For example, if your struct S contains a Vec<u32> field but no explicit Drop impl, it will still have some "drop glue" for dropping the Vec<u32>.

A common and subtle point of confusion is that a type having "drop glue" is not the same thing as a type implementing the Drop trait. This is why T: Drop bounds don't do what anyone expects them to do, and changes to make this less confusing have been discussed in the past. That sort of change is what I believe the end of @CAD97 's post is referring to.

[steffahn]

Ah, so as I thought. I remembered reading about the needs_drop function, but didn’t hear it called “glue” before. Apparantly the people writing the documentation didn’t like “glue” when stabalizing that intrinsic (whose unstable API does use the word “glue”).

It would probably be beneficial to documentation, teaching, and the reference if there was some better terminology for “drop glue” that was used everywhere.

I think the corresponding terminology in C++ would be “trivially destructible”.

[cuviper]

A prominent example is String, which does not implement Drop itself, but its inner Vec<u8> does.

[RustyYato]

@CAD97

Is removing an explicit Drop impl (and relying on implicit Drop glue) a semver-compatible change?

this is already a breaking change, because you can do fn foo<T: Drop>() {}. Interstingly, adding a Drop impl is also a breaking change! (because const fns can't evaluate destructors)

@CAD97

I think the answer should be no, and then coherence can count impl Drop for T as also providing impl !Copy for T (if/when said impl is actually considered by coherence).

I don't think that Copy should be linked to Drop in this way, one reason why is that it means that you can't provide a nice api for ArrayVec (and other structures like it). ArrayVec can be Copy when T: Copy, but it must also implement Drop in order to drop the elements of the array.

Of course, removing this like would be disastrous, now that many libraries rely on Copy implying no Drop for soundness, and if we ever do remove this like (probably never), we would need another way to band drop glue.

[scottmcm]

@dhm

explicit negative impls

That's where I think things would go too. There are a few other silly errors than can pop up right now, like "you need to implement Clone for a mutable reference", and having the explicit negative impl there seems both good for improving the error message and as a place to write about why that's definitely not ok in the docs. Previous conversation:

Pondering: Negative trait promises

I was looking at this error message in the QotW thread: note: downstream crates may implement trait `std::clone::Clone` for type `&mut _` It made me wonder if impl !Trait would make sense just from a diagnostics and documentation perspective: errors like “the core crate has promised that &mut _ will never be Clone”, and a place in rustdoc to, say, explain why Option doesn’t have a Display impl. I would expect it to be semver-compatible to add a new negative impl for a trait you didn’t impl be…

[PoignardAzur]

@CAD97

These impls aren't usually necessary; Deref coersion means that you could do x.op() for any of these types without an explicit forwarding impl. These impls are only required because I want a function taking impl Op by value to be able to take &_ , &Box<_> , etc.

Note also that I'm not implementing the traits for Box<_> , but &Box<_> which makes things more fun . This is definitely a messy corner of the type system.

Just to be clear, is there a semantic meaning attached to these trait implementations, or are they a way to avoid as_ref and &** soup?

Because if it's the second, it still feels like a failure of the type system.

@Ixrec

I believe &, Box and Rc are all #[fundamental] types for similar reasons, so in practice each trait author gets to decide which of those wrappers automatically get impl'd too.

Oof, that RFC.

I must have read it three times by now and I still don't completely understand it.

[withoutboats]

This is a longstanding issue & I believe Niko has written about it on his blog at some point. The short answer is that it would be ideal to make coherence incorporate explicit negative impls into its reasoning, but that changing coherence in this way is a delicate matter in terms of backwards compatibility, correctness, etc.