Skip to content
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

Override default Write methods for cursor-like types #137107

Merged
merged 2 commits into from
Mar 7, 2025

Conversation

thaliaarchi
Copy link
Contributor

@thaliaarchi thaliaarchi commented Feb 16, 2025

Override the default io::Write methods for cursor-like types to provide more efficient versions.

Writes to resizable containers already write everything, so implement write_all and write_all_vectored in terms of those. For fixed-sized containers, cut out unnecessary error checking and looping for those same methods.

impl Write for T vectored all all_vectored fmt
&mut [u8] Y Y new
Vec<u8> Y Y new #137762
VecDeque<u8> Y Y new #137762
std::io::Cursor<&mut [u8]> Y new new
std::io::Cursor<&mut Vec<u8>> Y new new #137762
std::io::Cursor<Vec<u8>> Y new new #137762
std::io::Cursor<Box<[u8]>> Y new new
std::io::Cursor<[u8; N]> Y new new
core::io::BorrowedCursor<'_> new new new

Tracked in #136756.

Open questions

Is it guaranteed by Write::write_all that the maximal write is performed when not everything can be written? Its documentation describes the behavior of the default implementation, which writes until a 0-length write is encountered, thus implying that a maximal write is expected. In contrast, Read::read_exact declares that the contents of the buffer are unspecified for short reads. If it were allowed, these cursor-like types could bail on the write altogether if it has insufficient capacity.

@rustbot
Copy link
Collaborator

rustbot commented Feb 16, 2025

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 16, 2025
@thaliaarchi
Copy link
Contributor Author

r? libs, since you're probably busy on the release

All other methods in this file have #[inline] and these methods are very
similar to those of &[u8] which are already inlined here.
@thaliaarchi thaliaarchi force-pushed the io-optional-methods/cursors branch from a76c82c to 41bdd2b Compare February 28, 2025 01:10
@thaliaarchi
Copy link
Contributor Author

I've extracted the write_fmt changes as #137762.

@thaliaarchi
Copy link
Contributor Author

@joboet Friendly 2-week ping. Would you mind taking a look at this?

@joboet
Copy link
Member

joboet commented Mar 5, 2025

I've extracted the write_fmt changes as #137762.

Thank you, that helps!

@joboet Friendly 2-week ping. Would you mind taking a look at this?

Sure! Sorry for the delay, I've been a bit busy recently.

Anyway, this looks fine to me!
@bors r+

@bors
Copy link
Contributor

bors commented Mar 5, 2025

📌 Commit 41bdd2b has been approved by joboet

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 5, 2025
…rsors, r=joboet

Override default `Write` methods for cursor-like types

Override the default `io::Write` methods for cursor-like types to provide more efficient versions.

Writes to resizable containers already write everything, so implement `write_all` and `write_all_vectored` in terms of those. For fixed-sized containers, cut out unnecessary error checking and looping for those same methods.

| `impl Write for T`              | `vectored` | `all` | `all_vectored` | `fmt`   |
| ------------------------------- | ---------- | ----- | -------------- | ------- |
| `&mut [u8]`                     | Y          | Y     | new            |         |
| `Vec<u8>`                       | Y          | Y     | new            | rust-lang#137762 |
| `VecDeque<u8>`                  | Y          | Y     | new            | rust-lang#137762 |
| `std::io::Cursor<&mut [u8]>`    | Y          | new   | new            |         |
| `std::io::Cursor<&mut Vec<u8>>` | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Vec<u8>>`      | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Box<[u8]>>`    | Y          | new   | new            |         |
| `std::io::Cursor<[u8; N]>`      | Y          | new   | new            |         |
| `core::io::BorrowedCursor<'_>`  | new        | new   | new            |         |

Tracked in rust-lang#136756.

# Open questions

Is it guaranteed by `Write::write_all` that the maximal write is performed when not everything can be written? Its documentation describes the behavior of the default implementation, which writes until a 0-length write is encountered, thus implying that a maximal write is expected. In contrast, `Read::read_exact` declares that the contents of the buffer are unspecified for short reads. If it were allowed, these cursor-like types could bail on the write altogether if it has insufficient capacity.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
Rollup of 29 pull requests

Successful merges:

 - rust-lang#135733 (Implement `&pin const self` and `&pin mut self` sugars)
 - rust-lang#137107 (Override default `Write` methods for cursor-like types)
 - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion)
 - rust-lang#137327 (Undeprecate env::home_dir)
 - rust-lang#137358 (Match Ergonomics 2024: add context and examples to the unstable book)
 - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden))
 - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var)
 - rust-lang#137612 (Update bootstrap to edition 2024)
 - rust-lang#137637 (Check dyn flavor before registering upcast goal on wide pointer cast in MIR typeck)
 - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums)
 - rust-lang#137685 (self-contained linker: conservatively default to `-znostart-stop-gc` on x64 linux)
 - rust-lang#137744 (Re-add `Clone`-derive on `Thir`)
 - rust-lang#137758 (fix usage of ty decl macro fragments in attributes)
 - rust-lang#137764 (Ensure that negative auto impls are always applicable)
 - rust-lang#137772 (Fix char count in `Display` for `ByteStr`)
 - rust-lang#137798 (ci: use ubuntu 24 on arm large runner)
 - rust-lang#137802 (miri native-call support: all previously exposed provenance is accessible to the callee)
 - rust-lang#137805 (adjust Layout debug printing to match the internal field name)
 - rust-lang#137808 (Do not require that unsafe fields lack drop glue)
 - rust-lang#137820 (Clarify why InhabitedPredicate::instantiate_opt exists)
 - rust-lang#137825 (Provide more context on resolve error caused from incorrect RTN)
 - rust-lang#137827 (Add timestamp to unstable feature usage metrics)
 - rust-lang#137832 (Fix crash in BufReader::peek())
 - rust-lang#137910 (Improve error message for `AsyncFn` trait failure for RPIT)
 - rust-lang#137920 (interpret/provenance_map: consistently use range_is_empty)
 - rust-lang#138038 (Update `compiler-builtins` to 0.1.151)
 - rust-lang#138046 (trim channel value in `get_closest_merge_commit`)
 - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test)
 - rust-lang#138053 (Increase the max. custom try jobs requested to `20`)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 5, 2025
…rsors, r=joboet

Override default `Write` methods for cursor-like types

Override the default `io::Write` methods for cursor-like types to provide more efficient versions.

Writes to resizable containers already write everything, so implement `write_all` and `write_all_vectored` in terms of those. For fixed-sized containers, cut out unnecessary error checking and looping for those same methods.

| `impl Write for T`              | `vectored` | `all` | `all_vectored` | `fmt`   |
| ------------------------------- | ---------- | ----- | -------------- | ------- |
| `&mut [u8]`                     | Y          | Y     | new            |         |
| `Vec<u8>`                       | Y          | Y     | new            | rust-lang#137762 |
| `VecDeque<u8>`                  | Y          | Y     | new            | rust-lang#137762 |
| `std::io::Cursor<&mut [u8]>`    | Y          | new   | new            |         |
| `std::io::Cursor<&mut Vec<u8>>` | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Vec<u8>>`      | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Box<[u8]>>`    | Y          | new   | new            |         |
| `std::io::Cursor<[u8; N]>`      | Y          | new   | new            |         |
| `core::io::BorrowedCursor<'_>`  | new        | new   | new            |         |

Tracked in rust-lang#136756.

# Open questions

Is it guaranteed by `Write::write_all` that the maximal write is performed when not everything can be written? Its documentation describes the behavior of the default implementation, which writes until a 0-length write is encountered, thus implying that a maximal write is expected. In contrast, `Read::read_exact` declares that the contents of the buffer are unspecified for short reads. If it were allowed, these cursor-like types could bail on the write altogether if it has insufficient capacity.
@thaliaarchi
Copy link
Contributor Author

@bors rollup

@bors
Copy link
Contributor

bors commented Mar 6, 2025

@thaliaarchi: 🔑 Insufficient privileges: not in try users

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 7, 2025
…rsors, r=joboet

Override default `Write` methods for cursor-like types

Override the default `io::Write` methods for cursor-like types to provide more efficient versions.

Writes to resizable containers already write everything, so implement `write_all` and `write_all_vectored` in terms of those. For fixed-sized containers, cut out unnecessary error checking and looping for those same methods.

| `impl Write for T`              | `vectored` | `all` | `all_vectored` | `fmt`   |
| ------------------------------- | ---------- | ----- | -------------- | ------- |
| `&mut [u8]`                     | Y          | Y     | new            |         |
| `Vec<u8>`                       | Y          | Y     | new            | rust-lang#137762 |
| `VecDeque<u8>`                  | Y          | Y     | new            | rust-lang#137762 |
| `std::io::Cursor<&mut [u8]>`    | Y          | new   | new            |         |
| `std::io::Cursor<&mut Vec<u8>>` | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Vec<u8>>`      | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Box<[u8]>>`    | Y          | new   | new            |         |
| `std::io::Cursor<[u8; N]>`      | Y          | new   | new            |         |
| `core::io::BorrowedCursor<'_>`  | new        | new   | new            |         |

Tracked in rust-lang#136756.

# Open questions

Is it guaranteed by `Write::write_all` that the maximal write is performed when not everything can be written? Its documentation describes the behavior of the default implementation, which writes until a 0-length write is encountered, thus implying that a maximal write is expected. In contrast, `Read::read_exact` declares that the contents of the buffer are unspecified for short reads. If it were allowed, these cursor-like types could bail on the write altogether if it has insufficient capacity.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
…kingjubilee

Rollup of 12 pull requests

Successful merges:

 - rust-lang#136667 (Revert vita's c_char back to i8)
 - rust-lang#136780 (std: move stdio to `sys`)
 - rust-lang#137107 (Override default `Write` methods for cursor-like types)
 - rust-lang#137363 (compiler: factor Windows x86-32 ABI impl into its own file)
 - rust-lang#137528 (Windows: Fix error in `fs::rename` on Windows 1607)
 - rust-lang#137537 (Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do)
 - rust-lang#137777 (Specialize `OsString::push` and `OsString as From` for UTF-8)
 - rust-lang#137832 (Fix crash in BufReader::peek())
 - rust-lang#137904 (Improve the generic MIR in the default `PartialOrd::le` and friends)
 - rust-lang#138115 (Suggest typo fix for static lifetime)
 - rust-lang#138125 (Simplify `printf` and shell format suggestions)
 - rust-lang#138129 (Stabilize const_char_classify, const_sockaddr_setters)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136667 (Revert vita's c_char back to i8)
 - rust-lang#137107 (Override default `Write` methods for cursor-like types)
 - rust-lang#137777 (Specialize `OsString::push` and `OsString as From` for UTF-8)
 - rust-lang#137832 (Fix crash in BufReader::peek())
 - rust-lang#137904 (Improve the generic MIR in the default `PartialOrd::le` and friends)
 - rust-lang#138115 (Suggest typo fix for static lifetime)
 - rust-lang#138125 (Simplify `printf` and shell format suggestions)
 - rust-lang#138129 (Stabilize const_char_classify, const_sockaddr_setters)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 458095a into rust-lang:master Mar 7, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 7, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
Rollup merge of rust-lang#137107 - thaliaarchi:io-optional-methods/cursors, r=joboet

Override default `Write` methods for cursor-like types

Override the default `io::Write` methods for cursor-like types to provide more efficient versions.

Writes to resizable containers already write everything, so implement `write_all` and `write_all_vectored` in terms of those. For fixed-sized containers, cut out unnecessary error checking and looping for those same methods.

| `impl Write for T`              | `vectored` | `all` | `all_vectored` | `fmt`   |
| ------------------------------- | ---------- | ----- | -------------- | ------- |
| `&mut [u8]`                     | Y          | Y     | new            |         |
| `Vec<u8>`                       | Y          | Y     | new            | rust-lang#137762 |
| `VecDeque<u8>`                  | Y          | Y     | new            | rust-lang#137762 |
| `std::io::Cursor<&mut [u8]>`    | Y          | new   | new            |         |
| `std::io::Cursor<&mut Vec<u8>>` | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Vec<u8>>`      | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Box<[u8]>>`    | Y          | new   | new            |         |
| `std::io::Cursor<[u8; N]>`      | Y          | new   | new            |         |
| `core::io::BorrowedCursor<'_>`  | new        | new   | new            |         |

Tracked in rust-lang#136756.

# Open questions

Is it guaranteed by `Write::write_all` that the maximal write is performed when not everything can be written? Its documentation describes the behavior of the default implementation, which writes until a 0-length write is encountered, thus implying that a maximal write is expected. In contrast, `Read::read_exact` declares that the contents of the buffer are unspecified for short reads. If it were allowed, these cursor-like types could bail on the write altogether if it has insufficient capacity.
@thaliaarchi thaliaarchi deleted the io-optional-methods/cursors branch March 7, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants