std::io: Modify some ReadBuf method signatures to return &mut Self
#95770
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nrc
added
T-libs-api
rust-highfive
added
the
S-waiting-on-review
joshtriplett
reviewed
May 3, 2022
This allows using `ReadBuf` in a builder-like style and to setup a `ReadBuf` and pass it to `read_buf` in a single expression, e.g., ``` // With this PR: reader.read_buf(ReadBuf::uninit(buf).assume_init(init_len))?; // Previously: let mut buf = ReadBuf::uninit(buf); buf.assume_init(init_len); reader.read_buf(&mut buf)?; ``` Signed-off-by: Nick Cameron <nrc@ncameron.org>
|
@bors r+ |
|
📌 Commit a3c30e4 has been approved by |
bors
added
S-waiting-on-bors
compiler-errors
added a commit
to compiler-errors/rust
that referenced
this pull request
Jun 9, 2022
std::io: Modify some ReadBuf method signatures to return `&mut Self` This allows using `ReadBuf` in a builder-like style and to setup a `ReadBuf` and pass it to `read_buf` in a single expression, e.g., ``` // With this PR: reader.read_buf(ReadBuf::uninit(buf).assume_init(init_len))?; // Previously: let mut buf = ReadBuf::uninit(buf); buf.assume_init(init_len); reader.read_buf(&mut buf)?; ``` r? `@sfackler` cc rust-lang#78485, rust-lang#94741
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (52ee2a2): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 28, 2022
std::io: migrate ReadBuf to BorrowBuf/BorrowCursor
This PR replaces `ReadBuf` (used by the `Read::read_buf` family of methods) with `BorrowBuf` and `BorrowCursor`.
The general idea is to split `ReadBuf` because its API is large and confusing. `BorrowBuf` represents a borrowed buffer which is mostly read-only and (other than for construction) deals only with filled vs unfilled segments. a `BorrowCursor` is a mostly write-only view of the unfilled part of a `BorrowBuf` which distinguishes between initialized and uninitialized segments. For `Read::read_buf`, the caller would create a `BorrowBuf`, then pass a `BorrowCursor` to `read_buf`.
In addition to the major API split, I've made the following smaller changes:
* Removed some methods entirely from the API (mostly the functionality can be replicated with two calls rather than a single one)
* Unified naming, e.g., by replacing initialized with init and assume_init with set_init
* Added an easy way to get the number of bytes written to a cursor (`written` method)
As well as simplifying the API (IMO), this approach has the following advantages:
* Since we pass the cursor by value, we remove the 'unsoundness footgun' where a malicious `read_buf` could swap out the `ReadBuf`.
* Since `read_buf` cannot write into the filled part of the buffer, we prevent the filled part shrinking or changing which could cause underflow for the caller or unexpected behaviour.
## Outline
```rust
pub struct BorrowBuf<'a>
impl Debug for BorrowBuf<'_>
impl<'a> From<&'a mut [u8]> for BorrowBuf<'a>
impl<'a> From<&'a mut [MaybeUninit<u8>]> for BorrowBuf<'a>
impl<'a> BorrowBuf<'a> {
pub fn capacity(&self) -> usize
pub fn len(&self) -> usize
pub fn init_len(&self) -> usize
pub fn filled(&self) -> &[u8]
pub fn unfilled<'this>(&'this mut self) -> BorrowCursor<'this, 'a>
pub fn clear(&mut self) -> &mut Self
pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
}
pub struct BorrowCursor<'buf, 'data>
impl<'buf, 'data> BorrowCursor<'buf, 'data> {
pub fn clone<'this>(&'this mut self) -> BorrowCursor<'this, 'data>
pub fn capacity(&self) -> usize
pub fn written(&self) -> usize
pub fn init_ref(&self) -> &[u8]
pub fn init_mut(&mut self) -> &mut [u8]
pub fn uninit_mut(&mut self) -> &mut [MaybeUninit<u8>]
pub unsafe fn as_mut(&mut self) -> &mut [MaybeUninit<u8>]
pub unsafe fn advance(&mut self, n: usize) -> &mut Self
pub fn ensure_init(&mut self) -> &mut Self
pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
pub fn append(&mut self, buf: &[u8])
}
```
## TODO
* ~~Migrate non-unix libs and tests~~
* ~~Naming~~
* ~~`BorrowBuf` or `BorrowedBuf` or `SliceBuf`? (We might want an owned equivalent for the async IO traits)~~
* ~~Should we rename the `readbuf` module? We might keep the name indicate it includes both the buf and cursor variations and someday the owned version too. Or we could change it. It is not publicly exposed, so it is not that important~~.
* ~~`read_buf` method: we read into the cursor now, so the `_buf` suffix is a bit weird.~~
* ~~Documentation~~
* Tests are incomplete (I adjusted existing tests, but did not add new ones).
cc rust-lang#78485, rust-lang#94741
supersedes: rust-lang#95770, rust-lang#93359
fixes rust-lang#93305
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 28, 2022
std::io: migrate ReadBuf to BorrowBuf/BorrowCursor
This PR replaces `ReadBuf` (used by the `Read::read_buf` family of methods) with `BorrowBuf` and `BorrowCursor`.
The general idea is to split `ReadBuf` because its API is large and confusing. `BorrowBuf` represents a borrowed buffer which is mostly read-only and (other than for construction) deals only with filled vs unfilled segments. a `BorrowCursor` is a mostly write-only view of the unfilled part of a `BorrowBuf` which distinguishes between initialized and uninitialized segments. For `Read::read_buf`, the caller would create a `BorrowBuf`, then pass a `BorrowCursor` to `read_buf`.
In addition to the major API split, I've made the following smaller changes:
* Removed some methods entirely from the API (mostly the functionality can be replicated with two calls rather than a single one)
* Unified naming, e.g., by replacing initialized with init and assume_init with set_init
* Added an easy way to get the number of bytes written to a cursor (`written` method)
As well as simplifying the API (IMO), this approach has the following advantages:
* Since we pass the cursor by value, we remove the 'unsoundness footgun' where a malicious `read_buf` could swap out the `ReadBuf`.
* Since `read_buf` cannot write into the filled part of the buffer, we prevent the filled part shrinking or changing which could cause underflow for the caller or unexpected behaviour.
## Outline
```rust
pub struct BorrowBuf<'a>
impl Debug for BorrowBuf<'_>
impl<'a> From<&'a mut [u8]> for BorrowBuf<'a>
impl<'a> From<&'a mut [MaybeUninit<u8>]> for BorrowBuf<'a>
impl<'a> BorrowBuf<'a> {
pub fn capacity(&self) -> usize
pub fn len(&self) -> usize
pub fn init_len(&self) -> usize
pub fn filled(&self) -> &[u8]
pub fn unfilled<'this>(&'this mut self) -> BorrowCursor<'this, 'a>
pub fn clear(&mut self) -> &mut Self
pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
}
pub struct BorrowCursor<'buf, 'data>
impl<'buf, 'data> BorrowCursor<'buf, 'data> {
pub fn clone<'this>(&'this mut self) -> BorrowCursor<'this, 'data>
pub fn capacity(&self) -> usize
pub fn written(&self) -> usize
pub fn init_ref(&self) -> &[u8]
pub fn init_mut(&mut self) -> &mut [u8]
pub fn uninit_mut(&mut self) -> &mut [MaybeUninit<u8>]
pub unsafe fn as_mut(&mut self) -> &mut [MaybeUninit<u8>]
pub unsafe fn advance(&mut self, n: usize) -> &mut Self
pub fn ensure_init(&mut self) -> &mut Self
pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
pub fn append(&mut self, buf: &[u8])
}
```
## TODO
* ~~Migrate non-unix libs and tests~~
* ~~Naming~~
* ~~`BorrowBuf` or `BorrowedBuf` or `SliceBuf`? (We might want an owned equivalent for the async IO traits)~~
* ~~Should we rename the `readbuf` module? We might keep the name indicate it includes both the buf and cursor variations and someday the owned version too. Or we could change it. It is not publicly exposed, so it is not that important~~.
* ~~`read_buf` method: we read into the cursor now, so the `_buf` suffix is a bit weird.~~
* ~~Documentation~~
* Tests are incomplete (I adjusted existing tests, but did not add new ones).
cc rust-lang#78485, rust-lang#94741
supersedes: rust-lang#95770, rust-lang#93359
fixes rust-lang#93305
workingjubilee
pushed a commit
to tcdi/postgrestd
that referenced
this pull request
Sep 15, 2022
std::io: migrate ReadBuf to BorrowBuf/BorrowCursor
This PR replaces `ReadBuf` (used by the `Read::read_buf` family of methods) with `BorrowBuf` and `BorrowCursor`.
The general idea is to split `ReadBuf` because its API is large and confusing. `BorrowBuf` represents a borrowed buffer which is mostly read-only and (other than for construction) deals only with filled vs unfilled segments. a `BorrowCursor` is a mostly write-only view of the unfilled part of a `BorrowBuf` which distinguishes between initialized and uninitialized segments. For `Read::read_buf`, the caller would create a `BorrowBuf`, then pass a `BorrowCursor` to `read_buf`.
In addition to the major API split, I've made the following smaller changes:
* Removed some methods entirely from the API (mostly the functionality can be replicated with two calls rather than a single one)
* Unified naming, e.g., by replacing initialized with init and assume_init with set_init
* Added an easy way to get the number of bytes written to a cursor (`written` method)
As well as simplifying the API (IMO), this approach has the following advantages:
* Since we pass the cursor by value, we remove the 'unsoundness footgun' where a malicious `read_buf` could swap out the `ReadBuf`.
* Since `read_buf` cannot write into the filled part of the buffer, we prevent the filled part shrinking or changing which could cause underflow for the caller or unexpected behaviour.
## Outline
```rust
pub struct BorrowBuf<'a>
impl Debug for BorrowBuf<'_>
impl<'a> From<&'a mut [u8]> for BorrowBuf<'a>
impl<'a> From<&'a mut [MaybeUninit<u8>]> for BorrowBuf<'a>
impl<'a> BorrowBuf<'a> {
pub fn capacity(&self) -> usize
pub fn len(&self) -> usize
pub fn init_len(&self) -> usize
pub fn filled(&self) -> &[u8]
pub fn unfilled<'this>(&'this mut self) -> BorrowCursor<'this, 'a>
pub fn clear(&mut self) -> &mut Self
pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
}
pub struct BorrowCursor<'buf, 'data>
impl<'buf, 'data> BorrowCursor<'buf, 'data> {
pub fn clone<'this>(&'this mut self) -> BorrowCursor<'this, 'data>
pub fn capacity(&self) -> usize
pub fn written(&self) -> usize
pub fn init_ref(&self) -> &[u8]
pub fn init_mut(&mut self) -> &mut [u8]
pub fn uninit_mut(&mut self) -> &mut [MaybeUninit<u8>]
pub unsafe fn as_mut(&mut self) -> &mut [MaybeUninit<u8>]
pub unsafe fn advance(&mut self, n: usize) -> &mut Self
pub fn ensure_init(&mut self) -> &mut Self
pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
pub fn append(&mut self, buf: &[u8])
}
```
## TODO
* ~~Migrate non-unix libs and tests~~
* ~~Naming~~
* ~~`BorrowBuf` or `BorrowedBuf` or `SliceBuf`? (We might want an owned equivalent for the async IO traits)~~
* ~~Should we rename the `readbuf` module? We might keep the name indicate it includes both the buf and cursor variations and someday the owned version too. Or we could change it. It is not publicly exposed, so it is not that important~~.
* ~~`read_buf` method: we read into the cursor now, so the `_buf` suffix is a bit weird.~~
* ~~Documentation~~
* Tests are incomplete (I adjusted existing tests, but did not add new ones).
cc rust-lang/rust#78485, rust-lang/rust#94741
supersedes: rust-lang/rust#95770, rust-lang/rust#93359
fixes #93305
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows using
ReadBufin a builder-like style and to setup aReadBufandpass it to
read_bufin a single expression, e.g.,r? @sfackler
cc #78485, #94741