std::io::Take: Clarify & optimize BorrowedBuf::set_init usage.#155317
std::io::Take: Clarify & optimize BorrowedBuf::set_init usage.#155317rust-bors[bot] merged 4 commits intorust-lang:mainfrom
std::io::Take: Clarify & optimize BorrowedBuf::set_init usage.#155317Conversation
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
briansmith
force-pushed
the
b/take-opt
branch
2 times, most recently
from
ec92cd6 to
3c478cb
Compare
| unsafe { | ||
| // SAFETY: filled bytes have been filled | ||
| buf.advance(filled); | ||
| } |
There was a problem hiding this comment.
Doesn't moving this up cause limit to be wrong when we decide what to initialize below? It would need to be limit - filled now, unless I'm missing something?
Either way I think a comment would be good noting whether order matters (or not), and if my suspicion of breakage is right is accurate then we probably should try to come up with a test...
There was a problem hiding this comment.
Doesn't moving this up cause limit to be wrong when we decide what to initialize below? It would need to be limit - filled now, unless I'm missing something?
Yes, you are right. Thanks for catching this. I decided to remove the comment referencing ensure_init and I moved the advance call back below.
Either way I think a comment would be good noting whether order matters (or not),
I created a local variable named unfilled_before_advance to clarify this.
and if my suspicion of breakage is right is accurate then we probably should try to come up with a test...
I would like to write a test for this but I am not sure I will have time soon.
rustbot
removed
the
S-waiting-on-review
|
Reminder, once the PR becomes ready for a review, use |
rustbot
added
the
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
Eliminate `cursor` and `ibuf` as named variables, as their presence makes things more confusing.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@rustbot ready |
rustbot
added
S-waiting-on-review
rust-bors
Bot
added
S-waiting-on-bors
…lacrum `std::io::Take`: Clarify & optimize `BorrowedBuf::set_init` usage. Don't initialize `buf` if it was already initialized. Clarify safety comments. Move the `buf.advance()` call to make the initialization more like calling `buf.ensure_init()`, then clarify how the code here is an optimized variant of `ensure_init`.
…lacrum `std::io::Take`: Clarify & optimize `BorrowedBuf::set_init` usage. Don't initialize `buf` if it was already initialized. Clarify safety comments. Move the `buf.advance()` call to make the initialization more like calling `buf.ensure_init()`, then clarify how the code here is an optimized variant of `ensure_init`.
…uwer Rollup of 9 pull requests Successful merges: - #149624 (Fix requires_lto targets needing lto set in cargo) - #152443 (NVPTX: Drop support for old architectures and old ISAs) - #155317 (`std::io::Take`: Clarify & optimize `BorrowedBuf::set_init` usage.) - #155588 (Implement more traits for FRTs) - #155682 (Add boxing suggestions for `impl Trait` return type mismatches) - #155770 (Avoid misleading closure return type note) - #155818 (Convert attribute `FinalizeFn` to fn pointer) - #155829 (rustc_attr_parsing: use a `try {}` in `or_malformed`) - #155835 (couple of `crate_name` cleanups)
Rollup of 12 pull requests Successful merges: - #149624 (Fix requires_lto targets needing lto set in cargo) - #155317 (`std::io::Take`: Clarify & optimize `BorrowedBuf::set_init` usage.) - #155579 (Make Rcs and Arcs use pointer comparison for unsized types) - #155588 (Implement more traits for FRTs) - #155708 (Fix heap overflow in slice::join caused by misbehaving Borrow) - #155778 (Avoid Vec allocation in TyCtxt::mk_place_elem) - #151014 (std: sys: process: uefi: Add program searching) - #155682 (Add boxing suggestions for `impl Trait` return type mismatches) - #155770 (Avoid misleading closure return type note) - #155818 (Convert attribute `FinalizeFn` to fn pointer) - #155829 (rustc_attr_parsing: use a `try {}` in `or_malformed`) - #155835 (couple of `crate_name` cleanups)
Rollup merge of #155317 - briansmith:b/take-opt, r=Mark-Simulacrum `std::io::Take`: Clarify & optimize `BorrowedBuf::set_init` usage. Don't initialize `buf` if it was already initialized. Clarify safety comments. Move the `buf.advance()` call to make the initialization more like calling `buf.ensure_init()`, then clarify how the code here is an optimized variant of `ensure_init`.
3 participants
Don't initialize
bufif it was already initialized. Clarify safety comments.Move the
buf.advance()call to make the initialization more likecalling
buf.ensure_init(), then clarify how the code here is anoptimized variant of
ensure_init.