implement Deref for Box.

[sampsonwong]

fixes #18624

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

[Gankra]

Can you add a test for this?

[huonw]

@gankro, what sort of test are you thinking? (It seems to me that the only sort of test that distinguishes between these impls existing and them not is something like fn assert_deref<T: Deref<u8>>() { assert_deref::<Box<u8>>() } which isn't necessarily very interesting?)

(Metapoint: @sampsonwong and I wrote this together but have not had time to actually test if it compiles yet.)

[seanmonstar]

Doesn't derefing a box currently move the value out of the box?

[huonw]

It can, but is not required to, e.g. &*x is a non-moving dereference. (The Deref and DerefMut traits cover the times when moving is not necessary, having generic moving-out requires a new DerefMove trait.)

[japaric]

I have a few questions:

We already have built-in deref. By that I mean that let s: &[u8] = &*(box [0, 1]); already works even though Box<[T]> doesn't currently implement Deref<[T]>.

• After this PR, what does &*(box [0, 1]) do? Does it use the built-in deref, or does it go through the Deref trait? (It probably goes through the built-in deref, just like built-in indexing vs impl Index for [T] - just want to make sure that's the case)
• This:

impl<Sized? T> Deref<T> for Box<T> {
    fn deref(&self) -> &T { &**self }
}

looks like it could end in a infinite recursive call, since Deref allows the *thing sugar. (Unless the built-in deref kicks in first). Have you checked?

• Does:

fn foo(s: Box<str>) {
    let s: &mut str = &mut *s;
}

works because of DerefMut? It doesn't currently work, and it shouldn't work after this PR. We should add a compile-fail test for this, if there isn't one already.

[huonw]

After this PR, what does &*(box [0, 1]) do? Does it use the built-in deref, or does it go through the Deref trait? (It probably goes through the built-in deref, just like built-in indexing vs impl Index for [T] - just want to make sure that's the case)

looks like it could end in a infinite recursive call, since Deref allows the *thing sugar. (Unless the built-in deref kicks in first). Have you checked?

Yes, built-in operations override the overloaded ones (and I have checked).

works because of DerefMut? It doesn't currently work, and it shouldn't work after this PR. We should add a compile-fail test for this, if there isn't one already.

It gives

deref-box.rs:4:23: 4:30 error: mismatched types: expected `&mut str`, found `&mut str` (values differ in mutability)
deref-box.rs:4     let s: &mut str = &mut *s;
                                     ^~~~~~~

and this diagnostic is certainly a bug (not one that needs to be addressed here). Either way, even if it worked, I don't know of any safe thing one can do with a &mut str other than convert to a &str.

[japaric]

Yes, built-in operations override the overloaded ones (and I have checked).

Great, I asked because I read that you haven't tried compiling yet, but that probably was too long ago.

and this diagnostic is certainly a bug (not one that needs to be addressed here). Either way, even if it worked, I don't know of any safe thing one can do with a &mut str other than convert to a &str.

Sure, just curious if this opens the door to creating &mut str and I am wondering if we want to allow that, since that type is not useful, although it's probably harmless as well. BTW, if you change let s: &mut str = &mut *s to let s: &mut str = s.deref_mut() does that compile?.

[nikomatsakis]

@huonw a test that asserts that you can use Box<T> with a generic function that requires U:Deref<T> is certainly worthwhile, see e.g. the tests for deref from #18714

[Gankra]

Yes I'd like to see a test confirming that this actually lets you do what it should, if only because the interaction with the built-in impl is delicate.

[alexcrichton]

ping @sampsonwong would you be willing to add a few tests to this PR along the lines of the comments?

[sampsonwong]

updated with a test case

[alexcrichton]

ping @sampsonwong, looks like the tests failed because of some tab characters

[alexcrichton]

Closing due to inactivity, but feel free to reopen with tests fixed!