std::io::copy should take Read and Write traits by value

[est31]

This was suggested by @alexcrichton way back in 2015:

mod std::io {
    fn copy<R: Read, W: Write>(r: R, w: W) -> io::Result<u64> { ... }
}

but it wasn't done as it regressed 3 (root) crates in a crater run. Still, ideally if the API breakage policy is ever revisited, it would be better to take Read and Write by-value, as a type impl'ing Read transmits this property to &mut references of it.

@rustbot label T-libs-api rust-2-breakage-wishlist

[rustbot]

Error: Label rust-2-breakage-wishlist can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[est31]

FTR this is the error I'm getting if I modify the copy function on a local toolchain based off current master and try to build tar 0.3.1:

error[E0382]: use of moved value: `self`
   --> src/entry.rs:134:48
    |
134 |             if try!(io::copy(self, &mut f)) != self.size {
    |                              ----              ^^^^^^^^^ value used here after move
    |                              |
    |                              value moved here
    |
    = note: move occurs because `self` has type `&mut EntryFields<'_>`, which does not implement the `Copy` trait

error[E0382]: borrow of moved value: `self`
   --> src/entry.rs:145:28
    |
97  |     fn _unpack(&mut self, dst: &Path) -> io::Result<()> {
    |                --------- move occurs because `self` has type `&mut EntryFields<'_>`, which does not implement the `Copy` trait
...
133 |         try!(fs::File::create(dst).and_then(|mut f| {
    |                                             ------- value moved into closure here
134 |             if try!(io::copy(self, &mut f)) != self.size {
    |                              ---- variable moved due to use in closure
...
145 |         if let Ok(mtime) = self.header.mtime() {
    |                            ^^^^^^^^^^^^^^^^^^^ value borrowed here after move

So the borrow checker improvements in 2018-ish haven't fixed the regression here.

tar 0.3.4 is also affected, but latest 0.4.33 is not.

I'm closing this as with current policies this can't be changed.

@rustbot label T-libs-api

[est31]

For the libs team meeting, see #111063 with crater runs for the suggested change as well as my analysis of the results.

[dtolnay]

The proposed change breaks reborrows. For example in the following code:

• https://github.com/Byron/gitoxide/blob/24dcba44abbea979aec609274d4327ab61f18048/gix-config/src/file/includes/mod.rs#L88-L102

the error is:

• Make std::io::copy take Read and Write by value instead of by reference #111063 (comment)

because against the current copy signature, the following is a reborrow of buf:

let buf: &mut Vec<u8> = ...;
io::copy(..., buf)?;  // IMPLICIT REBORROW equivalent to `&mut *buf`

// buf remains usable here

whereas in the proposed new copy signature it is a move of buf.

As far as I know, there is no inherent reason it has to be this way. If you have fn old_copy<W: ?Sized + Write>(w: &mut W) and fn new_copy<W: Write>(w: W), then old_copy::<Vec<u8>> and new_copy::<&mut Vec<u8>> have the same signature. Both of them take an argument of type &mut Vec<u8>. One could imagine wanting Rust's reborrow behavior to be identical between these two identical signatures. I don't currently know any reason that it would be a breaking change to perform reborrowing for both.

For now the crater breakages are beyond where I think we can push this change through. But if a future smarter borrow checker is able to do reborrows for an argument of type &mut T even when the formal parameter's type is a generic type, then I would love to reconsider this change at that point.

@rfcbot fcp close

[rfcbot]

Team member @dtolnay has proposed to close this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔