Skip to content

Commit

Permalink
Auto merge of rust-lang#85746 - m-ou-se:io-error-other, r=joshtriplett
Browse files Browse the repository at this point in the history
Redefine `ErrorKind::Other` and stop using it in std.

This implements the idea I shared yesterday in the libs meeting when we were discussing how to handle adding new `ErrorKind`s to the standard library: This redefines `Other` to be for *user defined errors only*, and changes all uses of `Other` in the standard library to a `#[doc(hidden)]` and permanently `#[unstable]` `ErrorKind` that users can not match on. This ensures that adding `ErrorKind`s at a later point in time is not a breaking change, since the user couldn't match on these errors anyway. This way, we use the `#[non_exhaustive]` property of the enum in a more effective way.

Open questions:
- How do we check this change doesn't cause too much breakage? Will a crate run help and be enough?
- How do we ensure we don't accidentally start using `Other` again in the standard library? We don't have a `pub(not crate)` or `#[deprecated(in this crate only)]`.

cc rust-lang#79965

cc `@rust-lang/libs` `@ijackson`

r? `@dtolnay`
  • Loading branch information
bors committed Jul 2, 2021
2 parents 1aa6c7c + cc90733 commit f9fa13f
Show file tree
Hide file tree
Showing 24 changed files with 99 additions and 74 deletions.
12 changes: 6 additions & 6 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,8 +880,7 @@ impl OpenOptions {
/// This function will return an error under a number of different
/// circumstances. Some of these error conditions are listed here, together
/// with their [`io::ErrorKind`]. The mapping to [`io::ErrorKind`]s is not
/// part of the compatibility contract of the function, especially the
/// [`Other`] kind might change to more specific kinds in the future.
/// part of the compatibility contract of the function.
///
/// * [`NotFound`]: The specified file does not exist and neither `create`
/// or `create_new` is set.
Expand All @@ -895,9 +894,11 @@ impl OpenOptions {
/// exists.
/// * [`InvalidInput`]: Invalid combinations of open options (truncate
/// without write access, no access mode set, etc.).
/// * [`Other`]: One of the directory components of the specified file path
///
/// The following errors don't match any existing [`io::ErrorKind`] at the moment:
/// * One of the directory components of the specified file path
/// was not, in fact, a directory.
/// * [`Other`]: Filesystem-level errors: full disk, write permission
/// * Filesystem-level errors: full disk, write permission
/// requested on a read-only file system, exceeded disk quota, too many
/// open files, too long filename, too many symbolic links in the
/// specified path (Unix-like systems only), etc.
Expand All @@ -913,7 +914,6 @@ impl OpenOptions {
/// [`AlreadyExists`]: io::ErrorKind::AlreadyExists
/// [`InvalidInput`]: io::ErrorKind::InvalidInput
/// [`NotFound`]: io::ErrorKind::NotFound
/// [`Other`]: io::ErrorKind::Other
/// [`PermissionDenied`]: io::ErrorKind::PermissionDenied
#[stable(feature = "rust1", since = "1.0.0")]
pub fn open<P: AsRef<Path>>(&self, path: P) -> io::Result<File> {
Expand Down Expand Up @@ -2216,7 +2216,7 @@ impl DirBuilder {
Some(p) => self.create_dir_all(p)?,
None => {
return Err(io::Error::new_const(
io::ErrorKind::Other,
io::ErrorKind::Uncategorized,
&"failed to create whole tree",
));
}
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/fs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,8 @@ fn metadata_access_times() {
match (a.created(), b.created()) {
(Ok(t1), Ok(t2)) => assert!(t1 <= t2),
(Err(e1), Err(e2))
if e1.kind() == ErrorKind::Other && e2.kind() == ErrorKind::Other
if e1.kind() == ErrorKind::Uncategorized
&& e2.kind() == ErrorKind::Uncategorized
|| e1.kind() == ErrorKind::Unsupported
&& e2.kind() == ErrorKind::Unsupported => {}
(a, b) => {
Expand Down
29 changes: 22 additions & 7 deletions library/std/src/io/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,17 @@ pub enum ErrorKind {
/// Interrupted operations can typically be retried.
#[stable(feature = "rust1", since = "1.0.0")]
Interrupted,
/// Any I/O error not part of this list.

/// A custom error that does not fall under any other I/O error kind.
///
/// Errors that are `Other` now may move to a different or a new
/// [`ErrorKind`] variant in the future. It is not recommended to match
/// an error against `Other` and to expect any additional characteristics,
/// e.g., a specific [`Error::raw_os_error`] return value.
/// This can be used to construct your own [`Error`]s that do not match any
/// [`ErrorKind`].
///
/// This [`ErrorKind`] is not used by the standard library.
///
/// Errors from the standard library that do not fall under any of the I/O
/// error kinds cannot be `match`ed on, and will only match a wildcard (`_`) pattern.
/// New [`ErrorKind`]s might be added in the future for some of those.
#[stable(feature = "rust1", since = "1.0.0")]
Other,

Expand All @@ -191,6 +196,15 @@ pub enum ErrorKind {
/// to allocate enough memory.
#[stable(feature = "out_of_memory_error", since = "1.54.0")]
OutOfMemory,

/// Any I/O error from the standard library that's not part of this list.
///
/// Errors that are `Uncategorized` now may move to a different or a new
/// [`ErrorKind`] variant in the future. It is not recommended to match
/// an error against `Uncategorized`; use a wildcard match (`_`) instead.
#[unstable(feature = "io_error_uncategorized", issue = "none")]
#[doc(hidden)]
Uncategorized,
}

impl ErrorKind {
Expand All @@ -212,10 +226,11 @@ impl ErrorKind {
ErrorKind::TimedOut => "timed out",
ErrorKind::WriteZero => "write zero",
ErrorKind::Interrupted => "operation interrupted",
ErrorKind::Other => "other os error",
ErrorKind::UnexpectedEof => "unexpected end of file",
ErrorKind::Unsupported => "unsupported",
ErrorKind::OutOfMemory => "out of memory",
ErrorKind::Other => "other error",
ErrorKind::Uncategorized => "uncategorized error",
}
}
}
Expand Down Expand Up @@ -538,7 +553,7 @@ impl Error {
/// }
///
/// fn main() {
/// // Will print "Other".
/// // Will print "Uncategorized".
/// print_error(Error::last_os_error());
/// // Will print "AddrInUse".
/// print_error(Error::new(ErrorKind::AddrInUse, "oh no!"));
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1644,7 +1644,7 @@ pub trait Write {
if output.error.is_err() {
output.error
} else {
Err(Error::new_const(ErrorKind::Other, &"formatter error"))
Err(Error::new_const(ErrorKind::Uncategorized, &"formatter error"))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/net/tcp/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ fn double_bind() {
Err(e) => {
assert!(
e.kind() == ErrorKind::ConnectionRefused
|| e.kind() == ErrorKind::Other
|| e.kind() == ErrorKind::Uncategorized
|| e.kind() == ErrorKind::AddrInUse,
"unknown error: {} {:?}",
e,
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/os/wasi/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,5 +532,6 @@ pub fn symlink_path<P: AsRef<Path>, U: AsRef<Path>>(old_path: P, new_path: U) ->
}

fn osstr2str(f: &OsStr) -> io::Result<&str> {
f.to_str().ok_or_else(|| io::Error::new_const(io::ErrorKind::Other, &"input must be utf-8"))
f.to_str()
.ok_or_else(|| io::Error::new_const(io::ErrorKind::Uncategorized, &"input must be utf-8"))
}
4 changes: 1 addition & 3 deletions library/std/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1658,8 +1658,7 @@ impl Child {
/// Forces the child process to exit. If the child has already exited, an [`InvalidInput`]
/// error is returned.
///
/// The mapping to [`ErrorKind`]s is not part of the compatibility contract of the function,
/// especially the [`Other`] kind might change to more specific kinds in the future.
/// The mapping to [`ErrorKind`]s is not part of the compatibility contract of the function.
///
/// This is equivalent to sending a SIGKILL on Unix platforms.
///
Expand All @@ -1680,7 +1679,6 @@ impl Child {
///
/// [`ErrorKind`]: io::ErrorKind
/// [`InvalidInput`]: io::ErrorKind::InvalidInput
/// [`Other`]: io::ErrorKind::Other
#[stable(feature = "process", since = "1.0.0")]
pub fn kill(&mut self) -> io::Result<()> {
self.handle.kill()
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/hermit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub fn decode_error_kind(errno: i32) -> ErrorKind {
x if x == 1 as i32 => ErrorKind::PermissionDenied,
x if x == 32 as i32 => ErrorKind::BrokenPipe,
x if x == 110 as i32 => ErrorKind::TimedOut,
_ => ErrorKind::Other,
_ => ErrorKind::Uncategorized,
}
}

Expand Down
51 changes: 28 additions & 23 deletions library/std/src/sys/hermit/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::time::Duration;
pub fn init() -> io::Result<()> {
if abi::network_init() < 0 {
return Err(io::Error::new_const(
ErrorKind::Other,
ErrorKind::Uncategorized,
&"Unable to initialize network interface",
));
}
Expand Down Expand Up @@ -51,7 +51,7 @@ impl TcpStream {
match abi::tcpstream::connect(addr.ip().to_string().as_bytes(), addr.port(), None) {
Ok(handle) => Ok(TcpStream(Arc::new(Socket(handle)))),
_ => Err(io::Error::new_const(
ErrorKind::Other,
ErrorKind::Uncategorized,
&"Unable to initiate a connection on a socket",
)),
}
Expand All @@ -65,44 +65,46 @@ impl TcpStream {
) {
Ok(handle) => Ok(TcpStream(Arc::new(Socket(handle)))),
_ => Err(io::Error::new_const(
ErrorKind::Other,
ErrorKind::Uncategorized,
&"Unable to initiate a connection on a socket",
)),
}
}

pub fn set_read_timeout(&self, duration: Option<Duration>) -> io::Result<()> {
abi::tcpstream::set_read_timeout(*self.0.as_inner(), duration.map(|d| d.as_millis() as u64))
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"Unable to set timeout value"))
.map_err(|_| {
io::Error::new_const(ErrorKind::Uncategorized, &"Unable to set timeout value")
})
}

pub fn set_write_timeout(&self, duration: Option<Duration>) -> io::Result<()> {
abi::tcpstream::set_write_timeout(
*self.0.as_inner(),
duration.map(|d| d.as_millis() as u64),
)
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"Unable to set timeout value"))
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"Unable to set timeout value"))
}

pub fn read_timeout(&self) -> io::Result<Option<Duration>> {
let duration = abi::tcpstream::get_read_timeout(*self.0.as_inner()).map_err(|_| {
io::Error::new_const(ErrorKind::Other, &"Unable to determine timeout value")
io::Error::new_const(ErrorKind::Uncategorized, &"Unable to determine timeout value")
})?;

Ok(duration.map(|d| Duration::from_millis(d)))
}

pub fn write_timeout(&self) -> io::Result<Option<Duration>> {
let duration = abi::tcpstream::get_write_timeout(*self.0.as_inner()).map_err(|_| {
io::Error::new_const(ErrorKind::Other, &"Unable to determine timeout value")
io::Error::new_const(ErrorKind::Uncategorized, &"Unable to determine timeout value")
})?;

Ok(duration.map(|d| Duration::from_millis(d)))
}

pub fn peek(&self, buf: &mut [u8]) -> io::Result<usize> {
abi::tcpstream::peek(*self.0.as_inner(), buf)
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"set_nodelay failed"))
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"peek failed"))
}

pub fn read(&self, buffer: &mut [u8]) -> io::Result<usize> {
Expand All @@ -113,8 +115,9 @@ impl TcpStream {
let mut size: usize = 0;

for i in ioslice.iter_mut() {
let ret = abi::tcpstream::read(*self.0.as_inner(), &mut i[0..])
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"Unable to read on socket"))?;
let ret = abi::tcpstream::read(*self.0.as_inner(), &mut i[0..]).map_err(|_| {
io::Error::new_const(ErrorKind::Uncategorized, &"Unable to read on socket")
})?;

if ret != 0 {
size += ret;
Expand All @@ -138,7 +141,7 @@ impl TcpStream {

for i in ioslice.iter() {
size += abi::tcpstream::write(*self.0.as_inner(), i).map_err(|_| {
io::Error::new_const(ErrorKind::Other, &"Unable to write on socket")
io::Error::new_const(ErrorKind::Uncategorized, &"Unable to write on socket")
})?;
}

Expand All @@ -152,13 +155,13 @@ impl TcpStream {

pub fn peer_addr(&self) -> io::Result<SocketAddr> {
let (ipaddr, port) = abi::tcpstream::peer_addr(*self.0.as_inner())
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"peer_addr failed"))?;
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"peer_addr failed"))?;

let saddr = match ipaddr {
Ipv4(ref addr) => SocketAddr::new(IpAddr::V4(Ipv4Addr::from(addr.0)), port),
Ipv6(ref addr) => SocketAddr::new(IpAddr::V6(Ipv6Addr::from(addr.0)), port),
_ => {
return Err(io::Error::new_const(ErrorKind::Other, &"peer_addr failed"));
return Err(io::Error::new_const(ErrorKind::Uncategorized, &"peer_addr failed"));
}
};

Expand All @@ -170,8 +173,9 @@ impl TcpStream {
}

pub fn shutdown(&self, how: Shutdown) -> io::Result<()> {
abi::tcpstream::shutdown(*self.0.as_inner(), how as i32)
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"unable to shutdown socket"))
abi::tcpstream::shutdown(*self.0.as_inner(), how as i32).map_err(|_| {
io::Error::new_const(ErrorKind::Uncategorized, &"unable to shutdown socket")
})
}

pub fn duplicate(&self) -> io::Result<TcpStream> {
Expand All @@ -180,31 +184,32 @@ impl TcpStream {

pub fn set_nodelay(&self, mode: bool) -> io::Result<()> {
abi::tcpstream::set_nodelay(*self.0.as_inner(), mode)
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"set_nodelay failed"))
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"set_nodelay failed"))
}

pub fn nodelay(&self) -> io::Result<bool> {
abi::tcpstream::nodelay(*self.0.as_inner())
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"nodelay failed"))
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"nodelay failed"))
}

pub fn set_ttl(&self, tll: u32) -> io::Result<()> {
abi::tcpstream::set_tll(*self.0.as_inner(), tll)
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"unable to set TTL"))
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"unable to set TTL"))
}

pub fn ttl(&self) -> io::Result<u32> {
abi::tcpstream::get_tll(*self.0.as_inner())
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"unable to get TTL"))
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"unable to get TTL"))
}

pub fn take_error(&self) -> io::Result<Option<io::Error>> {
unsupported()
}

pub fn set_nonblocking(&self, mode: bool) -> io::Result<()> {
abi::tcpstream::set_nonblocking(*self.0.as_inner(), mode)
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"unable to set blocking mode"))
abi::tcpstream::set_nonblocking(*self.0.as_inner(), mode).map_err(|_| {
io::Error::new_const(ErrorKind::Uncategorized, &"unable to set blocking mode")
})
}
}

Expand All @@ -230,12 +235,12 @@ impl TcpListener {

pub fn accept(&self) -> io::Result<(TcpStream, SocketAddr)> {
let (handle, ipaddr, port) = abi::tcplistener::accept(self.0.port())
.map_err(|_| io::Error::new_const(ErrorKind::Other, &"accept failed"))?;
.map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"accept failed"))?;
let saddr = match ipaddr {
Ipv4(ref addr) => SocketAddr::new(IpAddr::V4(Ipv4Addr::from(addr.0)), port),
Ipv6(ref addr) => SocketAddr::new(IpAddr::V6(Ipv6Addr::from(addr.0)), port),
_ => {
return Err(io::Error::new_const(ErrorKind::Other, &"accept failed"));
return Err(io::Error::new_const(ErrorKind::Uncategorized, &"accept failed"));
}
};

Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/hermit/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl io::Write for Stdout {
unsafe { len = abi::write(1, data.as_ptr() as *const u8, data.len()) }

if len < 0 {
Err(io::Error::new_const(io::ErrorKind::Other, &"Stdout is not able to print"))
Err(io::Error::new_const(io::ErrorKind::Uncategorized, &"Stdout is not able to print"))
} else {
Ok(len as usize)
}
Expand All @@ -52,7 +52,7 @@ impl io::Write for Stdout {
unsafe { len = abi::write(1, data.as_ptr() as *const u8, data.len()) }

if len < 0 {
Err(io::Error::new_const(io::ErrorKind::Other, &"Stdout is not able to print"))
Err(io::Error::new_const(io::ErrorKind::Uncategorized, &"Stdout is not able to print"))
} else {
Ok(len as usize)
}
Expand Down Expand Up @@ -81,7 +81,7 @@ impl io::Write for Stderr {
unsafe { len = abi::write(2, data.as_ptr() as *const u8, data.len()) }

if len < 0 {
Err(io::Error::new_const(io::ErrorKind::Other, &"Stderr is not able to print"))
Err(io::Error::new_const(io::ErrorKind::Uncategorized, &"Stderr is not able to print"))
} else {
Ok(len as usize)
}
Expand All @@ -93,7 +93,7 @@ impl io::Write for Stderr {
unsafe { len = abi::write(2, data.as_ptr() as *const u8, data.len()) }

if len < 0 {
Err(io::Error::new_const(io::ErrorKind::Other, &"Stderr is not able to print"))
Err(io::Error::new_const(io::ErrorKind::Uncategorized, &"Stderr is not able to print"))
} else {
Ok(len as usize)
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/hermit/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl Thread {
// The thread failed to start and as a result p was not consumed. Therefore, it is
// safe to reconstruct the box so that it gets deallocated.
drop(Box::from_raw(p));
Err(io::Error::new_const(io::ErrorKind::Other, &"Unable to create thread!"))
Err(io::Error::new_const(io::ErrorKind::Uncategorized, &"Unable to create thread!"))
} else {
Ok(Thread { tid: tid })
};
Expand Down
Loading

0 comments on commit f9fa13f

Please sign in to comment.