BufWriter: Provide into_raw_parts #79705
Merged
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
|
r? @dtolnay (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
If something goes wrong, one might want to unpeel the layers of nested Writers to perform recovery actions on the underlying writer, or reuse its resources. `into_inner` can be used for this when the inner writer is still working. But when the inner writer is broken, and returning errors, `into_inner` simply gives you the error from flush, and the same `Bufwriter` back again. Here I provide the necessary function, which I have chosen to call `into_raw_parts`. I had to do something with `panicked`. Returning it to the caller as a boolean seemed rather bare. Throwing the buffered data away in this situation also seems unfriendly: maybe the programmer knows something about the underlying writer and can recover somehow. So I went for a custom Error. This may be overkill, but it does have the nice property that a caller who actually wants to look at the buffered data, rather than simply extracting the inner writer, will be told by the type system if they forget to handle the panicked case. If a caller doesn't need the buffer, it can just be discarded. That WriterPanicked is a newtype around Vec<u8> means that hopefully the layouts of the Ok and Err variants can be very similar, with just a boolean discriminant. So this custom error type should compile down to nearly no code. Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
ijackson
force-pushed
the
bufwriter-disassemble
branch
from
997670a
to
3817631
Compare
m-ou-se
added
A-io
pickfire
reviewed
Dec 5, 2020
pickfire
reviewed
Dec 5, 2020
pickfire
reviewed
Dec 5, 2020
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Co-authored-by: Ivan Tham <pickfire@riseup.net>
|
I spend some time thinking about this, and I agree the API you propose makes more sense than the alternatives. We'll just have to get some experience with this new feature to find out if it works well in practice. Can you open a tracking issue for this? Thanks! |
m-ou-se
added
S-waiting-on-author
3 tasks
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
This comment has been minimized.
This comment has been minimized.
|
@rustbot modify labels -S-waiting-on-author +S-waiting-on-review |
rustbot
added
S-waiting-on-review
|
@bors r+ |
|
📌 Commit dea6d6c has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit dea6d6c with merge 049587609826dd4af26d187ed4be5bb464d6f3d2... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
Weird. Same thing happened on another PR yesterday. @bors retry |
bors
added
S-waiting-on-bors
|
Mara Bos writes ("Re: [rust-lang/rust] BufWriter: Provide into_raw_parts (#79705)"):
Weird. Same thing happened on another PR yesterday.
13 is SIGPIPE. That is odd because Rust's startup code (IMO
unjustifiably) catches SIGPIPE. See #62569, #46016.
The test case for #80893 tests rustdoc `-test-builder`, passing
`true`. Perhaps that makes rustdoc much faster so we lose some race.
…--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.
|
|
☀️ Test successful - checks-actions |
Merged
Closed
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.
If something goes wrong, one might want to unpeel the layers of nested
Writers to perform recovery actions on the underlying writer, or reuse
its resources.
into_innercan be used for this when the inner writer is stillworking. But when the inner writer is broken, and returning errors,
into_innersimply gives you the error from flush, and the sameBufwriterback again.Here I provide the necessary function, which I have chosen to call
into_raw_parts.I had to do something with
panicked. Returning it to the caller asa boolean seemed rather bare. Throwing the buffered data away in this
situation also seems unfriendly: maybe the programmer knows something
about the underlying writer and can recover somehow.
So I went for a custom Error. This may be overkill, but it does have
the nice property that a caller who actually wants to look at the
buffered data, rather than simply extracting the inner writer, will be
told by the type system if they forget to handle the panicked case.
If a caller doesn't need the buffer, it can just be discarded. That
WriterPanicked is a newtype around Vec means that hopefully the
layouts of the Ok and Err variants can be very similar, with just a
boolean discriminant. So this custom error type should compile down
to nearly no code.
If this general idea is felt appropriate, I will open a tracking issue, etc.