From c9467d7317a8c0154f5cb507e3a115a56dedca02 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 10:55:11 -0800 Subject: [PATCH 01/10] Update references to Reader/Writer --- text/0517-io-os-reform.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index 3105a9fae4a..c60f56413a2 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -639,8 +639,8 @@ The `io` module is split into the parts that can live in `libcore` (most of it) and the parts that are added in the `std::io` facade. Being able to move components into `libcore` at all is made possible through the use of -[associated error types](#revising-reader-and-writer) for `Reader` and -`Writer`. +[associated error types](#revising-reader-and-writer) for `Read` and +`Write`. #### Adapters [Adapters]: #adapters @@ -657,13 +657,13 @@ trait ReadExt: Read { fn by_ref<'a>(&'a mut self) -> ByRef<'a, Self> { ... } // Read everything from `self`, then read from `next` - fn chain(self, next: R) -> Chain { ... } + fn chain(self, next: R) -> Chain { ... } // Adapt `self` to yield only the first `limit` bytes fn take(self, limit: u64) -> Take { ... } // Whenever reading from `self`, push the bytes read to `out` - fn tee(self, out: W) -> Tee { ... } + fn tee(self, out: W) -> Tee { ... } } trait WriteExt: Write { @@ -673,7 +673,7 @@ trait WriteExt: Write { fn by_ref<'a>(&'a mut self) -> ByRef<'a, Self> { ... } // Whenever bytes are written to `self`, write them to `other` as well - fn broadcast(self, other: W) -> Broadcast { ... } + fn broadcast(self, other: W) -> Broadcast { ... } } // An adaptor converting an `Iterator` to `Read`. @@ -715,7 +715,7 @@ readers and writers, as well as `copy`. These are updated as follows: ```rust // A reader that yields no bytes -fn empty() -> Empty; // in theory just returns `impl Reader` +fn empty() -> Empty; // in theory just returns `impl Read` // A reader that yields `byte` repeatedly (generalizes today's ZeroReader) fn repeat(byte: u8) -> Repeat; @@ -723,7 +723,7 @@ fn repeat(byte: u8) -> Repeat; // A writer that ignores the bytes written to it (/dev/null) fn sink() -> Sink; -// Copies all data from a Reader to a Writer +// Copies all data from a `Read` to a `Write` pub fn copy(r: &mut R, w: &mut W) -> Result<(), E> where R: Read, W: Write @@ -873,7 +873,7 @@ error code. The `IoErrorKind` type will become `std::io::ErrorKind`, and `ShortWrite` will be dropped (it is no longer needed with the new -`Writer` semantics), which should decrease its footprint. The +`Write` semantics), which should decrease its footprint. The `OtherIoError` variant will become `Other` now that `enum`s are namespaced. From 6469aae4e1955e8b1d9564adafa5896be55b837f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 10:57:44 -0800 Subject: [PATCH 02/10] Mark tee()/broadcast() as unstable --- text/0517-io-os-reform.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index c60f56413a2..f194da17cf8 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -663,6 +663,7 @@ trait ReadExt: Read { fn take(self, limit: u64) -> Take { ... } // Whenever reading from `self`, push the bytes read to `out` + #[unstable] // uncertain semantics of errors "halfway through the operation" fn tee(self, out: W) -> Tee { ... } } @@ -673,6 +674,7 @@ trait WriteExt: Write { fn by_ref<'a>(&'a mut self) -> ByRef<'a, Self> { ... } // Whenever bytes are written to `self`, write them to `other` as well + #[unstable] // uncertain semantics of errors "halfway through the operation" fn broadcast(self, other: W) -> Broadcast { ... } } From f3ff336fe7dbc9b9e75d8ea57f65ecc2d7215fc9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:07:29 -0800 Subject: [PATCH 03/10] Add info about `Void` --- text/0517-io-os-reform.md | 46 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index f194da17cf8..80d702711ff 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -51,6 +51,8 @@ follow-up PRs against this RFC. * [Modules] * [core::io] * [Adapters] + * [Free functions] + * [Void] * [Seeking] * [Buffering] * [Cursor] @@ -654,7 +656,10 @@ trait ReadExt: Read { // ... eliding the methods already described above // Reify a borrowed reader as owned - fn by_ref<'a>(&'a mut self) -> ByRef<'a, Self> { ... } + fn by_ref(&mut self) -> ByRef { ... } + + // Map all errors to another type via `FromError` + fn map_err>(self) -> MapErr { ... } // Read everything from `self`, then read from `next` fn chain(self, next: R) -> Chain { ... } @@ -673,6 +678,9 @@ trait WriteExt: Write { // Reify a borrowed writer as owned fn by_ref<'a>(&'a mut self) -> ByRef<'a, Self> { ... } + // Map all errors to another type via `FromError` + fn map_err>(self) -> MapErr { ... } + // Whenever bytes are written to `self`, write them to `other` as well #[unstable] // uncertain semantics of errors "halfway through the operation" fn broadcast(self, other: W) -> Broadcast { ... } @@ -731,6 +739,42 @@ pub fn copy(r: &mut R, w: &mut W) -> Result<(), E> where W: Write ``` +Each of the `Empty`, `Repeat`, and `Sink` types will use the [`Void`][Void] +associated error type. + +#### Void +[Void]: #void + +A new concrete error type will be added in the standard library. A new module +`std::void` will be introduced with the following contents: + +```rust +pub enum Void {} + +impl FromError for E { + fn from_error(v: Void) -> E { + match v {} + } +} +``` + +Applications for an uninhabited enum have come up from time-to-time, and some of +the core I/O adapters represent a fairly concrete use case motivating its +existence. By using an associated `Err` type of `Void`, each I/O object is +indicating that it *can never fail*. This allows the types themselves to be more +optimized in the future as well as enabling interoperation with many other error +types via the `map_err` adaptor. + +Some possible future optimizations include: + +* `Result` could be represented in memory exactly as `T` (no + discriminant). +* The `unused_must_use` lint could understand that `Result` does not + need to be warned about. + +This RFC does not propose implementing these modifications at this time, +however. + #### Seeking [Seeking]: #seeking From 6662f758b7b5aac49a6ad2e410c4ff650d9fe0c1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:11:41 -0800 Subject: [PATCH 04/10] Clarify associated error types for each adaptor --- text/0517-io-os-reform.md | 45 ++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index 80d702711ff..42e5b5a9501 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -727,21 +727,25 @@ readers and writers, as well as `copy`. These are updated as follows: // A reader that yields no bytes fn empty() -> Empty; // in theory just returns `impl Read` +impl Read for Empty { type Err = Void; ... } + // A reader that yields `byte` repeatedly (generalizes today's ZeroReader) fn repeat(byte: u8) -> Repeat; +impl Read for Repeat { type Err = Void; ... } + // A writer that ignores the bytes written to it (/dev/null) fn sink() -> Sink; -// Copies all data from a `Read` to a `Write` -pub fn copy(r: &mut R, w: &mut W) -> Result<(), E> where +impl Write for Sink { type Err = Void; ... } + +// Copies all data from a `Read` to a `Write`, returning the amount of data +// copied. +pub fn copy(r: &mut R, w: &mut W) -> Result where R: Read, W: Write ``` -Each of the `Empty`, `Repeat`, and `Sink` types will use the [`Void`][Void] -associated error type. - #### Void [Void]: #void @@ -843,9 +847,9 @@ or `Write`. This is often useful when composing streams or creating test cases. This functionality primarily comes from the following implementations: ```rust -impl<'a> Read for &'a [u8] { ... } -impl<'a> Write for &'a mut [u8] { ... } -impl Write for Vec { ... } +impl<'a> Read for &'a [u8] { type Err = Void; ... } +impl<'a> Write for &'a mut [u8] { type Err = Void; ... } +impl Write for Vec { type Err = Void; ... } ``` While efficient, none of these implementations support seeking (via an @@ -865,20 +869,23 @@ impl Cursor { pub fn get_ref(&self) -> &T; } -impl Seek for Cursor> { ... } -impl<'a> Seek for Cursor<&'a [u8]> { ... } -impl<'a> Seek for Cursor<&'a mut [u8]> { ... } +// Error indicating that a negative offset was seeked to. +pub struct NegativeOffset; + +impl Seek for Cursor> { type Err = NegativeOffset; ... } +impl<'a> Seek for Cursor<&'a [u8]> { type Err = NegativeOffset; ... } +impl<'a> Seek for Cursor<&'a mut [u8]> { type Err = NegativeOffset; ... } -impl Read for Cursor> { ... } -impl<'a> Read for Cursor<&'a [u8]> { ... } -impl<'a> Read for Cursor<&'a mut [u8]> { ... } +impl Read for Cursor> { type Err = Void; ... } +impl<'a> Read for Cursor<&'a [u8]> { type Err = Void; ... } +impl<'a> Read for Cursor<&'a mut [u8]> { type Err = Void; ... } -impl BufferedRead for Cursor> { ... } -impl<'a> BufferedRead for Cursor<&'a [u8]> { ... } -impl<'a> BufferedRead for Cursor<&'a mut [u8]> { ... } +impl BufferedRead for Cursor> { type Err = Void; ... } +impl<'a> BufferedRead for Cursor<&'a [u8]> { type Err = Void; ... } +impl<'a> BufferedRead for Cursor<&'a mut [u8]> { type Err = Void; ... } -impl<'a> Write for Cursor<&'a mut [u8]> { ... } -impl Write for Cursor> { ... } +impl<'a> Write for Cursor<&'a mut [u8]> { type Err = Void; ... } +impl Write for Cursor> { type Err = Void; ... } ``` A sample implementation can be found in [a gist][cursor-impl]. Using one From f2fc570df0245b85069c029a5d7f2acb125e1c2c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:12:07 -0800 Subject: [PATCH 05/10] Move read_until to BufferedReadExt --- text/0517-io-os-reform.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index 42e5b5a9501..ed5495f1a48 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -813,11 +813,10 @@ point): pub trait BufferedRead: Read { fn fill_buf(&mut self) -> Result<&[u8], Self::Err>; fn consume(&mut self, amt: uint); - - fn read_until(&mut self, byte: u8) -> ReadUntil { ... } } pub trait BufferedReadExt: BufferedRead { + fn read_until(&mut self, byte: u8) -> ReadUntil { ... } fn lines(&mut self) -> Lines { ... }; } ``` From 89b45b3fece378fc06140fa148b293cf568c7374 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:13:50 -0800 Subject: [PATCH 06/10] Clarify `write` can return `Ok(0)` for "eof" as well --- text/0517-io-os-reform.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index ed5495f1a48..b70f396766a 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -930,12 +930,12 @@ The `IoErrorKind` type will become `std::io::ErrorKind`, and namespaced. The `EndOfFile` variant will be removed in favor of returning `Ok(0)` -from `read` on end of file. This approach clarifies the meaning of the -return value of `read`, matches Posix APIs, and makes it easier to use -`try!` in the case that a "real" error should be bubbled out. (The -main downside is that higher-level operations that might use -`Result` with some `T != usize` may need to wrap `IoError` -in a further enum if they wish to forward unexpected EOF.) +from `read` on end of file (or `write` on an empty slice for example). This +approach clarifies the meaning of the return value of `read`, matches Posix +APIs, and makes it easier to use `try!` in the case that a "real" error should +be bubbled out. (The main downside is that higher-level operations that might +use `Result` with some `T != usize` may need to wrap `IoError` in a +further enum if they wish to forward unexpected EOF.) #### Channel adapters [Channel adapters]: #channel-adapters From 78d6dd324fd12f22fa91c2dfa3b65a5fedda74a7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:14:51 -0800 Subject: [PATCH 07/10] Clarify more variants of IoErrorKind may be added. --- text/0517-io-os-reform.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index b70f396766a..6a65e404e1a 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -923,11 +923,12 @@ It will remain largely as it is today, but its fields will be made private. It may eventually grow a field to track the underlying OS error code. -The `IoErrorKind` type will become `std::io::ErrorKind`, and +The `std::io::IoErrorKind` type will become `std::io::ErrorKind`, and `ShortWrite` will be dropped (it is no longer needed with the new `Write` semantics), which should decrease its footprint. The `OtherIoError` variant will become `Other` now that `enum`s are -namespaced. +namespaced. Other variants may be added over time, such as `Interrupted`, +as more errors are classified from the system. The `EndOfFile` variant will be removed in favor of returning `Ok(0)` from `read` on end of file (or `write` on an empty slice for example). This From cc58adebc8748d85662c771c4efdffaf2ebe258c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:22:02 -0800 Subject: [PATCH 08/10] Remove `position` from the `Seek` trait This hones the trait to "one method which matches the trait name" and the position can still be learned with a relative seek of 0 bytes. --- text/0517-io-os-reform.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index 6a65e404e1a..0b9a5c7cad3 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -783,14 +783,12 @@ however. [Seeking]: #seeking The seeking infrastructure is largely the same as today's, except that -`tell` is renamed to follow the RFC's design principles and the `seek` -signature is refactored with more precise types: +`tell` is removed and the `seek` signature is refactored with more precise +types: ```rust pub trait Seek { type Err; - fn position(&self) -> Result; - // returns the new position after seeking fn seek(&mut self, pos: SeekPos) -> Result; } @@ -802,6 +800,8 @@ pub enum SeekPos { } ``` +The old `tell` function can be regained via `seek(SeekPos::FromCur(0))`. + #### Buffering [Buffering]: #buffering From c22c6c66fe3d4198b922ce953313e20dfbce7e32 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:34:58 -0800 Subject: [PATCH 09/10] Clearly indicate that `copy` is lossy --- text/0517-io-os-reform.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index 0b9a5c7cad3..e0fff783e8c 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -746,6 +746,11 @@ pub fn copy(r: &mut R, w: &mut W) -> Result where W: Write ``` +Like `write_all`, the `copy` method will discard the amount of data already +written on any error and also discard any partially read data on a `write` +error. This method is intended to be a convenience and `write` should be used +directly if this is not desirable. + #### Void [Void]: #void From e439eb571c697d2c523547253dbfae9d0c1d66c9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Jan 2015 11:40:46 -0800 Subject: [PATCH 10/10] Add WriteAllError trait bound to write_all This is needed to construct an instance on an `Ok(0)` return value as well as testing for whether the operation was interrupted or not. --- text/0517-io-os-reform.md | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/text/0517-io-os-reform.md b/text/0517-io-os-reform.md index e0fff783e8c..32d7b630ac6 100644 --- a/text/0517-io-os-reform.md +++ b/text/0517-io-os-reform.md @@ -584,10 +584,18 @@ trait Write { } trait WriteExt { - fn write_all(&mut self, buf: &[u8]) -> Result<(), Err> { ... }; - fn write_fmt(&mut self, fmt: &fmt::Arguments) -> Result<(), Err> { ... } + fn write_all(&mut self, buf: &[u8]) -> Result<(), Err> + where Self::Err: WriteAllError { ... }; + fn write_fmt(&mut self, fmt: &fmt::Arguments) -> Result<(), Err> + where Self::Err: WriteAllError { ... }; } + impl WriteExt for W {} + +trait WriteAllError: Sized { + fn eof() -> Self; + fn interrupted(&self) -> bool; +} ``` The biggest change here is to the semantics of `write`. Instead of @@ -604,6 +612,11 @@ complete, the intermediate result (of how much has been written) is discarded. To meaningfully recover from an intermediate error, code should work with `write` directly. +A trait bound of `WriteAllError` is also imposed on the error type for the +`write_all` and `write_fmt` methods to construct an "end of file" related error +when `Ok(0)` is returned from `write` as well as detecting intermittent errors +like `EINTR` that can be "safely ignored" to try to continue writing data. + The `write_fmt` method, like `write_all`, will loop until its entire input is written or an error occurs. @@ -741,9 +754,10 @@ impl Write for Sink { type Err = Void; ... } // Copies all data from a `Read` to a `Write`, returning the amount of data // copied. -pub fn copy(r: &mut R, w: &mut W) -> Result where - R: Read, - W: Write +pub fn copy(r: &mut R, w: &mut W) -> Result + where R: Read, + W: Write, + R::Err: WriteAllError ``` Like `write_all`, the `copy` method will discard the amount of data already