-
Notifications
You must be signed in to change notification settings - Fork 128
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
Redesign BufResult
to be std::result::Result
#178
Comments
The currently promoted pattern of error handling could be enabled by an adapter trait: pub trait LiftBuf {
type Output;
type Buf;
fn lift_buf(self) -> (io::Result<Self::Output>, Self::Buf);
}
impl<T, B> LiftBuf for BufResult<T, B> {
type Output = T;
type Buf = B;
fn lift_buf(self) -> (io::Result<Self::Output>, Self::Buf) {
match self {
Ok((out, buf)) => (Ok(out), buf),
Err(BufError(e, buf)) => (Err(e), buf),
}
}
} |
I like it. It places the buffer as the second of an Ok tuple or an Err tuple. Returns would be Ok(val, buf)
// or
Err(err, buf) |
@mzabaluev Do you have a login on rust's zulip chat? There is a thread, "IO traits" in the wg-async stream, that you should be aware of. It basically is about the long term goals of rust, in supporting async runtimes with more common features built into the std library and nrc has created a git repo for capturing their thoughts on buffer traits, and there are pieces for completion based APIs like uring requires. Your experience with slices and features like indexed buffers and returning the owned buffer and owned slice in results and errors would be very welcome I believe, and it doesn't hurt that you know the trait system so well. |
I completely agree with this proposed change. |
Responding to points raised in a zulip thread:
I find that the pattern of error handling depends a lot on the code around the call site. If the calling function does not have the buffer in its return signature, it can't be propagated anyway and you need to drop it. But I believe more often than not the buffer will need to be propagated, so a Then there are loops that reuse the buffer. Note how the compiler forces recovery of the moved let mut buf = vec![0u8; 4096];
loop {
match stream.read(buf).await {
Ok((bytes_read, nbuf)) => {
buf = nbuf;
if bytes_read == 0 {
break;
}
// Do something with received data
}
Err(e) if is_recoverable(&e.0) => {
todo!("need to recover buf for the next iteration");
continue;
}
Err(BufError(e, buf)) => {
todo!("handle the error, deal with the buffer")
}
}
} Aside from these two situations, I'd like to come up with an example where it would be easy to lose the buffer in error handling and not have some visible hint that this happens. This proposal also provides an adapter trait to convert to
We already have the distinction between functions that work with a buffer and functions that don't, and the latter return |
An alternative (which I'm not sure is better) would be to use a newtype rather than an alias. The advantage would be that you could implement |
This is interesting to me - could you explain why you think it?
I'm not sure I understand the example - is this different to the current situation with |
tokio-uring (or, for that matter, any completion-driven I/O crate) is a low level crate, I expect that applications would add their customized I/O APIs on top of it. If the application is not OK with consuming the allocation for each operation (or does not use a reference-counted buffer collection like
My point is that the |
#216 may have bearing on this, if accepted. It means the success case for read like operations can just return the buffer |
This also seemed to have wide spread support. Maybe we should get something in sooner rather than later for this issue. |
BufResult
being aliased to a tuple makes it less ergonomic than a standardResult
could be.In future, a
std::ops::Try
implementation could help, but it needs a local type to be implemented on.I propose to make use of the standard infrastructure provided by
Result
and provide the buffer with a new error type:The text was updated successfully, but these errors were encountered: