-
Notifications
You must be signed in to change notification settings - Fork 14k
DSTify BytesContainer #18463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DSTify BytesContainer #18463
Conversation
|
I'm fine with removing |
|
Agreed with @alexcrichton. Can you update with the proposed change for |
|
Addressed Path ergonomics and squashed. @aturon re-r? |
|
Rebased. @alexcrichton or @aturon re-r? |
|
@aturon / @alexcrichton Could you tell bors to retry? The previous attempt failed because one of the bots was continuously failing an udp-related test. But, I think the bots are properly working now. |
|
@japaric done! |
- The `BytesContainer::container_into_owned_bytes` method has been removed
- Methods that used to take `BytesContainer` implementors by value, now take them by reference. In particular, this breaks some uses of Path:
``` rust
Path::new("foo") // Still works
path.join(another_path) -> path.join(&another_path)
```
[breaking-change]
---
Re: `container_into_owned_bytes`, I've removed it because
- Nothing in the whole repository uses it
- Takes `self` by value, which is incompatible with unsized types (`str`)
The alternative to removing this method is to split `BytesContainer` into `BytesContainer for Sized?` and `SizedBytesContainer: BytesContainer + Sized`, where the second trait only contains the `container_into_owned_bytes` method. I tried this alternative [in another branch](https://github.com/japaric/rust/commits/bytes) and it works, but it seemed better not to create a new trait for an unused method.
Re: Breakage of `Path` methods
We could use the idea that @alexcrichton proposed in #18457 (add blanket `impl BytesContainer for &T where T: BytesContainer` + keep taking `T: BytesContainer` by value in `Path` methods) to avoid breaking any code.
r? @aturon
cc #16918
…r-msg-improvement Improve error message for too new proc-macro server
BytesContainer::container_into_owned_bytesmethod has been removedBytesContainerimplementors by value, now take them by reference. In particular, this breaks some uses of Path:[breaking-change]
Re:
container_into_owned_bytes, I've removed it becauseselfby value, which is incompatible with unsized types (str)The alternative to removing this method is to split
BytesContainerintoBytesContainer for Sized?andSizedBytesContainer: BytesContainer + Sized, where the second trait only contains thecontainer_into_owned_bytesmethod. I tried this alternative in another branch and it works, but it seemed better not to create a new trait for an unused method.Re: Breakage of
PathmethodsWe could use the idea that @alexcrichton proposed in #18457 (add blanket
impl BytesContainer for &T where T: BytesContainer+ keep takingT: BytesContainerby value inPathmethods) to avoid breaking any code.r? @aturon
cc #16918