From 554918e311a1221744aa9b6d60e2f00f5b3155e5 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sun, 8 Aug 2021 01:04:33 -0700 Subject: [PATCH 1/9] Hide Repr details from io::Error, and rework `io::Error::new_const`. --- library/std/src/ffi/c_str.rs | 2 +- library/std/src/fs.rs | 4 +- library/std/src/io/buffered/bufreader.rs | 4 +- library/std/src/io/buffered/bufwriter.rs | 6 +- library/std/src/io/cursor.rs | 10 +- library/std/src/io/error.rs | 160 +++++++++++------- library/std/src/io/error/repr_unpacked.rs | 50 ++++++ library/std/src/io/error/tests.rs | 8 +- library/std/src/io/impls.rs | 9 +- library/std/src/io/mod.rs | 18 +- library/std/src/io/tests.rs | 4 +- library/std/src/net/mod.rs | 4 +- library/std/src/net/udp.rs | 6 +- library/std/src/os/fd/owned.rs | 4 +- library/std/src/os/unix/fs.rs | 6 +- library/std/src/os/unix/net/addr.rs | 16 +- library/std/src/os/wasi/fs.rs | 12 +- library/std/src/os/windows/io/socket.rs | 2 +- library/std/src/sys/hermit/fs.rs | 10 +- library/std/src/sys/hermit/mod.rs | 4 +- library/std/src/sys/hermit/net.rs | 49 +++--- library/std/src/sys/hermit/stdio.rs | 8 +- library/std/src/sys/hermit/thread.rs | 2 +- library/std/src/sys/sgx/mod.rs | 6 +- library/std/src/sys/sgx/net.rs | 12 +- library/std/src/sys/solid/fs.rs | 6 +- library/std/src/sys/solid/mod.rs | 4 +- library/std/src/sys/solid/net.rs | 10 +- library/std/src/sys/solid/os.rs | 6 +- library/std/src/sys/unix/fs.rs | 8 +- library/std/src/sys/unix/l4re.rs | 4 +- library/std/src/sys/unix/mod.rs | 5 +- library/std/src/sys/unix/net.rs | 14 +- library/std/src/sys/unix/os.rs | 20 +-- .../src/sys/unix/process/process_fuchsia.rs | 16 +- .../std/src/sys/unix/process/process_unix.rs | 13 +- .../src/sys/unix/process/process_vxworks.rs | 8 +- library/std/src/sys/unix/thread.rs | 10 +- library/std/src/sys/unsupported/common.rs | 4 +- library/std/src/sys/unsupported/os.rs | 4 +- library/std/src/sys/wasi/fs.rs | 4 +- library/std/src/sys/windows/fs.rs | 8 +- library/std/src/sys/windows/mod.rs | 4 +- library/std/src/sys/windows/net.rs | 12 +- library/std/src/sys/windows/process.rs | 8 +- library/std/src/sys/windows/stdio.rs | 16 +- library/std/src/sys/windows/thread.rs | 4 +- library/std/src/sys_common/fs.rs | 4 +- library/std/src/sys_common/net.rs | 6 +- 49 files changed, 345 insertions(+), 269 deletions(-) create mode 100644 library/std/src/io/error/repr_unpacked.rs diff --git a/library/std/src/ffi/c_str.rs b/library/std/src/ffi/c_str.rs index 66fee2fe54837..f747325deafbe 100644 --- a/library/std/src/ffi/c_str.rs +++ b/library/std/src/ffi/c_str.rs @@ -1077,7 +1077,7 @@ impl fmt::Display for NulError { impl From for io::Error { /// Converts a [`NulError`] into a [`io::Error`]. fn from(_: NulError) -> io::Error { - io::Error::new_const(io::ErrorKind::InvalidInput, &"data provided contains a nul byte") + io::const_io_error!(io::ErrorKind::InvalidInput, "data provided contains a nul byte") } } diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 7ca64c38e5d3a..95e9064442634 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -2263,9 +2263,9 @@ impl DirBuilder { match path.parent() { Some(p) => self.create_dir_all(p)?, None => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::Uncategorized, - &"failed to create whole tree", + "failed to create whole tree", )); } } diff --git a/library/std/src/io/buffered/bufreader.rs b/library/std/src/io/buffered/bufreader.rs index b56dc65f0b2f3..e7eee4436249b 100644 --- a/library/std/src/io/buffered/bufreader.rs +++ b/library/std/src/io/buffered/bufreader.rs @@ -357,9 +357,9 @@ impl Read for BufReader { let mut bytes = Vec::new(); self.read_to_end(&mut bytes)?; let string = crate::str::from_utf8(&bytes).map_err(|_| { - io::Error::new_const( + io::const_io_error!( io::ErrorKind::InvalidData, - &"stream did not contain valid UTF-8", + "stream did not contain valid UTF-8", ) })?; *buf += string; diff --git a/library/std/src/io/buffered/bufwriter.rs b/library/std/src/io/buffered/bufwriter.rs index c7423e4d92a89..2d3a0f37b4c2a 100644 --- a/library/std/src/io/buffered/bufwriter.rs +++ b/library/std/src/io/buffered/bufwriter.rs @@ -1,7 +1,7 @@ use crate::error; use crate::fmt; use crate::io::{ - self, Error, ErrorKind, IntoInnerError, IoSlice, Seek, SeekFrom, Write, DEFAULT_BUF_SIZE, + self, ErrorKind, IntoInnerError, IoSlice, Seek, SeekFrom, Write, DEFAULT_BUF_SIZE, }; use crate::mem; use crate::ptr; @@ -168,9 +168,9 @@ impl BufWriter { match r { Ok(0) => { - return Err(Error::new_const( + return Err(io::const_io_error!( ErrorKind::WriteZero, - &"failed to write the buffered data", + "failed to write the buffered data", )); } Ok(n) => guard.consume(n), diff --git a/library/std/src/io/cursor.rs b/library/std/src/io/cursor.rs index 416cc906e65a5..fc19704becee2 100644 --- a/library/std/src/io/cursor.rs +++ b/library/std/src/io/cursor.rs @@ -4,7 +4,7 @@ mod tests; use crate::io::prelude::*; use crate::cmp; -use crate::io::{self, Error, ErrorKind, IoSlice, IoSliceMut, ReadBuf, SeekFrom}; +use crate::io::{self, ErrorKind, IoSlice, IoSliceMut, ReadBuf, SeekFrom}; use core::convert::TryInto; @@ -297,9 +297,9 @@ where self.pos = n; Ok(self.pos) } - None => Err(Error::new_const( + None => Err(io::const_io_error!( ErrorKind::InvalidInput, - &"invalid seek to a negative or overflowing position", + "invalid seek to a negative or overflowing position", )), } } @@ -400,9 +400,9 @@ fn slice_write_vectored( // Resizing write implementation fn vec_write(pos_mut: &mut u64, vec: &mut Vec, buf: &[u8]) -> io::Result { let pos: usize = (*pos_mut).try_into().map_err(|_| { - Error::new_const( + io::const_io_error!( ErrorKind::InvalidInput, - &"cursor position exceeds maximum possible vector length", + "cursor position exceeds maximum possible vector length", ) })?; // Make sure the internal buffer is as least as big as where we diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 074d693b83155..67526090e5830 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -1,6 +1,9 @@ #[cfg(test)] mod tests; +mod repr_unpacked; +use repr_unpacked::Repr; + use crate::convert::From; use crate::error; use crate::fmt; @@ -66,15 +69,38 @@ impl fmt::Debug for Error { } } -enum Repr { +enum ErrorData { Os(i32), Simple(ErrorKind), - // &str is a fat pointer, but &&str is a thin pointer. - SimpleMessage(ErrorKind, &'static &'static str), - Custom(Box), + SimpleMessage(&'static SimpleMessage), + Custom(C), +} + +#[repr(align(4))] +#[doc(hidden)] +pub(crate) struct SimpleMessage { + kind: ErrorKind, + message: &'static str, +} + +impl SimpleMessage { + pub(crate) const fn new(kind: ErrorKind, message: &'static str) -> Self { + Self { kind, message } + } +} + +/// Create and return an `io::Error` for a given `ErrorKind` and constant +/// message. This doesn't allocate. +pub(crate) macro const_io_error($kind:expr, $message:expr $(,)?) { + $crate::io::error::Error::from_static_message({ + const MESSAGE_DATA: $crate::io::error::SimpleMessage = + $crate::io::error::SimpleMessage::new($kind, $message); + &MESSAGE_DATA + }) } #[derive(Debug)] +#[repr(align(4))] struct Custom { kind: ErrorKind, error: Box, @@ -396,7 +422,7 @@ impl From for Error { /// ``` #[inline] fn from(kind: ErrorKind) -> Error { - Error { repr: Repr::Simple(kind) } + Error { repr: Repr::new_simple(kind) } } } @@ -461,20 +487,22 @@ impl Error { } fn _new(kind: ErrorKind, error: Box) -> Error { - Error { repr: Repr::Custom(Box::new(Custom { kind, error })) } + Error { repr: Repr::new_custom(Box::new(Custom { kind, error })) } } - /// Creates a new I/O error from a known kind of error as well as a - /// constant message. + /// Creates a new I/O error from a known kind of error as well as a constant + /// message. /// /// This function does not allocate. /// - /// This function should maybe change to - /// `new_const(kind: ErrorKind)` - /// in the future, when const generics allow that. + /// You should not use this directly, and instead use the `const_io_error!` + /// macro: `io::const_io_error!(ErrorKind::Something, "some_message")`. + /// + /// This function should maybe change to `from_static_message(kind: ErrorKind)` in the future, when const generics allow that. #[inline] - pub(crate) const fn new_const(kind: ErrorKind, message: &'static &'static str) -> Error { - Self { repr: Repr::SimpleMessage(kind, message) } + pub(crate) const fn from_static_message(msg: &'static SimpleMessage) -> Error { + Self { repr: Repr::new_simple_message(msg) } } /// Returns an error representing the last OS error which occurred. @@ -532,7 +560,7 @@ impl Error { #[must_use] #[inline] pub fn from_raw_os_error(code: i32) -> Error { - Error { repr: Repr::Os(code) } + Error { repr: Repr::new_os(code) } } /// Returns the OS error that this error represents (if any). @@ -568,11 +596,11 @@ impl Error { #[must_use] #[inline] pub fn raw_os_error(&self) -> Option { - match self.repr { - Repr::Os(i) => Some(i), - Repr::Custom(..) => None, - Repr::Simple(..) => None, - Repr::SimpleMessage(..) => None, + match self.repr.data() { + ErrorData::Os(i) => Some(i), + ErrorData::Custom(..) => None, + ErrorData::Simple(..) => None, + ErrorData::SimpleMessage(..) => None, } } @@ -607,11 +635,11 @@ impl Error { #[must_use] #[inline] pub fn get_ref(&self) -> Option<&(dyn error::Error + Send + Sync + 'static)> { - match self.repr { - Repr::Os(..) => None, - Repr::Simple(..) => None, - Repr::SimpleMessage(..) => None, - Repr::Custom(ref c) => Some(&*c.error), + match self.repr.data() { + ErrorData::Os(..) => None, + ErrorData::Simple(..) => None, + ErrorData::SimpleMessage(..) => None, + ErrorData::Custom(c) => Some(&*c.error), } } @@ -681,11 +709,11 @@ impl Error { #[must_use] #[inline] pub fn get_mut(&mut self) -> Option<&mut (dyn error::Error + Send + Sync + 'static)> { - match self.repr { - Repr::Os(..) => None, - Repr::Simple(..) => None, - Repr::SimpleMessage(..) => None, - Repr::Custom(ref mut c) => Some(&mut *c.error), + match self.repr.data_mut() { + ErrorData::Os(..) => None, + ErrorData::Simple(..) => None, + ErrorData::SimpleMessage(..) => None, + ErrorData::Custom(c) => Some(&mut *c.error), } } @@ -720,11 +748,11 @@ impl Error { #[must_use = "`self` will be dropped if the result is not used"] #[inline] pub fn into_inner(self) -> Option> { - match self.repr { - Repr::Os(..) => None, - Repr::Simple(..) => None, - Repr::SimpleMessage(..) => None, - Repr::Custom(c) => Some(c.error), + match self.repr.into_data() { + ErrorData::Os(..) => None, + ErrorData::Simple(..) => None, + ErrorData::SimpleMessage(..) => None, + ErrorData::Custom(c) => Some(c.error), } } @@ -750,29 +778,31 @@ impl Error { #[must_use] #[inline] pub fn kind(&self) -> ErrorKind { - match self.repr { - Repr::Os(code) => sys::decode_error_kind(code), - Repr::Custom(ref c) => c.kind, - Repr::Simple(kind) => kind, - Repr::SimpleMessage(kind, _) => kind, + match self.repr.data() { + ErrorData::Os(code) => sys::decode_error_kind(code), + ErrorData::Custom(ref c) => c.kind, + ErrorData::Simple(kind) => kind, + ErrorData::SimpleMessage(m) => m.kind, } } } impl fmt::Debug for Repr { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - Repr::Os(code) => fmt + match self.data() { + ErrorData::Os(code) => fmt .debug_struct("Os") .field("code", &code) .field("kind", &sys::decode_error_kind(code)) .field("message", &sys::os::error_string(code)) .finish(), - Repr::Custom(ref c) => fmt::Debug::fmt(&c, fmt), - Repr::Simple(kind) => fmt.debug_tuple("Kind").field(&kind).finish(), - Repr::SimpleMessage(kind, &message) => { - fmt.debug_struct("Error").field("kind", &kind).field("message", &message).finish() - } + ErrorData::Custom(c) => fmt::Debug::fmt(&c, fmt), + ErrorData::Simple(kind) => fmt.debug_tuple("Kind").field(&kind).finish(), + ErrorData::SimpleMessage(msg) => fmt + .debug_struct("Error") + .field("kind", &msg.kind) + .field("message", &msg.message) + .finish(), } } } @@ -780,14 +810,14 @@ impl fmt::Debug for Repr { #[stable(feature = "rust1", since = "1.0.0")] impl fmt::Display for Error { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - match self.repr { - Repr::Os(code) => { + match self.repr.data() { + ErrorData::Os(code) => { let detail = sys::os::error_string(code); write!(fmt, "{} (os error {})", detail, code) } - Repr::Custom(ref c) => c.error.fmt(fmt), - Repr::Simple(kind) => write!(fmt, "{}", kind.as_str()), - Repr::SimpleMessage(_, &msg) => msg.fmt(fmt), + ErrorData::Custom(ref c) => c.error.fmt(fmt), + ErrorData::Simple(kind) => write!(fmt, "{}", kind.as_str()), + ErrorData::SimpleMessage(msg) => msg.message.fmt(fmt), } } } @@ -796,29 +826,29 @@ impl fmt::Display for Error { impl error::Error for Error { #[allow(deprecated, deprecated_in_future)] fn description(&self) -> &str { - match self.repr { - Repr::Os(..) | Repr::Simple(..) => self.kind().as_str(), - Repr::SimpleMessage(_, &msg) => msg, - Repr::Custom(ref c) => c.error.description(), + match self.repr.data() { + ErrorData::Os(..) | ErrorData::Simple(..) => self.kind().as_str(), + ErrorData::SimpleMessage(msg) => msg.message, + ErrorData::Custom(ref c) => c.error.description(), } } #[allow(deprecated)] fn cause(&self) -> Option<&dyn error::Error> { - match self.repr { - Repr::Os(..) => None, - Repr::Simple(..) => None, - Repr::SimpleMessage(..) => None, - Repr::Custom(ref c) => c.error.cause(), + match self.repr.data() { + ErrorData::Os(..) => None, + ErrorData::Simple(..) => None, + ErrorData::SimpleMessage(..) => None, + ErrorData::Custom(ref c) => c.error.cause(), } } fn source(&self) -> Option<&(dyn error::Error + 'static)> { - match self.repr { - Repr::Os(..) => None, - Repr::Simple(..) => None, - Repr::SimpleMessage(..) => None, - Repr::Custom(ref c) => c.error.source(), + match self.repr.data() { + ErrorData::Os(..) => None, + ErrorData::Simple(..) => None, + ErrorData::SimpleMessage(..) => None, + ErrorData::Custom(ref c) => c.error.source(), } } } diff --git a/library/std/src/io/error/repr_unpacked.rs b/library/std/src/io/error/repr_unpacked.rs new file mode 100644 index 0000000000000..3729c039c42d7 --- /dev/null +++ b/library/std/src/io/error/repr_unpacked.rs @@ -0,0 +1,50 @@ +//! This is a fairly simple unpacked error representation that's used on +//! non-64bit targets, where the packed 64 bit representation wouldn't work, and +//! would have no benefit. + +use super::{Custom, ErrorData, ErrorKind, SimpleMessage}; +use alloc::boxed::Box; + +type Inner = ErrorData>; + +pub(super) struct Repr(Inner); + +impl Repr { + pub(super) fn new_custom(b: Box) -> Self { + Self(Inner::Custom(b)) + } + #[inline] + pub(super) fn new_os(code: i32) -> Self { + Self(Inner::Os(code)) + } + #[inline] + pub(super) fn new_simple(kind: ErrorKind) -> Self { + Self(Inner::Simple(kind)) + } + #[inline] + pub(super) const fn new_simple_message(m: &'static SimpleMessage) -> Self { + Self(Inner::SimpleMessage(m)) + } + #[inline] + pub(super) fn into_data(self) -> ErrorData> { + self.0 + } + #[inline] + pub(super) fn data(&self) -> ErrorData<&Custom> { + match &self.0 { + Inner::Os(c) => ErrorData::Os(*c), + Inner::Simple(k) => ErrorData::Simple(*k), + Inner::SimpleMessage(m) => ErrorData::SimpleMessage(*m), + Inner::Custom(m) => ErrorData::Custom(&*m), + } + } + #[inline] + pub(super) fn data_mut(&mut self) -> ErrorData<&mut Custom> { + match &mut self.0 { + Inner::Os(c) => ErrorData::Os(*c), + Inner::Simple(k) => ErrorData::Simple(*k), + Inner::SimpleMessage(m) => ErrorData::SimpleMessage(*m), + Inner::Custom(m) => ErrorData::Custom(&mut *m), + } + } +} diff --git a/library/std/src/io/error/tests.rs b/library/std/src/io/error/tests.rs index 5098a46313de3..5d9290f4ac4bb 100644 --- a/library/std/src/io/error/tests.rs +++ b/library/std/src/io/error/tests.rs @@ -1,4 +1,4 @@ -use super::{Custom, Error, ErrorKind, Repr}; +use super::{const_io_error, Custom, Error, ErrorKind, Repr}; use crate::error; use crate::fmt; use crate::mem::size_of; @@ -16,9 +16,9 @@ fn test_debug_error() { let msg = error_string(code); let kind = decode_error_kind(code); let err = Error { - repr: Repr::Custom(box Custom { + repr: Repr::new_custom(box Custom { kind: ErrorKind::InvalidInput, - error: box Error { repr: super::Repr::Os(code) }, + error: box Error { repr: super::Repr::new_os(code) }, }), }; let expected = format!( @@ -60,7 +60,7 @@ fn test_downcasting() { #[test] fn test_const() { - const E: Error = Error::new_const(ErrorKind::NotFound, &"hello"); + const E: Error = const_io_error!(ErrorKind::NotFound, "hello"); assert_eq!(E.kind(), ErrorKind::NotFound); assert_eq!(E.to_string(), "hello"); diff --git a/library/std/src/io/impls.rs b/library/std/src/io/impls.rs index 23201f9fc5c94..64d2457bce159 100644 --- a/library/std/src/io/impls.rs +++ b/library/std/src/io/impls.rs @@ -5,7 +5,7 @@ use crate::alloc::Allocator; use crate::cmp; use crate::fmt; use crate::io::{ - self, BufRead, Error, ErrorKind, IoSlice, IoSliceMut, Read, ReadBuf, Seek, SeekFrom, Write, + self, BufRead, ErrorKind, IoSlice, IoSliceMut, Read, ReadBuf, Seek, SeekFrom, Write, }; use crate::mem; @@ -279,7 +279,10 @@ impl Read for &[u8] { #[inline] fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { if buf.len() > self.len() { - return Err(Error::new_const(ErrorKind::UnexpectedEof, &"failed to fill whole buffer")); + return Err(io::const_io_error!( + ErrorKind::UnexpectedEof, + "failed to fill whole buffer" + )); } let (a, b) = self.split_at(buf.len()); @@ -361,7 +364,7 @@ impl Write for &mut [u8] { if self.write(data)? == data.len() { Ok(()) } else { - Err(Error::new_const(ErrorKind::WriteZero, &"failed to write whole buffer")) + Err(io::const_io_error!(ErrorKind::WriteZero, "failed to write whole buffer")) } } diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 824938ce38e68..71a59fb580321 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -282,6 +282,7 @@ pub use self::{ #[unstable(feature = "read_buf", issue = "78485")] pub use self::readbuf::ReadBuf; +pub(crate) use error::const_io_error; mod buffered; pub(crate) mod copy; @@ -337,7 +338,10 @@ where let ret = f(g.buf); if str::from_utf8(&g.buf[g.len..]).is_err() { ret.and_then(|_| { - Err(Error::new_const(ErrorKind::InvalidData, &"stream did not contain valid UTF-8")) + Err(error::const_io_error!( + ErrorKind::InvalidData, + "stream did not contain valid UTF-8" + )) }) } else { g.len = g.buf.len(); @@ -454,7 +458,7 @@ pub(crate) fn default_read_exact(this: &mut R, mut buf: &mut [ } } if !buf.is_empty() { - Err(Error::new_const(ErrorKind::UnexpectedEof, &"failed to fill whole buffer")) + Err(error::const_io_error!(ErrorKind::UnexpectedEof, "failed to fill whole buffer")) } else { Ok(()) } @@ -1512,9 +1516,9 @@ pub trait Write { while !buf.is_empty() { match self.write(buf) { Ok(0) => { - return Err(Error::new_const( + return Err(error::const_io_error!( ErrorKind::WriteZero, - &"failed to write whole buffer", + "failed to write whole buffer", )); } Ok(n) => buf = &buf[n..], @@ -1580,9 +1584,9 @@ pub trait Write { while !bufs.is_empty() { match self.write_vectored(bufs) { Ok(0) => { - return Err(Error::new_const( + return Err(error::const_io_error!( ErrorKind::WriteZero, - &"failed to write whole buffer", + "failed to write whole buffer", )); } Ok(n) => IoSlice::advance_slices(&mut bufs, n), @@ -1657,7 +1661,7 @@ pub trait Write { if output.error.is_err() { output.error } else { - Err(Error::new_const(ErrorKind::Uncategorized, &"formatter error")) + Err(error::const_io_error!(ErrorKind::Uncategorized, "formatter error")) } } } diff --git a/library/std/src/io/tests.rs b/library/std/src/io/tests.rs index ea49bfe3421d1..eb62634856462 100644 --- a/library/std/src/io/tests.rs +++ b/library/std/src/io/tests.rs @@ -185,12 +185,12 @@ fn take_eof() { impl Read for R { fn read(&mut self, _: &mut [u8]) -> io::Result { - Err(io::Error::new_const(io::ErrorKind::Other, &"")) + Err(io::const_io_error!(io::ErrorKind::Other, "")) } } impl BufRead for R { fn fill_buf(&mut self) -> io::Result<&[u8]> { - Err(io::Error::new_const(io::ErrorKind::Other, &"")) + Err(io::const_io_error!(io::ErrorKind::Other, "")) } fn consume(&mut self, _amt: usize) {} } diff --git a/library/std/src/net/mod.rs b/library/std/src/net/mod.rs index 2669f4dbf3068..f676e0a04f000 100644 --- a/library/std/src/net/mod.rs +++ b/library/std/src/net/mod.rs @@ -17,7 +17,7 @@ #![stable(feature = "rust1", since = "1.0.0")] -use crate::io::{self, Error, ErrorKind}; +use crate::io::{self, ErrorKind}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::addr::{SocketAddr, SocketAddrV4, SocketAddrV6, ToSocketAddrs}; @@ -90,6 +90,6 @@ where } } Err(last_err.unwrap_or_else(|| { - Error::new_const(ErrorKind::InvalidInput, &"could not resolve to any addresses") + io::const_io_error!(ErrorKind::InvalidInput, "could not resolve to any addresses") })) } diff --git a/library/std/src/net/udp.rs b/library/std/src/net/udp.rs index 6354752e64e76..11a696e92c825 100644 --- a/library/std/src/net/udp.rs +++ b/library/std/src/net/udp.rs @@ -2,7 +2,7 @@ mod tests; use crate::fmt; -use crate::io::{self, Error, ErrorKind}; +use crate::io::{self, ErrorKind}; use crate::net::{Ipv4Addr, Ipv6Addr, SocketAddr, ToSocketAddrs}; use crate::sys_common::net as net_imp; use crate::sys_common::{AsInner, FromInner, IntoInner}; @@ -175,7 +175,9 @@ impl UdpSocket { pub fn send_to(&self, buf: &[u8], addr: A) -> io::Result { match addr.to_socket_addrs()?.next() { Some(addr) => self.0.send_to(buf, &addr), - None => Err(Error::new_const(ErrorKind::InvalidInput, &"no addresses to send data to")), + None => { + Err(io::const_io_error!(ErrorKind::InvalidInput, "no addresses to send data to")) + } } } diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs index 0b6588db92c83..b53c3e79b0fe6 100644 --- a/library/std/src/os/fd/owned.rs +++ b/library/std/src/os/fd/owned.rs @@ -93,9 +93,9 @@ impl OwnedFd { #[cfg(target_os = "wasi")] pub fn try_clone(&self) -> crate::io::Result { - Err(crate::io::Error::new_const( + Err(crate::io::const_io_error!( crate::io::ErrorKind::Unsupported, - &"operation not supported on WASI yet", + "operation not supported on WASI yet", )) } } diff --git a/library/std/src/os/unix/fs.rs b/library/std/src/os/unix/fs.rs index 0284a428b5d74..db7edcd057432 100644 --- a/library/std/src/os/unix/fs.rs +++ b/library/std/src/os/unix/fs.rs @@ -114,7 +114,7 @@ pub trait FileExt { } } if !buf.is_empty() { - Err(io::Error::new_const(io::ErrorKind::UnexpectedEof, &"failed to fill whole buffer")) + Err(io::const_io_error!(io::ErrorKind::UnexpectedEof, "failed to fill whole buffer",)) } else { Ok(()) } @@ -196,9 +196,9 @@ pub trait FileExt { while !buf.is_empty() { match self.write_at(buf, offset) { Ok(0) => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::WriteZero, - &"failed to write whole buffer", + "failed to write whole buffer", )); } Ok(n) => { diff --git a/library/std/src/os/unix/net/addr.rs b/library/std/src/os/unix/net/addr.rs index 9dbd4548bc92d..034fa301ba1ea 100644 --- a/library/std/src/os/unix/net/addr.rs +++ b/library/std/src/os/unix/net/addr.rs @@ -30,16 +30,16 @@ pub(super) fn sockaddr_un(path: &Path) -> io::Result<(libc::sockaddr_un, libc::s let bytes = path.as_os_str().as_bytes(); if bytes.contains(&0) { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"paths must not contain interior null bytes", + "paths must not contain interior null bytes", )); } if bytes.len() >= addr.sun_path.len() { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"path must be shorter than SUN_LEN", + "path must be shorter than SUN_LEN", )); } // SAFETY: `bytes` and `addr.sun_path` are not overlapping and @@ -121,9 +121,9 @@ impl SocketAddr { // linux returns zero bytes of address len = sun_path_offset(&addr) as libc::socklen_t; // i.e., zero-length address } else if addr.sun_family != libc::AF_UNIX as libc::sa_family_t { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"file descriptor did not correspond to a Unix socket", + "file descriptor did not correspond to a Unix socket", )); } @@ -323,9 +323,9 @@ impl SocketAddr { addr.sun_family = libc::AF_UNIX as libc::sa_family_t; if namespace.len() + 1 > addr.sun_path.len() { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"namespace must be shorter than SUN_LEN", + "namespace must be shorter than SUN_LEN", )); } diff --git a/library/std/src/os/wasi/fs.rs b/library/std/src/os/wasi/fs.rs index 37126069f942b..160c8f1eca251 100644 --- a/library/std/src/os/wasi/fs.rs +++ b/library/std/src/os/wasi/fs.rs @@ -87,7 +87,7 @@ pub trait FileExt { } } if !buf.is_empty() { - Err(io::Error::new_const(io::ErrorKind::UnexpectedEof, &"failed to fill whole buffer")) + Err(io::const_io_error!(io::ErrorKind::UnexpectedEof, "failed to fill whole buffer")) } else { Ok(()) } @@ -153,9 +153,9 @@ pub trait FileExt { while !buf.is_empty() { match self.write_at(buf, offset) { Ok(0) => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::WriteZero, - &"failed to write whole buffer", + "failed to write whole buffer", )); } Ok(n) => { @@ -258,9 +258,9 @@ impl FileExt for fs::File { a if a == wasi::ADVICE_DONTNEED.raw() => wasi::ADVICE_DONTNEED, a if a == wasi::ADVICE_NOREUSE.raw() => wasi::ADVICE_NOREUSE, _ => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"invalid parameter 'advice'", + "invalid parameter 'advice'", )); } }; @@ -554,5 +554,5 @@ pub fn symlink_path, U: AsRef>(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::Uncategorized, &"input must be utf-8")) + .ok_or_else(|| io::const_io_error!(io::ErrorKind::Uncategorized, "input must be utf-8")) } diff --git a/library/std/src/os/windows/io/socket.rs b/library/std/src/os/windows/io/socket.rs index 26b569bcdd362..d3a5b6dcc76c6 100644 --- a/library/std/src/os/windows/io/socket.rs +++ b/library/std/src/os/windows/io/socket.rs @@ -135,7 +135,7 @@ impl OwnedSocket { #[cfg(target_vendor = "uwp")] pub(crate) fn set_no_inherit(&self) -> io::Result<()> { - Err(io::Error::new_const(io::ErrorKind::Unsupported, &"Unavailable on UWP")) + Err(io::const_io_error!(io::ErrorKind::Unsupported, "Unavailable on UWP")) } } diff --git a/library/std/src/sys/hermit/fs.rs b/library/std/src/sys/hermit/fs.rs index 974c44eb8dd5e..fa9a7fb19e463 100644 --- a/library/std/src/sys/hermit/fs.rs +++ b/library/std/src/sys/hermit/fs.rs @@ -226,7 +226,7 @@ impl OpenOptions { (false, _, true) => Ok(O_WRONLY | O_APPEND), (true, _, true) => Ok(O_RDWR | O_APPEND), (false, false, false) => { - Err(io::Error::new_const(ErrorKind::InvalidInput, &"invalid access mode")) + Err(io::const_io_error!(ErrorKind::InvalidInput, "invalid access mode")) } } } @@ -236,17 +236,17 @@ impl OpenOptions { (true, false) => {} (false, false) => { if self.truncate || self.create || self.create_new { - return Err(io::Error::new_const( + return Err(io::const_io_error!( ErrorKind::InvalidInput, - &"invalid creation mode", + "invalid creation mode", )); } } (_, true) => { if self.truncate && !self.create_new { - return Err(io::Error::new_const( + return Err(io::const_io_error!( ErrorKind::InvalidInput, - &"invalid creation mode", + "invalid creation mode", )); } } diff --git a/library/std/src/sys/hermit/mod.rs b/library/std/src/sys/hermit/mod.rs index 185b68c0a7803..b798c97448b8f 100644 --- a/library/std/src/sys/hermit/mod.rs +++ b/library/std/src/sys/hermit/mod.rs @@ -58,9 +58,9 @@ pub fn unsupported() -> crate::io::Result { } pub fn unsupported_err() -> crate::io::Error { - crate::io::Error::new_const( + crate::io::const_io_error!( crate::io::ErrorKind::Unsupported, - &"operation not supported on HermitCore yet", + "operation not supported on HermitCore yet", ) } diff --git a/library/std/src/sys/hermit/net.rs b/library/std/src/sys/hermit/net.rs index 1a6b3bc63e6de..f65fd8e53bdc9 100644 --- a/library/std/src/sys/hermit/net.rs +++ b/library/std/src/sys/hermit/net.rs @@ -14,9 +14,9 @@ use crate::time::Duration; /// if not, starts it. pub fn init() -> io::Result<()> { if abi::network_init() < 0 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( ErrorKind::Uncategorized, - &"Unable to initialize network interface", + "Unable to initialize network interface", )); } @@ -50,9 +50,9 @@ 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( + _ => Err(io::const_io_error!( ErrorKind::Uncategorized, - &"Unable to initiate a connection on a socket", + "Unable to initiate a connection on a socket", )), } } @@ -64,9 +64,9 @@ impl TcpStream { Some(duration.as_millis() as u64), ) { Ok(handle) => Ok(TcpStream(Arc::new(Socket(handle)))), - _ => Err(io::Error::new_const( + _ => Err(io::const_io_error!( ErrorKind::Uncategorized, - &"Unable to initiate a connection on a socket", + "Unable to initiate a connection on a socket", )), } } @@ -74,7 +74,7 @@ impl TcpStream { pub fn set_read_timeout(&self, duration: Option) -> 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::Uncategorized, &"Unable to set timeout value") + io::const_io_error!(ErrorKind::Uncategorized, "Unable to set timeout value") }) } @@ -83,12 +83,12 @@ impl TcpStream { *self.0.as_inner(), duration.map(|d| d.as_millis() as u64), ) - .map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"Unable to set timeout value")) + .map_err(|_| io::const_io_error!(ErrorKind::Uncategorized, "Unable to set timeout value")) } pub fn read_timeout(&self) -> io::Result> { let duration = abi::tcpstream::get_read_timeout(*self.0.as_inner()).map_err(|_| { - io::Error::new_const(ErrorKind::Uncategorized, &"Unable to determine timeout value") + io::const_io_error!(ErrorKind::Uncategorized, "Unable to determine timeout value") })?; Ok(duration.map(|d| Duration::from_millis(d))) @@ -96,7 +96,7 @@ impl TcpStream { pub fn write_timeout(&self) -> io::Result> { let duration = abi::tcpstream::get_write_timeout(*self.0.as_inner()).map_err(|_| { - io::Error::new_const(ErrorKind::Uncategorized, &"Unable to determine timeout value") + io::const_io_error!(ErrorKind::Uncategorized, "Unable to determine timeout value") })?; Ok(duration.map(|d| Duration::from_millis(d))) @@ -104,7 +104,7 @@ impl TcpStream { pub fn peek(&self, buf: &mut [u8]) -> io::Result { abi::tcpstream::peek(*self.0.as_inner(), buf) - .map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"peek failed")) + .map_err(|_| io::const_io_error!(ErrorKind::Uncategorized, "peek failed")) } pub fn read(&self, buffer: &mut [u8]) -> io::Result { @@ -116,7 +116,7 @@ impl TcpStream { 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::Uncategorized, &"Unable to read on socket") + io::const_io_error!(ErrorKind::Uncategorized, "Unable to read on socket") })?; if ret != 0 { @@ -141,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::Uncategorized, &"Unable to write on socket") + io::const_io_error!(ErrorKind::Uncategorized, "Unable to write on socket") })?; } @@ -155,13 +155,13 @@ impl TcpStream { pub fn peer_addr(&self) -> io::Result { let (ipaddr, port) = abi::tcpstream::peer_addr(*self.0.as_inner()) - .map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"peer_addr failed"))?; + .map_err(|_| io::const_io_error!(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::Uncategorized, &"peer_addr failed")); + return Err(io::const_io_error!(ErrorKind::Uncategorized, "peer_addr failed")); } }; @@ -173,9 +173,8 @@ 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::Uncategorized, &"unable to shutdown socket") - }) + abi::tcpstream::shutdown(*self.0.as_inner(), how as i32) + .map_err(|_| io::const_io_error!(ErrorKind::Uncategorized, "unable to shutdown socket")) } pub fn duplicate(&self) -> io::Result { @@ -192,22 +191,22 @@ 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::Uncategorized, &"set_nodelay failed")) + .map_err(|_| io::const_io_error!(ErrorKind::Uncategorized, "set_nodelay failed")) } pub fn nodelay(&self) -> io::Result { abi::tcpstream::nodelay(*self.0.as_inner()) - .map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"nodelay failed")) + .map_err(|_| io::const_io_error!(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::Uncategorized, &"unable to set TTL")) + .map_err(|_| io::const_io_error!(ErrorKind::Uncategorized, "unable to set TTL")) } pub fn ttl(&self) -> io::Result { abi::tcpstream::get_tll(*self.0.as_inner()) - .map_err(|_| io::Error::new_const(ErrorKind::Uncategorized, &"unable to get TTL")) + .map_err(|_| io::const_io_error!(ErrorKind::Uncategorized, "unable to get TTL")) } pub fn take_error(&self) -> io::Result> { @@ -216,7 +215,7 @@ impl TcpStream { 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::Uncategorized, &"unable to set blocking mode") + io::const_io_error!(ErrorKind::Uncategorized, "unable to set blocking mode") }) } } @@ -243,12 +242,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::Uncategorized, &"accept failed"))?; + .map_err(|_| io::const_io_error!(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::Uncategorized, &"accept failed")); + return Err(io::const_io_error!(ErrorKind::Uncategorized, "accept failed")); } }; diff --git a/library/std/src/sys/hermit/stdio.rs b/library/std/src/sys/hermit/stdio.rs index 33b8390431f6d..514de1df6f9c3 100644 --- a/library/std/src/sys/hermit/stdio.rs +++ b/library/std/src/sys/hermit/stdio.rs @@ -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::Uncategorized, &"Stdout is not able to print")) + Err(io::const_io_error!(io::ErrorKind::Uncategorized, "Stdout is not able to print")) } else { Ok(len as usize) } @@ -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::Uncategorized, &"Stdout is not able to print")) + Err(io::const_io_error!(io::ErrorKind::Uncategorized, "Stdout is not able to print")) } else { Ok(len as usize) } @@ -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::Uncategorized, &"Stderr is not able to print")) + Err(io::const_io_error!(io::ErrorKind::Uncategorized, "Stderr is not able to print")) } else { Ok(len as usize) } @@ -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::Uncategorized, &"Stderr is not able to print")) + Err(io::const_io_error!(io::ErrorKind::Uncategorized, "Stderr is not able to print")) } else { Ok(len as usize) } diff --git a/library/std/src/sys/hermit/thread.rs b/library/std/src/sys/hermit/thread.rs index 81b21fbbb1656..e53a1fea6a0dc 100644 --- a/library/std/src/sys/hermit/thread.rs +++ b/library/std/src/sys/hermit/thread.rs @@ -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::Uncategorized, &"Unable to create thread!")) + Err(io::const_io_error!(io::ErrorKind::Uncategorized, "Unable to create thread!")) } else { Ok(Thread { tid: tid }) }; diff --git a/library/std/src/sys/sgx/mod.rs b/library/std/src/sys/sgx/mod.rs index a2a763c2b4e0f..158c92e7a77d4 100644 --- a/library/std/src/sys/sgx/mod.rs +++ b/library/std/src/sys/sgx/mod.rs @@ -58,7 +58,7 @@ pub fn unsupported() -> crate::io::Result { } pub fn unsupported_err() -> crate::io::Error { - crate::io::Error::new_const(ErrorKind::Unsupported, &"operation not supported on SGX yet") + crate::io::const_io_error!(ErrorKind::Unsupported, "operation not supported on SGX yet") } /// This function is used to implement various functions that doesn't exist, @@ -69,9 +69,9 @@ pub fn unsupported_err() -> crate::io::Error { pub fn sgx_ineffective(v: T) -> crate::io::Result { static SGX_INEFFECTIVE_ERROR: AtomicBool = AtomicBool::new(false); if SGX_INEFFECTIVE_ERROR.load(Ordering::Relaxed) { - Err(crate::io::Error::new_const( + Err(crate::io::const_io_error!( ErrorKind::Uncategorized, - &"operation can't be trusted to have any effect on SGX", + "operation can't be trusted to have any effect on SGX", )) } else { Ok(v) diff --git a/library/std/src/sys/sgx/net.rs b/library/std/src/sys/sgx/net.rs index 89c5af6124f20..d14990c6877af 100644 --- a/library/std/src/sys/sgx/net.rs +++ b/library/std/src/sys/sgx/net.rs @@ -97,9 +97,9 @@ impl TcpStream { pub fn connect_timeout(addr: &SocketAddr, dur: Duration) -> io::Result { if dur == Duration::default() { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } Self::connect(Ok(addr)) // FIXME: ignoring timeout @@ -108,9 +108,9 @@ impl TcpStream { pub fn set_read_timeout(&self, dur: Option) -> io::Result<()> { match dur { Some(dur) if dur == Duration::default() => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } _ => sgx_ineffective(()), @@ -120,9 +120,9 @@ impl TcpStream { pub fn set_write_timeout(&self, dur: Option) -> io::Result<()> { match dur { Some(dur) if dur == Duration::default() => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } _ => sgx_ineffective(()), diff --git a/library/std/src/sys/solid/fs.rs b/library/std/src/sys/solid/fs.rs index 8a0eeff0c4d75..a6ed10f7789d2 100644 --- a/library/std/src/sys/solid/fs.rs +++ b/library/std/src/sys/solid/fs.rs @@ -461,7 +461,7 @@ impl fmt::Debug for File { pub fn unlink(p: &Path) -> io::Result<()> { if stat(p)?.file_type().is_dir() { - Err(io::Error::new_const(io::ErrorKind::IsADirectory, &"is a directory")) + Err(io::const_io_error!(io::ErrorKind::IsADirectory, "is a directory")) } else { error::SolidError::err_if_negative(unsafe { abi::SOLID_FS_Unlink(cstr(p)?.as_ptr()) }) .map_err(|e| e.as_io_error())?; @@ -491,7 +491,7 @@ pub fn rmdir(p: &Path) -> io::Result<()> { .map_err(|e| e.as_io_error())?; Ok(()) } else { - Err(io::Error::new_const(io::ErrorKind::NotADirectory, &"not a directory")) + Err(io::const_io_error!(io::ErrorKind::NotADirectory, "not a directory")) } } @@ -511,7 +511,7 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> { pub fn readlink(p: &Path) -> io::Result { // This target doesn't support symlinks stat(p)?; - Err(io::Error::new_const(io::ErrorKind::InvalidInput, &"not a symbolic link")) + Err(io::const_io_error!(io::ErrorKind::InvalidInput, "not a symbolic link")) } pub fn symlink(_original: &Path, _link: &Path) -> io::Result<()> { diff --git a/library/std/src/sys/solid/mod.rs b/library/std/src/sys/solid/mod.rs index 211b8d7de31f5..2082c9401535e 100644 --- a/library/std/src/sys/solid/mod.rs +++ b/library/std/src/sys/solid/mod.rs @@ -57,9 +57,9 @@ pub fn unsupported() -> crate::io::Result { } pub fn unsupported_err() -> crate::io::Error { - crate::io::Error::new_const( + crate::io::const_io_error!( crate::io::ErrorKind::Unsupported, - &"operation not supported on this platform", + "operation not supported on this platform", ) } diff --git a/library/std/src/sys/solid/net.rs b/library/std/src/sys/solid/net.rs index c91ecce4d728b..a43407bd0f865 100644 --- a/library/std/src/sys/solid/net.rs +++ b/library/std/src/sys/solid/net.rs @@ -243,9 +243,9 @@ impl Socket { } if timeout.as_secs() == 0 && timeout.subsec_nanos() == 0 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } @@ -271,7 +271,7 @@ impl Socket { }; match n { - 0 => Err(io::Error::new_const(io::ErrorKind::TimedOut, &"connection timed out")), + 0 => Err(io::const_io_error!(io::ErrorKind::TimedOut, "connection timed out")), _ => { let can_write = writefds.num_fds != 0; if !can_write { @@ -364,9 +364,9 @@ impl Socket { let timeout = match dur { Some(dur) => { if dur.as_secs() == 0 && dur.subsec_nanos() == 0 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } diff --git a/library/std/src/sys/solid/os.rs b/library/std/src/sys/solid/os.rs index 82542d81e6709..22239e1fa8ebc 100644 --- a/library/std/src/sys/solid/os.rs +++ b/library/std/src/sys/solid/os.rs @@ -173,11 +173,7 @@ pub fn unsetenv(n: &OsStr) -> io::Result<()> { /// In kmclib, `setenv` and `unsetenv` don't always set `errno`, so this /// function just returns a generic error. fn cvt_env(t: c_int) -> io::Result { - if t == -1 { - Err(io::Error::new_const(io::ErrorKind::Uncategorized, &"failure")) - } else { - Ok(t) - } + if t == -1 { Err(io::const_io_error!(io::ErrorKind::Uncategorized, "failure")) } else { Ok(t) } } pub fn temp_dir() -> PathBuf { diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 878796065c8f1..8bd0b9b14afed 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -387,17 +387,17 @@ impl FileAttr { tv_nsec: ext.stx_btime.tv_nsec as _, })) } else { - Err(io::Error::new_const( + Err(io::const_io_error!( io::ErrorKind::Uncategorized, - &"creation time is not available for the filesystem", + "creation time is not available for the filesystem", )) }; } } - Err(io::Error::new_const( + Err(io::const_io_error!( io::ErrorKind::Unsupported, - &"creation time is not available on this platform \ + "creation time is not available on this platform \ currently", )) } diff --git a/library/std/src/sys/unix/l4re.rs b/library/std/src/sys/unix/l4re.rs index ba63b41534c1a..d13e1ecbbfed4 100644 --- a/library/std/src/sys/unix/l4re.rs +++ b/library/std/src/sys/unix/l4re.rs @@ -1,8 +1,8 @@ macro_rules! unimpl { () => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::Unsupported, - &"No networking available on L4Re.", + "No networking available on L4Re.", )); }; } diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs index 2ba6c8d830ede..6382354eb6ebd 100644 --- a/library/std/src/sys/unix/mod.rs +++ b/library/std/src/sys/unix/mod.rs @@ -322,9 +322,6 @@ mod unsupported { } pub fn unsupported_err() -> io::Error { - io::Error::new_const( - io::ErrorKind::Unsupported, - &"operation not supported on this platform", - ) + io::const_io_error!(io::ErrorKind::Unsupported, "operation not supported on this platform",) } } diff --git a/library/std/src/sys/unix/net.rs b/library/std/src/sys/unix/net.rs index a82a0172126d4..61c15ecd85de3 100644 --- a/library/std/src/sys/unix/net.rs +++ b/library/std/src/sys/unix/net.rs @@ -154,9 +154,9 @@ impl Socket { let mut pollfd = libc::pollfd { fd: self.as_raw_fd(), events: libc::POLLOUT, revents: 0 }; if timeout.as_secs() == 0 && timeout.subsec_nanos() == 0 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } @@ -165,7 +165,7 @@ impl Socket { loop { let elapsed = start.elapsed(); if elapsed >= timeout { - return Err(io::Error::new_const(io::ErrorKind::TimedOut, &"connection timed out")); + return Err(io::const_io_error!(io::ErrorKind::TimedOut, "connection timed out")); } let timeout = timeout - elapsed; @@ -192,9 +192,9 @@ impl Socket { // for POLLHUP rather than read readiness if pollfd.revents & libc::POLLHUP != 0 { let e = self.take_error()?.unwrap_or_else(|| { - io::Error::new_const( + io::const_io_error!( io::ErrorKind::Uncategorized, - &"no error set after POLLHUP", + "no error set after POLLHUP", ) }); return Err(e); @@ -338,9 +338,9 @@ impl Socket { let timeout = match dur { Some(dur) => { if dur.as_secs() == 0 && dur.subsec_nanos() == 0 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs index 7466c77356c7c..b268ef5c36400 100644 --- a/library/std/src/sys/unix/os.rs +++ b/library/std/src/sys/unix/os.rs @@ -294,9 +294,9 @@ pub fn current_exe() -> io::Result { 0, ))?; if path_len <= 1 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::Uncategorized, - &"KERN_PROC_PATHNAME sysctl returned zero-length string", + "KERN_PROC_PATHNAME sysctl returned zero-length string", )); } let mut path: Vec = Vec::with_capacity(path_len); @@ -317,9 +317,9 @@ pub fn current_exe() -> io::Result { if curproc_exe.is_file() { return crate::fs::read_link(curproc_exe); } - Err(io::Error::new_const( + Err(io::const_io_error!( io::ErrorKind::Uncategorized, - &"/proc/curproc/exe doesn't point to regular file.", + "/proc/curproc/exe doesn't point to regular file.", )) } sysctl().or_else(|_| procfs()) @@ -336,9 +336,9 @@ pub fn current_exe() -> io::Result { cvt(libc::sysctl(mib, 4, argv.as_mut_ptr() as *mut _, &mut argv_len, ptr::null_mut(), 0))?; argv.set_len(argv_len as usize); if argv[0].is_null() { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::Uncategorized, - &"no current exe available", + "no current exe available", )); } let argv0 = CStr::from_ptr(argv[0]).to_bytes(); @@ -353,9 +353,9 @@ pub fn current_exe() -> io::Result { #[cfg(any(target_os = "linux", target_os = "android", target_os = "emscripten"))] pub fn current_exe() -> io::Result { match crate::fs::read_link("/proc/self/exe") { - Err(ref e) if e.kind() == io::ErrorKind::NotFound => Err(io::Error::new_const( + Err(ref e) if e.kind() == io::ErrorKind::NotFound => Err(io::const_io_error!( io::ErrorKind::Uncategorized, - &"no /proc/self/exe available. Is /proc mounted?", + "no /proc/self/exe available. Is /proc mounted?", )), other => other, } @@ -417,7 +417,7 @@ pub fn current_exe() -> io::Result { ); if result != 0 { use crate::io::ErrorKind; - Err(io::Error::new_const(ErrorKind::Uncategorized, &"Error getting executable path")) + Err(io::const_io_error!(ErrorKind::Uncategorized, "Error getting executable path")) } else { let name = CStr::from_ptr((*info.as_ptr()).name.as_ptr()).to_bytes(); Ok(PathBuf::from(OsStr::from_bytes(name))) @@ -433,7 +433,7 @@ pub fn current_exe() -> io::Result { #[cfg(any(target_os = "fuchsia", target_os = "l4re"))] pub fn current_exe() -> io::Result { use crate::io::ErrorKind; - Err(io::Error::new_const(ErrorKind::Unsupported, &"Not yet implemented!")) + Err(io::const_io_error!(ErrorKind::Unsupported, "Not yet implemented!")) } #[cfg(target_os = "vxworks")] diff --git a/library/std/src/sys/unix/process/process_fuchsia.rs b/library/std/src/sys/unix/process/process_fuchsia.rs index ce77c210a6220..09bfd9680f5b2 100644 --- a/library/std/src/sys/unix/process/process_fuchsia.rs +++ b/library/std/src/sys/unix/process/process_fuchsia.rs @@ -23,9 +23,9 @@ impl Command { let envp = self.capture_env(); if self.saw_nul() { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"nul byte found in provided data", + "nul byte found in provided data", )); } @@ -38,9 +38,9 @@ impl Command { pub fn exec(&mut self, default: Stdio) -> io::Error { if self.saw_nul() { - return io::Error::new_const( + return io::const_io_error!( io::ErrorKind::InvalidInput, - &"nul byte found in provided data", + "nul byte found in provided data", ); } @@ -186,9 +186,9 @@ impl Process { ))?; } if actual != 1 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidData, - &"Failed to get exit status of process", + "Failed to get exit status of process", )); } Ok(ExitStatus(proc_info.return_code)) @@ -224,9 +224,9 @@ impl Process { ))?; } if actual != 1 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidData, - &"Failed to get exit status of process", + "Failed to get exit status of process", )); } Ok(Some(ExitStatus(proc_info.return_code))) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index bce35b380e677..9fc2d9fce4dc4 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -44,9 +44,9 @@ impl Command { let envp = self.capture_env(); if self.saw_nul() { - return Err(io::Error::new_const( + return Err(io::const_io_error!( ErrorKind::InvalidInput, - &"nul byte found in provided data", + "nul byte found in provided data", )); } @@ -222,10 +222,7 @@ impl Command { let envp = self.capture_env(); if self.saw_nul() { - return io::Error::new_const( - ErrorKind::InvalidInput, - &"nul byte found in provided data", - ); + return io::const_io_error!(ErrorKind::InvalidInput, "nul byte found in provided data",); } match self.setup_io(default, true) { @@ -581,9 +578,9 @@ impl Process { // and used for another process, and we probably shouldn't be killing // random processes, so just return an error. if self.status.is_some() { - Err(Error::new_const( + Err(io::const_io_error!( ErrorKind::InvalidInput, - &"invalid argument: can't kill an exited process", + "invalid argument: can't kill an exited process", )) } else { cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop) diff --git a/library/std/src/sys/unix/process/process_vxworks.rs b/library/std/src/sys/unix/process/process_vxworks.rs index c17822f512532..c6714d3aae246 100644 --- a/library/std/src/sys/unix/process/process_vxworks.rs +++ b/library/std/src/sys/unix/process/process_vxworks.rs @@ -24,9 +24,9 @@ impl Command { let envp = self.capture_env(); if self.saw_nul() { - return Err(io::Error::new_const( + return Err(io::const_io_error!( ErrorKind::InvalidInput, - &"nul byte found in provided data", + "nul byte found in provided data", )); } let (ours, theirs) = self.setup_io(default, needs_stdin)?; @@ -142,9 +142,9 @@ impl Process { // and used for another process, and we probably shouldn't be killing // random processes, so just return an error. if self.status.is_some() { - Err(Error::new_const( + Err(io::const_io_error!( ErrorKind::InvalidInput, - &"invalid argument: can't kill an exited process", + "invalid argument: can't kill an exited process", )) } else { cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(drop) diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 9e02966b57c0d..cf8cf5ad49f73 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -287,7 +287,7 @@ pub fn available_parallelism() -> io::Result { } match unsafe { libc::sysconf(libc::_SC_NPROCESSORS_ONLN) } { -1 => Err(io::Error::last_os_error()), - 0 => Err(io::Error::new_const(io::ErrorKind::NotFound, &"The number of hardware threads is not known for the target platform")), + 0 => Err(io::const_io_error!(io::ErrorKind::NotFound, "The number of hardware threads is not known for the target platform")), cpus => Ok(unsafe { NonZeroUsize::new_unchecked(cpus as usize) }), } } else if #[cfg(any(target_os = "freebsd", target_os = "dragonfly", target_os = "netbsd"))] { @@ -318,7 +318,7 @@ pub fn available_parallelism() -> io::Result { if res == -1 { return Err(io::Error::last_os_error()); } else if cpus == 0 { - return Err(io::Error::new_const(io::ErrorKind::NotFound, &"The number of hardware threads is not known for the target platform")); + return Err(io::const_io_error!(io::ErrorKind::NotFound, "The number of hardware threads is not known for the target platform")); } } Ok(unsafe { NonZeroUsize::new_unchecked(cpus as usize) }) @@ -344,7 +344,7 @@ pub fn available_parallelism() -> io::Result { if res == -1 { return Err(io::Error::last_os_error()); } else if cpus == 0 { - return Err(io::Error::new_const(io::ErrorKind::NotFound, &"The number of hardware threads is not known for the target platform")); + return Err(io::const_io_error!(io::ErrorKind::NotFound, "The number of hardware threads is not known for the target platform")); } Ok(unsafe { NonZeroUsize::new_unchecked(cpus as usize) }) @@ -356,14 +356,14 @@ pub fn available_parallelism() -> io::Result { let res = libc::get_system_info(&mut sinfo); if res != libc::B_OK { - return Err(io::Error::new_const(io::ErrorKind::NotFound, &"The number of hardware threads is not known for the target platform")); + return Err(io::const_io_error!(io::ErrorKind::NotFound, "The number of hardware threads is not known for the target platform")); } Ok(NonZeroUsize::new_unchecked(sinfo.cpu_count as usize)) } } else { // FIXME: implement on vxWorks, Redox, l4re - Err(io::Error::new_const(io::ErrorKind::Unsupported, &"Getting the number of hardware threads is not supported on the target platform")) + Err(io::const_io_error!(io::ErrorKind::Unsupported, "Getting the number of hardware threads is not supported on the target platform")) } } } diff --git a/library/std/src/sys/unsupported/common.rs b/library/std/src/sys/unsupported/common.rs index a06b44e96a923..5274f53a7dbdb 100644 --- a/library/std/src/sys/unsupported/common.rs +++ b/library/std/src/sys/unsupported/common.rs @@ -21,9 +21,9 @@ pub fn unsupported() -> std_io::Result { } pub fn unsupported_err() -> std_io::Error { - std_io::Error::new_const( + std_io::const_io_error!( std_io::ErrorKind::Unsupported, - &"operation not supported on this platform", + "operation not supported on this platform", ) } diff --git a/library/std/src/sys/unsupported/os.rs b/library/std/src/sys/unsupported/os.rs index 2886ec1180e54..e150ae143ad99 100644 --- a/library/std/src/sys/unsupported/os.rs +++ b/library/std/src/sys/unsupported/os.rs @@ -81,11 +81,11 @@ pub fn getenv(_: &OsStr) -> Option { } pub fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> { - Err(io::Error::new_const(io::ErrorKind::Unsupported, &"cannot set env vars on this platform")) + Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot set env vars on this platform")) } pub fn unsetenv(_: &OsStr) -> io::Result<()> { - Err(io::Error::new_const(io::ErrorKind::Unsupported, &"cannot unset env vars on this platform")) + Err(io::const_io_error!(io::ErrorKind::Unsupported, "cannot unset env vars on this platform")) } pub fn temp_dir() -> PathBuf { diff --git a/library/std/src/sys/wasi/fs.rs b/library/std/src/sys/wasi/fs.rs index 5924789d12ba4..cd6815bfc2136 100644 --- a/library/std/src/sys/wasi/fs.rs +++ b/library/std/src/sys/wasi/fs.rs @@ -711,7 +711,7 @@ fn open_parent(p: &Path) -> io::Result<(ManuallyDrop, PathBuf)> { pub fn osstr2str(f: &OsStr) -> io::Result<&str> { f.to_str() - .ok_or_else(|| io::Error::new_const(io::ErrorKind::Uncategorized, &"input must be utf-8")) + .ok_or_else(|| io::const_io_error!(io::ErrorKind::Uncategorized, "input must be utf-8")) } pub fn copy(from: &Path, to: &Path) -> io::Result { @@ -757,7 +757,7 @@ fn remove_dir_all_recursive(parent: &WasiFd, path: &Path) -> io::Result<()> { for entry in ReadDir::new(fd, dummy_root) { let entry = entry?; let path = crate::str::from_utf8(&entry.name).map_err(|_| { - io::Error::new_const(io::ErrorKind::Uncategorized, &"invalid utf-8 file name found") + io::const_io_error!(io::ErrorKind::Uncategorized, "invalid utf-8 file name found") })?; if entry.file_type()?.is_dir() { diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index 028b6b30099dd..fed655af87e63 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -511,9 +511,9 @@ impl File { ) } _ => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::Uncategorized, - &"Unsupported reparse point type", + "Unsupported reparse point type", )); } }; @@ -1124,9 +1124,9 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> { #[cfg(target_vendor = "uwp")] pub fn link(_original: &Path, _link: &Path) -> io::Result<()> { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::Unsupported, - &"hard link are not supported on UWP", + "hard link are not supported on UWP", )); } diff --git a/library/std/src/sys/windows/mod.rs b/library/std/src/sys/windows/mod.rs index ad4492f9d1f92..c70f254cf39f8 100644 --- a/library/std/src/sys/windows/mod.rs +++ b/library/std/src/sys/windows/mod.rs @@ -160,9 +160,9 @@ pub fn to_u16s>(s: S) -> crate::io::Result> { fn inner(s: &OsStr) -> crate::io::Result> { let mut maybe_result: Vec = s.encode_wide().collect(); if unrolled_find_u16s(0, &maybe_result).is_some() { - return Err(crate::io::Error::new_const( + return Err(crate::io::const_io_error!( ErrorKind::InvalidInput, - &"strings passed to WinAPI cannot contain NULs", + "strings passed to WinAPI cannot contain NULs", )); } maybe_result.push(0); diff --git a/library/std/src/sys/windows/net.rs b/library/std/src/sys/windows/net.rs index 14d5f15d20248..aa6400aeefa0d 100644 --- a/library/std/src/sys/windows/net.rs +++ b/library/std/src/sys/windows/net.rs @@ -152,9 +152,9 @@ impl Socket { match result { Err(ref error) if error.kind() == io::ErrorKind::WouldBlock => { if timeout.as_secs() == 0 && timeout.subsec_nanos() == 0 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } @@ -185,9 +185,7 @@ impl Socket { }; match count { - 0 => { - Err(io::Error::new_const(io::ErrorKind::TimedOut, &"connection timed out")) - } + 0 => Err(io::const_io_error!(io::ErrorKind::TimedOut, "connection timed out")), _ => { if writefds.fd_count != 1 { if let Some(e) = self.take_error()? { @@ -353,9 +351,9 @@ impl Socket { Some(dur) => { let timeout = sys::dur2timeout(dur); if timeout == 0 { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"cannot set a 0 duration timeout", + "cannot set a 0 duration timeout", )); } timeout diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index 5ad570427978e..c6f641d0932bf 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -149,7 +149,7 @@ impl AsRef for EnvKey { fn ensure_no_nuls>(str: T) -> io::Result { if str.as_ref().encode_wide().any(|b| b == 0) { - Err(io::Error::new_const(ErrorKind::InvalidInput, &"nul byte found in provided data")) + Err(io::const_io_error!(ErrorKind::InvalidInput, "nul byte found in provided data")) } else { Ok(str) } @@ -369,9 +369,9 @@ fn resolve_exe<'a>( ) -> io::Result { // Early return if there is no filename. if exe_path.is_empty() || path::has_trailing_slash(exe_path) { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidInput, - &"program path has no file name", + "program path has no file name", )); } // Test if the file name has the `exe` extension. @@ -422,7 +422,7 @@ fn resolve_exe<'a>( } } // If we get here then the executable cannot be found. - Err(io::Error::new_const(io::ErrorKind::NotFound, &"program not found")) + Err(io::const_io_error!(io::ErrorKind::NotFound, "program not found")) } // Calls `f` for every path that should be used to find an executable. diff --git a/library/std/src/sys/windows/stdio.rs b/library/std/src/sys/windows/stdio.rs index 684b8e3155e84..a001d6b985823 100644 --- a/library/std/src/sys/windows/stdio.rs +++ b/library/std/src/sys/windows/stdio.rs @@ -110,9 +110,9 @@ fn write( if data[0] >> 6 != 0b10 { // not a continuation byte - reject incomplete_utf8.len = 0; - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidData, - &"Windows stdio in console mode does not support writing non-UTF-8 byte sequences", + "Windows stdio in console mode does not support writing non-UTF-8 byte sequences", )); } incomplete_utf8.bytes[incomplete_utf8.len as usize] = data[0]; @@ -132,9 +132,9 @@ fn write( return Ok(1); } Err(_) => { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidData, - &"Windows stdio in console mode does not support writing non-UTF-8 byte sequences", + "Windows stdio in console mode does not support writing non-UTF-8 byte sequences", )); } } @@ -156,9 +156,9 @@ fn write( incomplete_utf8.len = 1; return Ok(1); } else { - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidData, - &"Windows stdio in console mode does not support writing non-UTF-8 byte sequences", + "Windows stdio in console mode does not support writing non-UTF-8 byte sequences", )); } } @@ -364,9 +364,9 @@ fn utf16_to_utf8(utf16: &[u16], utf8: &mut [u8]) -> io::Result { } Err(_) => { // We can't really do any better than forget all data and return an error. - return Err(io::Error::new_const( + return Err(io::const_io_error!( io::ErrorKind::InvalidData, - &"Windows stdin in console mode does not support non-UTF-16 input; \ + "Windows stdin in console mode does not support non-UTF-16 input; \ encountered unpaired surrogate", )); } diff --git a/library/std/src/sys/windows/thread.rs b/library/std/src/sys/windows/thread.rs index 75f70c2076ee1..e4bba9255d23e 100644 --- a/library/std/src/sys/windows/thread.rs +++ b/library/std/src/sys/windows/thread.rs @@ -107,9 +107,9 @@ pub fn available_parallelism() -> io::Result { sysinfo.dwNumberOfProcessors as usize }; match res { - 0 => Err(io::Error::new_const( + 0 => Err(io::const_io_error!( io::ErrorKind::NotFound, - &"The number of hardware threads is not known for the target platform", + "The number of hardware threads is not known for the target platform", )), cpus => Ok(unsafe { NonZeroUsize::new_unchecked(cpus) }), } diff --git a/library/std/src/sys_common/fs.rs b/library/std/src/sys_common/fs.rs index 309f5483874e0..617ac52e51ca8 100644 --- a/library/std/src/sys_common/fs.rs +++ b/library/std/src/sys_common/fs.rs @@ -4,9 +4,9 @@ use crate::fs; use crate::io::{self, Error, ErrorKind}; use crate::path::Path; -pub(crate) const NOT_FILE_ERROR: Error = Error::new_const( +pub(crate) const NOT_FILE_ERROR: Error = io::const_io_error!( ErrorKind::InvalidInput, - &"the source path is neither a regular file nor a symlink to a regular file", + "the source path is neither a regular file nor a symlink to a regular file", ); pub fn copy(from: &Path, to: &Path) -> io::Result { diff --git a/library/std/src/sys_common/net.rs b/library/std/src/sys_common/net.rs index c5c3df361f34b..70b29d4a92ed5 100644 --- a/library/std/src/sys_common/net.rs +++ b/library/std/src/sys_common/net.rs @@ -5,7 +5,7 @@ use crate::cmp; use crate::convert::{TryFrom, TryInto}; use crate::ffi::CString; use crate::fmt; -use crate::io::{self, Error, ErrorKind, IoSlice, IoSliceMut}; +use crate::io::{self, ErrorKind, IoSlice, IoSliceMut}; use crate::mem; use crate::net::{Ipv4Addr, Ipv6Addr, Shutdown, SocketAddr}; use crate::ptr; @@ -102,7 +102,7 @@ pub fn sockaddr_to_addr(storage: &c::sockaddr_storage, len: usize) -> io::Result *(storage as *const _ as *const c::sockaddr_in6) }))) } - _ => Err(Error::new_const(ErrorKind::InvalidInput, &"invalid argument")), + _ => Err(io::const_io_error!(ErrorKind::InvalidInput, "invalid argument")), } } @@ -165,7 +165,7 @@ impl TryFrom<&str> for LookupHost { ($e:expr, $msg:expr) => { match $e { Some(r) => r, - None => return Err(io::Error::new_const(io::ErrorKind::InvalidInput, &$msg)), + None => return Err(io::const_io_error!(io::ErrorKind::InvalidInput, $msg)), } }; } From ea211695bf924015f38d4a2de4f43a822f6c6a70 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sun, 8 Aug 2021 05:13:35 -0700 Subject: [PATCH 2/9] Optimize io::error::Repr layout on 64 bit targets. --- library/std/src/io/error.rs | 15 +- library/std/src/io/error/repr_bitpacked.rs | 338 +++++++++++++++++++++ library/std/src/io/error/tests.rs | 15 + 3 files changed, 364 insertions(+), 4 deletions(-) create mode 100644 library/std/src/io/error/repr_bitpacked.rs diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 67526090e5830..449771c676148 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -1,7 +1,14 @@ #[cfg(test)] mod tests; +#[cfg(target_pointer_width = "64")] +mod repr_bitpacked; +#[cfg(target_pointer_width = "64")] +use repr_bitpacked::Repr; + +#[cfg(not(target_pointer_width = "64"))] mod repr_unpacked; +#[cfg(not(target_pointer_width = "64"))] use repr_unpacked::Repr; use crate::convert::From; @@ -780,7 +787,7 @@ impl Error { pub fn kind(&self) -> ErrorKind { match self.repr.data() { ErrorData::Os(code) => sys::decode_error_kind(code), - ErrorData::Custom(ref c) => c.kind, + ErrorData::Custom(c) => c.kind, ErrorData::Simple(kind) => kind, ErrorData::SimpleMessage(m) => m.kind, } @@ -829,7 +836,7 @@ impl error::Error for Error { match self.repr.data() { ErrorData::Os(..) | ErrorData::Simple(..) => self.kind().as_str(), ErrorData::SimpleMessage(msg) => msg.message, - ErrorData::Custom(ref c) => c.error.description(), + ErrorData::Custom(c) => c.error.description(), } } @@ -839,7 +846,7 @@ impl error::Error for Error { ErrorData::Os(..) => None, ErrorData::Simple(..) => None, ErrorData::SimpleMessage(..) => None, - ErrorData::Custom(ref c) => c.error.cause(), + ErrorData::Custom(c) => c.error.cause(), } } @@ -848,7 +855,7 @@ impl error::Error for Error { ErrorData::Os(..) => None, ErrorData::Simple(..) => None, ErrorData::SimpleMessage(..) => None, - ErrorData::Custom(ref c) => c.error.source(), + ErrorData::Custom(c) => c.error.source(), } } } diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs new file mode 100644 index 0000000000000..8ff0bcc7cb5cd --- /dev/null +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -0,0 +1,338 @@ +//! This is a densely packed error representation which is used on targets with +//! 64-bit pointers. +//! +//! (Note that `bitpacked` vs `unpacked` here has no relationship to +//! `#[repr(packed)]`, it just refers to attempting to use any available +//! bits in a more clever manner than `rustc`'s default layout algorithm would). +//! +//! Conceptually, it stores the same information as the "unpacked" equivalent we +//! use on other targets: `repr_unpacked::Repr` (see repr_unpacked.rs), however +//! it packs it into a 64bit non-zero value. +//! +//! This optimization not only allows `io::Error` to occupy a single pointer, +//! but improves `io::Result` as well, especially for situations like +//! `Result<()>` (which is now 64 bits) or `Result` (which i), which are +//! quite common. +//! +//! # Layout +//! Tagged values are 64 bits, with the 2 least significant bits used for the +//! tag. This means there are there are 4 "variants": +//! +//! - **Tag 0b00**: The first variant is equivalent to +//! `ErrorData::SimpleMessage`, and holds a `&'static SimpleMessage` directly. +//! +//! `SimpleMessage` has an alignment >= 4 (which is requested with +//! `#[repr(align)]` and checked statically at the bottom of this file), which +//! means every `&'static SimpleMessage` should have the both tag bits as 0, +//! meaning its tagged and untagged representation are equivalent. +//! +//! This means we can skip tagging it, which is necessary as this variant can +//! be constructed from a `const fn`, which probably cannot tag pointers (or +//! at least it would be difficult. +//! +//! - **Tag 0b01**: The other pointer variant holds the data for +//! `ErrorData::Custom` and the remaining 62 bits are used to store a +//! `Box`. `Custom` also has alignment >= 4, so the bottom two bits +//! are free to use for the tag. +//! +//! The only important thing to note is that `ptr::add` and `ptr::sub` are +//! used to tag the pointer, rather than bitwise operations. This should +//! preserve the pointer's provenance, which would otherwise be lost. +//! +//! - **Tag 0b10**: Holds the data for `ErrorData::Os(i32)`. We store the `i32` +//! in the pointers most significant 32 bits, and don't use the bits `2..32` +//! for anything. Using the top 32 bits is just to let us easily recover the +//! `i32` code with the correct sign. +//! +//! - **Tag 0b11**: Holds the data for `ErrorData::Simple(ErrorKind)`. This +//! stores the `ErrorKind` in the top 32 bits as well, although it doesn't +//! occupy nearly that many. Most of the bits are unused here, but it's not +//! like we need them for anything else yet. +//! +//! # Use of `NonNull<()>` +//! +//! Everything is stored in a `NonNull<()>`, which is odd, but actually serves a +//! purpose. +//! +//! Conceptually you might think of this more like: +//! +//! ```ignore +//! union Repr { +//! // holds integer (Simple/Os) variants, and +//! // provides access to the tag bits. +//! bits: NonZeroU64, +//! // Tag is 0, so this is stored untagged. +//! msg: &'static SimpleMessage, +//! // Tagged (offset) `Box` pointer. +//! tagged_custom: NonNull<()>, +//! } +//! ``` +//! +//! But there are a few problems with this: +//! +//! 1. Union access is equivalent to a transmute, so this representation would +//! require we transmute between integers and pointers in at least one +//! direction, which may be UB (and even if not, it is likely harder for a +//! compiler to reason about than explicit ptr->int operations). +//! +//! 2. Even if all fields of a union have a niche, the union itself doesn't, +//! although this may change in the future. This would make things like +//! `io::Result<()>` and `io::Result` larger, which defeats part of +//! the motivation of this bitpacking. +//! +//! Storing everything in a `NonZeroUsize` (or some other integer) would be a +//! bit more traditional for pointer tagging, but it would lose provenance +//! information, couldn't be constructed from a `const fn`, and would probably +//! run into other issues as well. +//! +//! The `NonNull<()>` seems like the only alternative, even if it's fairly odd +//! to use a pointer type to store something that may hold an integer, some of +//! the time. + +use super::{Custom, ErrorData, ErrorKind, SimpleMessage}; +use alloc::boxed::Box; +use core::mem::{align_of, size_of}; +use core::ptr::NonNull; + +// The 2 least-significant bits are used as tag. +const TAG_MASK: usize = 0b11; +const TAG_SIMPLE_MESSAGE: usize = 0b00; +const TAG_CUSTOM: usize = 0b01; +const TAG_OS: usize = 0b10; +const TAG_SIMPLE: usize = 0b11; + +#[repr(transparent)] +pub(super) struct Repr(NonNull<()>); + +// All the types `Repr` stores internally are Send + Sync, and so is it. +unsafe impl Send for Repr {} +unsafe impl Sync for Repr {} + +impl Repr { + pub(super) fn new_custom(b: Box) -> Self { + let p = Box::into_raw(b).cast::(); + // Should only be possible if an allocator handed out a pointer with + // wrong alignment. + debug_assert_eq!((p as usize & TAG_MASK), 0); + // Safety: We know `TAG_CUSTOM <= size_of::()` (static_assert at + // end of file), and both the start and end of the expression must be + // valid without address space wraparound due to `Box`'s semantics. + // Note: `add` is used as a provenance-preserving way of pointer tagging. + let tagged = unsafe { p.add(TAG_CUSTOM).cast::<()>() }; + // Safety: the above safety comment also means the result can't be null. + let res = Self(unsafe { NonNull::new_unchecked(tagged) }); + // quickly smoke-check we encoded the right thing (This generally will + // only run in libstd's tests, unless the user uses -Zbuild-std) + debug_assert!(matches!(res.data(), ErrorData::Custom(_)), "repr(custom) encoding failed"); + res + } + + #[inline] + pub(super) fn new_os(code: i32) -> Self { + let utagged = ((code as usize) << 32) | TAG_OS; + // Safety: `TAG_OS` is not zero, so the result of the `|` is not 0. + let res = Self(unsafe { NonNull::new_unchecked(utagged as *mut ()) }); + // quickly smoke-check we encoded the right thing (This generally will + // only run in libstd's tests, unless the user uses -Zbuild-std) + debug_assert!( + matches!(res.data(), ErrorData::Os(c) if c == code), + "repr(os) encoding failed for {}", + code, + ); + res + } + + #[inline] + pub(super) fn new_simple(kind: ErrorKind) -> Self { + let utagged = ((kind as usize) << 32) | TAG_SIMPLE; + // Safety: `TAG_SIMPLE` is not zero, so the result of the `|` is not 0. + let res = Self(unsafe { NonNull::new_unchecked(utagged as *mut ()) }); + // quickly smoke-check we encoded the right thing (This generally will + // only run in libstd's tests, unless the user uses -Zbuild-std) + debug_assert!( + matches!(res.data(), ErrorData::Simple(k) if k == kind), + "repr(simple) encoding failed {:?}", + kind, + ); + res + } + + #[inline] + pub(super) const fn new_simple_message(m: &'static SimpleMessage) -> Self { + // Safety: We're a Repr, decode_repr is fine. + Self(unsafe { NonNull::new_unchecked(m as *const _ as *mut ()) }) + } + + #[inline] + pub(super) fn data(&self) -> ErrorData<&Custom> { + // Safety: We're a Repr, decode_repr is fine. + unsafe { decode_repr(self.0, |c| &*c) } + } + + #[inline] + pub(super) fn data_mut(&mut self) -> ErrorData<&mut Custom> { + // Safety: We're a Repr, decode_repr is fine. + unsafe { decode_repr(self.0, |c| &mut *c) } + } + + #[inline] + pub(super) fn into_data(self) -> ErrorData> { + let this = core::mem::ManuallyDrop::new(self); + // Safety: We're a Repr, decode_repr is fine. The `Box::from_raw` is + // safe because we prevent double-drop using `ManuallyDrop`. + unsafe { decode_repr(this.0, |p| Box::from_raw(p)) } + } +} + +impl Drop for Repr { + #[inline] + fn drop(&mut self) { + // Safety: We're a Repr, decode_repr is fine. The `Box::from_raw` is + // safe because we're being dropped. + unsafe { + let _ = decode_repr(self.0, |p| Box::::from_raw(p)); + } + } +} + +// Shared helper to decode a `Repr`'s internal pointer into an ErrorData. +// +// Safety: `ptr`'s bits should be encoded as described in the document at the +// top (it should `some_repr.0`) +#[inline] +unsafe fn decode_repr(ptr: NonNull<()>, make_custom: F) -> ErrorData +where + F: FnOnce(*mut Custom) -> C, +{ + let bits = ptr.as_ptr() as usize; + match bits & TAG_MASK { + TAG_OS => { + let code = ((bits as i64) >> 32) as i32; + ErrorData::Os(code) + } + TAG_SIMPLE => { + let kind_bits = (bits >> 32) as u32; + let kind = kind_from_prim(kind_bits).unwrap_or_else(|| { + debug_assert!(false, "Invalid io::error::Repr bits: `Repr({:#016x})`", bits); + // This means the `ptr` passed in was not valid, which voilates + // the unsafe contract of `decode_repr`. + // + // Using this rather than unwrap meaningfully improves the code + // for callers which only care about one variant (usually + // `Custom`) + core::hint::unreachable_unchecked(); + }); + ErrorData::Simple(kind) + } + TAG_SIMPLE_MESSAGE => ErrorData::SimpleMessage(&*ptr.cast::().as_ptr()), + TAG_CUSTOM => { + let custom = ptr.as_ptr().cast::().sub(TAG_CUSTOM).cast::(); + ErrorData::Custom(make_custom(custom)) + } + _ => { + // Can't happen, and compiler can tell + unreachable!(); + } + } +} + +// This compiles to the same code as the check+transmute, but doesn't require +// unsafe, or to hard-code max ErrorKind or its size in a way the compiler +// couldn't verify. +#[inline] +fn kind_from_prim(ek: u32) -> Option { + macro_rules! from_prim { + ($prim:expr => $Enum:ident { $($Variant:ident),* $(,)? }) => {{ + // Force a compile error if the list gets out of date. + const _: fn(e: $Enum) = |e: $Enum| match e { + $($Enum::$Variant => ()),* + }; + match $prim { + $(v if v == ($Enum::$Variant as _) => Some($Enum::$Variant),)* + _ => None, + } + }} + } + from_prim!(ek => ErrorKind { + NotFound, + PermissionDenied, + ConnectionRefused, + ConnectionReset, + HostUnreachable, + NetworkUnreachable, + ConnectionAborted, + NotConnected, + AddrInUse, + AddrNotAvailable, + NetworkDown, + BrokenPipe, + AlreadyExists, + WouldBlock, + NotADirectory, + IsADirectory, + DirectoryNotEmpty, + ReadOnlyFilesystem, + FilesystemLoop, + StaleNetworkFileHandle, + InvalidInput, + InvalidData, + TimedOut, + WriteZero, + StorageFull, + NotSeekable, + FilesystemQuotaExceeded, + FileTooLarge, + ResourceBusy, + ExecutableFileBusy, + Deadlock, + CrossesDevices, + TooManyLinks, + FilenameTooLong, + ArgumentListTooLong, + Interrupted, + Other, + UnexpectedEof, + Unsupported, + OutOfMemory, + Uncategorized, + }) +} + +// Some static checking to alert us if a change breaks any of the assumptions +// that our encoding relies on. If any of these are hit on a platform that +// libstd supports, we should just make sure `repr_unpacked.rs` is used. +macro_rules! static_assert { + ($condition:expr) => { + const _: [(); 0] = [(); (!$condition) as usize]; + }; +} + +// The bitpacking we use requires pointers be exactly 64 bits. +static_assert!(size_of::>() == 8); + +// We also require pointers and usize be the same size. +static_assert!(size_of::>() == size_of::()); + +// `Custom` and `SimpleMessage` need to be thin pointers. +static_assert!(size_of::<&'static SimpleMessage>() == 8); +static_assert!(size_of::>() == 8); + +// And they must have >= 4 byte alignment. +static_assert!(align_of::() >= 4); +static_assert!(align_of::() >= 4); + +// This is obviously true (`TAG_CUSTOM` is `0b01`), but our implementation of +// `Repr::new_custom` and such would be UB if it were not, so we check. +static_assert!(size_of::() >= TAG_CUSTOM); +// These two store a payload which is allowed to be zero, so they must be +// non-zero to preserve the `NonNull`'s range invariant. +static_assert!(TAG_OS != 0); +static_assert!(TAG_SIMPLE != 0); +// We can't tag `SimpleMessage`s, the tag must be 0. +static_assert!(TAG_SIMPLE_MESSAGE == 0); + +// Check that the point of all of this still holds. +static_assert!(size_of::() == 8); +static_assert!(size_of::>() == 8); +static_assert!(size_of::>() == 8); +static_assert!(size_of::>() == 16); diff --git a/library/std/src/io/error/tests.rs b/library/std/src/io/error/tests.rs index 5d9290f4ac4bb..81657033f5cb4 100644 --- a/library/std/src/io/error/tests.rs +++ b/library/std/src/io/error/tests.rs @@ -67,3 +67,18 @@ fn test_const() { assert!(format!("{:?}", E).contains("\"hello\"")); assert!(format!("{:?}", E).contains("NotFound")); } + +#[test] +fn test_error_packing() { + for code in -20i32..20i32 { + let e = Error::from_raw_os_error(code); + assert_eq!(e.raw_os_error(), Some(code)); + } + assert_eq!(Error::from(ErrorKind::NotFound).kind(), ErrorKind::NotFound); + assert_eq!(Error::from(ErrorKind::Uncategorized).kind(), ErrorKind::Uncategorized); + assert_eq!(Error::from(ErrorKind::NotFound).kind(), ErrorKind::NotFound); + assert_eq!(Error::from(ErrorKind::Uncategorized).kind(), ErrorKind::Uncategorized); + let dunno = const_io_error!(ErrorKind::Uncategorized, "dunno"); + assert_eq!(dunno.kind(), ErrorKind::Uncategorized); + assert!(format!("{:?}", dunno).contains("dunno")) +} From 6b068437cb47dd9ca783171525ae891f383190ae Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Mon, 9 Aug 2021 02:19:28 -0700 Subject: [PATCH 3/9] Address address comments, improve comments slightly --- library/std/src/io/error.rs | 7 +++++++ library/std/src/io/error/repr_bitpacked.rs | 18 +++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 449771c676148..8f82778e90b5b 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -83,6 +83,10 @@ enum ErrorData { Custom(C), } +// `#[repr(align(4))]` is probably redundant, it should have that value or +// higher already. We include it just because repr_bitpacked.rs's encoding +// requires an alignment >= 4 (note that `#[repr(align)]` will not reduce the +// alignment required by the struct, only increase it). #[repr(align(4))] #[doc(hidden)] pub(crate) struct SimpleMessage { @@ -106,6 +110,9 @@ pub(crate) macro const_io_error($kind:expr, $message:expr $(,)?) { }) } +// As with `SimpleMessage`: `#[repr(align(4))]` here is just because +// repr_bitpacked's encoding requires it. In practice it almost certainly be +// already be this high or higher. #[derive(Debug)] #[repr(align(4))] struct Custom { diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index 8ff0bcc7cb5cd..281cbdc23e4fa 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -56,7 +56,7 @@ //! //! Conceptually you might think of this more like: //! -//! ```ignore +//! ```ignore (exposition-only) //! union Repr { //! // holds integer (Simple/Os) variants, and //! // provides access to the tag bits. @@ -159,7 +159,7 @@ impl Repr { #[inline] pub(super) const fn new_simple_message(m: &'static SimpleMessage) -> Self { - // Safety: We're a Repr, decode_repr is fine. + // Safety: References are never null. Self(unsafe { NonNull::new_unchecked(m as *const _ as *mut ()) }) } @@ -213,7 +213,7 @@ where TAG_SIMPLE => { let kind_bits = (bits >> 32) as u32; let kind = kind_from_prim(kind_bits).unwrap_or_else(|| { - debug_assert!(false, "Invalid io::error::Repr bits: `Repr({:#016x})`", bits); + debug_assert!(false, "Invalid io::error::Repr bits: `Repr({:#018x})`", bits); // This means the `ptr` passed in was not valid, which voilates // the unsafe contract of `decode_repr`. // @@ -299,8 +299,11 @@ fn kind_from_prim(ek: u32) -> Option { } // Some static checking to alert us if a change breaks any of the assumptions -// that our encoding relies on. If any of these are hit on a platform that -// libstd supports, we should just make sure `repr_unpacked.rs` is used. +// that our encoding relies on for correctness and soundness. (Some of these are +// a bit overly thorough/cautious, admittedly) +// +// If any of these are hit on a platform that libstd supports, we should just +// make sure `repr_unpacked.rs` is used instead. macro_rules! static_assert { ($condition:expr) => { const _: [(); 0] = [(); (!$condition) as usize]; @@ -332,6 +335,11 @@ static_assert!(TAG_SIMPLE != 0); static_assert!(TAG_SIMPLE_MESSAGE == 0); // Check that the point of all of this still holds. +// +// We'd check against `io::Error`, but *technically* it's allowed to vary, +// as it's not `#[repr(transparent)]`/`#[repr(C)]`. We could add that, but +// the `#[repr()]` would show up in rustdoc, which might be seen as a stable +// commitment. static_assert!(size_of::() == 8); static_assert!(size_of::>() == 8); static_assert!(size_of::>() == 8); From 9f7eb7d4732e1acfa461439bf8ae30753271419f Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 10 Aug 2021 10:46:02 -0700 Subject: [PATCH 4/9] Fix comment typos noticed by code review. Co-authored-by: Ralf Jung --- library/std/src/io/error/repr_bitpacked.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index 281cbdc23e4fa..b61a64f4f9a07 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -28,7 +28,7 @@ //! //! This means we can skip tagging it, which is necessary as this variant can //! be constructed from a `const fn`, which probably cannot tag pointers (or -//! at least it would be difficult. +//! at least it would be difficult). //! //! - **Tag 0b01**: The other pointer variant holds the data for //! `ErrorData::Custom` and the remaining 62 bits are used to store a @@ -40,7 +40,7 @@ //! preserve the pointer's provenance, which would otherwise be lost. //! //! - **Tag 0b10**: Holds the data for `ErrorData::Os(i32)`. We store the `i32` -//! in the pointers most significant 32 bits, and don't use the bits `2..32` +//! in the pointer's most significant 32 bits, and don't use the bits `2..32` //! for anything. Using the top 32 bits is just to let us easily recover the //! `i32` code with the correct sign. //! From 06edf082c313de1424b06928457d76576302ba2f Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 10 Aug 2021 11:07:30 -0700 Subject: [PATCH 5/9] Update library/std/src/io/error/repr_bitpacked.rs Co-authored-by: the8472 --- library/std/src/io/error/repr_bitpacked.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index b61a64f4f9a07..9b7c34f67a0d3 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -214,7 +214,7 @@ where let kind_bits = (bits >> 32) as u32; let kind = kind_from_prim(kind_bits).unwrap_or_else(|| { debug_assert!(false, "Invalid io::error::Repr bits: `Repr({:#018x})`", bits); - // This means the `ptr` passed in was not valid, which voilates + // This means the `ptr` passed in was not valid, which violates // the unsafe contract of `decode_repr`. // // Using this rather than unwrap meaningfully improves the code From f950edbef773c101bf1ed79fb82e3704e1fb9ebc Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Fri, 29 Oct 2021 19:35:09 -0700 Subject: [PATCH 6/9] Elaborate some in the documentation and respond to some review comments --- library/std/src/io/error.rs | 1 - library/std/src/io/error/repr_bitpacked.rs | 26 ++++++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 8f82778e90b5b..5f0a8a72c02c5 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -88,7 +88,6 @@ enum ErrorData { // requires an alignment >= 4 (note that `#[repr(align)]` will not reduce the // alignment required by the struct, only increase it). #[repr(align(4))] -#[doc(hidden)] pub(crate) struct SimpleMessage { kind: ErrorKind, message: &'static str, diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index 9b7c34f67a0d3..dbe6c34d5477f 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -2,17 +2,29 @@ //! 64-bit pointers. //! //! (Note that `bitpacked` vs `unpacked` here has no relationship to -//! `#[repr(packed)]`, it just refers to attempting to use any available -//! bits in a more clever manner than `rustc`'s default layout algorithm would). +//! `#[repr(packed)]`, it just refers to attempting to use any available bits in +//! a more clever manner than `rustc`'s default layout algorithm would). //! -//! Conceptually, it stores the same information as the "unpacked" equivalent we -//! use on other targets: `repr_unpacked::Repr` (see repr_unpacked.rs), however -//! it packs it into a 64bit non-zero value. +//! Conceptually, it stores the same data as the "unpacked" equivalent we use on +//! other targets. Specifically, you can imagine it as an optimized following +//! data (which is equivalent to what's stored by `repr_unpacked::Repr`, e.g. +//! `super::ErrorData>`): +//! +//! ```ignore (exposition-only) +//! enum ErrorData { +//! Os(i32), +//! Simple(ErrorKind), +//! SimpleMessage(&'static SimpleMessage), +//! Custom(Box), +//! } +//! ``` +//! +//! However, it packs this data into a 64bit non-zero value. //! //! This optimization not only allows `io::Error` to occupy a single pointer, //! but improves `io::Result` as well, especially for situations like -//! `Result<()>` (which is now 64 bits) or `Result` (which i), which are -//! quite common. +//! `io::Result<()>` (which is now 64 bits) or `io::Result` (which is now +//! 128 bits), which are quite common. //! //! # Layout //! Tagged values are 64 bits, with the 2 least significant bits used for the From e98c7f720901f6a375bba749d08a75354c347ba6 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sat, 30 Oct 2021 04:22:10 -0700 Subject: [PATCH 7/9] Use wrapping pointer arithmetic in the bitpacked io::Error --- library/std/src/io/error/repr_bitpacked.rs | 23 ++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index dbe6c34d5477f..6f7b4ac19cc77 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -47,9 +47,10 @@ //! `Box`. `Custom` also has alignment >= 4, so the bottom two bits //! are free to use for the tag. //! -//! The only important thing to note is that `ptr::add` and `ptr::sub` are -//! used to tag the pointer, rather than bitwise operations. This should -//! preserve the pointer's provenance, which would otherwise be lost. +//! The only important thing to note is that `ptr::wrapping_add` and +//! `ptr::wrapping_sub` are used to tag the pointer, rather than bitwise +//! operations. This should preserve the pointer's provenance, which would +//! otherwise be lost. //! //! - **Tag 0b10**: Holds the data for `ErrorData::Os(i32)`. We store the `i32` //! in the pointer's most significant 32 bits, and don't use the bits `2..32` @@ -126,11 +127,14 @@ impl Repr { // Should only be possible if an allocator handed out a pointer with // wrong alignment. debug_assert_eq!((p as usize & TAG_MASK), 0); - // Safety: We know `TAG_CUSTOM <= size_of::()` (static_assert at + // Note: We know `TAG_CUSTOM <= size_of::()` (static_assert at // end of file), and both the start and end of the expression must be // valid without address space wraparound due to `Box`'s semantics. - // Note: `add` is used as a provenance-preserving way of pointer tagging. - let tagged = unsafe { p.add(TAG_CUSTOM).cast::<()>() }; + // + // This means it would be correct to implement this using `ptr::add` + // (rather than `ptr::wrapping_add`), but it's unclear this would give + // any benefit, so we just use `wrapping_add` instead. + let tagged = p.wrapping_add(TAG_CUSTOM).cast::<()>(); // Safety: the above safety comment also means the result can't be null. let res = Self(unsafe { NonNull::new_unchecked(tagged) }); // quickly smoke-check we encoded the right thing (This generally will @@ -238,7 +242,10 @@ where } TAG_SIMPLE_MESSAGE => ErrorData::SimpleMessage(&*ptr.cast::().as_ptr()), TAG_CUSTOM => { - let custom = ptr.as_ptr().cast::().sub(TAG_CUSTOM).cast::(); + // It would be correct for us to use `ptr::sub` here (see the + // comment above the `wrapping_add` call in `new_custom` for why), + // but it isn't clear that it makes a difference, so we don't. + let custom = ptr.as_ptr().cast::().wrapping_sub(TAG_CUSTOM).cast::(); ErrorData::Custom(make_custom(custom)) } _ => { @@ -337,7 +344,7 @@ static_assert!(align_of::() >= 4); static_assert!(align_of::() >= 4); // This is obviously true (`TAG_CUSTOM` is `0b01`), but our implementation of -// `Repr::new_custom` and such would be UB if it were not, so we check. +// `Repr::new_custom` and such would be wrong if it were not, so we check. static_assert!(size_of::() >= TAG_CUSTOM); // These two store a payload which is allowed to be zero, so they must be // non-zero to preserve the `NonNull`'s range invariant. From a17a896d097796af88cc184b391159c74958b9b3 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Mon, 31 Jan 2022 12:43:15 -0800 Subject: [PATCH 8/9] Update documentation somewhat --- library/std/src/io/error.rs | 10 ++++++++ library/std/src/io/error/repr_bitpacked.rs | 28 ++++++++++++---------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 5f0a8a72c02c5..e901d374a6f04 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -87,6 +87,16 @@ enum ErrorData { // higher already. We include it just because repr_bitpacked.rs's encoding // requires an alignment >= 4 (note that `#[repr(align)]` will not reduce the // alignment required by the struct, only increase it). +// +// If we add more variants to ErrorData, this can be increased to 8, but it +// should probably be behind `#[cfg_attr(target_pointer_width = "64", ...)]` or +// whatever cfg we're using to enable the `repr_bitpacked` code, since only the +// that version needs the alignment, and 8 is higher than the alignment we'll +// have on 32 bit platforms. +// +// (For the sake of being explicit: the alignment requirement here only matters +// if `error/repr_bitpacked.rs` is in use — for the unpacked repr it doesn't +// matter at all) #[repr(align(4))] pub(crate) struct SimpleMessage { kind: ErrorKind, diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index 6f7b4ac19cc77..a4d29b0ce43a2 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -321,23 +321,26 @@ fn kind_from_prim(ek: u32) -> Option { // that our encoding relies on for correctness and soundness. (Some of these are // a bit overly thorough/cautious, admittedly) // -// If any of these are hit on a platform that libstd supports, we should just -// make sure `repr_unpacked.rs` is used instead. +// If any of these are hit on a platform that libstd supports, we should likely +// just use `repr_unpacked.rs` there instead (unless the fix is easy). macro_rules! static_assert { ($condition:expr) => { - const _: [(); 0] = [(); (!$condition) as usize]; + const _: () = assert!($condition); + }; + (@usize_eq: $lhs:expr, $rhs:expr) => { + const _: [(); $lhs] = [(); $rhs]; }; } // The bitpacking we use requires pointers be exactly 64 bits. -static_assert!(size_of::>() == 8); +static_assert!(@usize_eq: size_of::>(), 8); // We also require pointers and usize be the same size. -static_assert!(size_of::>() == size_of::()); +static_assert!(@usize_eq: size_of::>(), size_of::()); // `Custom` and `SimpleMessage` need to be thin pointers. -static_assert!(size_of::<&'static SimpleMessage>() == 8); -static_assert!(size_of::>() == 8); +static_assert!(@usize_eq: size_of::<&'static SimpleMessage>(), 8); +static_assert!(@usize_eq: size_of::>(), 8); // And they must have >= 4 byte alignment. static_assert!(align_of::() >= 4); @@ -346,12 +349,13 @@ static_assert!(align_of::() >= 4); // This is obviously true (`TAG_CUSTOM` is `0b01`), but our implementation of // `Repr::new_custom` and such would be wrong if it were not, so we check. static_assert!(size_of::() >= TAG_CUSTOM); + // These two store a payload which is allowed to be zero, so they must be // non-zero to preserve the `NonNull`'s range invariant. static_assert!(TAG_OS != 0); static_assert!(TAG_SIMPLE != 0); // We can't tag `SimpleMessage`s, the tag must be 0. -static_assert!(TAG_SIMPLE_MESSAGE == 0); +static_assert!(@usize_eq: TAG_SIMPLE_MESSAGE, 0); // Check that the point of all of this still holds. // @@ -359,7 +363,7 @@ static_assert!(TAG_SIMPLE_MESSAGE == 0); // as it's not `#[repr(transparent)]`/`#[repr(C)]`. We could add that, but // the `#[repr()]` would show up in rustdoc, which might be seen as a stable // commitment. -static_assert!(size_of::() == 8); -static_assert!(size_of::>() == 8); -static_assert!(size_of::>() == 8); -static_assert!(size_of::>() == 16); +static_assert!(@usize_eq: size_of::(), 8); +static_assert!(@usize_eq: size_of::>(), 8); +static_assert!(@usize_eq: size_of::>(), 8); +static_assert!(@usize_eq: size_of::>(), 16); From 9cbe99488bbeb8bdd742a4d15fe484ee665f9363 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Fri, 4 Feb 2022 22:38:29 -0800 Subject: [PATCH 9/9] Add more tests for io::Error packing, and fix some comments that weren't quite accurate anymore --- library/std/src/io/error.rs | 4 ++ library/std/src/io/error/repr_bitpacked.rs | 40 +++++++++--- library/std/src/io/error/tests.rs | 73 +++++++++++++++++++--- 3 files changed, 101 insertions(+), 16 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index e901d374a6f04..4b55324a2424c 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -76,6 +76,9 @@ impl fmt::Debug for Error { } } +// Only derive debug in tests, to make sure it +// doesn't accidentally get printed. +#[cfg_attr(test, derive(Debug))] enum ErrorData { Os(i32), Simple(ErrorKind), @@ -98,6 +101,7 @@ enum ErrorData { // if `error/repr_bitpacked.rs` is in use — for the unpacked repr it doesn't // matter at all) #[repr(align(4))] +#[derive(Debug)] pub(crate) struct SimpleMessage { kind: ErrorKind, message: &'static str, diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index a4d29b0ce43a2..f317368e8e59d 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -6,9 +6,9 @@ //! a more clever manner than `rustc`'s default layout algorithm would). //! //! Conceptually, it stores the same data as the "unpacked" equivalent we use on -//! other targets. Specifically, you can imagine it as an optimized following -//! data (which is equivalent to what's stored by `repr_unpacked::Repr`, e.g. -//! `super::ErrorData>`): +//! other targets. Specifically, you can imagine it as an optimized version of +//! the following enum (which is roughly equivalent to what's stored by +//! `repr_unpacked::Repr`, e.g. `super::ErrorData>`): //! //! ```ignore (exposition-only) //! enum ErrorData { @@ -135,7 +135,16 @@ impl Repr { // (rather than `ptr::wrapping_add`), but it's unclear this would give // any benefit, so we just use `wrapping_add` instead. let tagged = p.wrapping_add(TAG_CUSTOM).cast::<()>(); - // Safety: the above safety comment also means the result can't be null. + // Safety: `TAG_CUSTOM + p` is the same as `TAG_CUSTOM | p`, + // because `p`'s alignment means it isn't allowed to have any of the + // `TAG_BITS` set (you can verify that addition and bitwise-or are the + // same when the operands have no bits in common using a truth table). + // + // Then, `TAG_CUSTOM | p` is not zero, as that would require + // `TAG_CUSTOM` and `p` both be zero, and neither is (as `p` came from a + // box, and `TAG_CUSTOM` just... isn't zero -- it's `0b01`). Therefore, + // `TAG_CUSTOM + p` isn't zero and so `tagged` can't be, and the + // `new_unchecked` is safe. let res = Self(unsafe { NonNull::new_unchecked(tagged) }); // quickly smoke-check we encoded the right thing (This generally will // only run in libstd's tests, unless the user uses -Zbuild-std) @@ -342,12 +351,25 @@ static_assert!(@usize_eq: size_of::>(), size_of::()); static_assert!(@usize_eq: size_of::<&'static SimpleMessage>(), 8); static_assert!(@usize_eq: size_of::>(), 8); -// And they must have >= 4 byte alignment. -static_assert!(align_of::() >= 4); -static_assert!(align_of::() >= 4); +static_assert!((TAG_MASK + 1).is_power_of_two()); +// And they must have sufficient alignment. +static_assert!(align_of::() >= TAG_MASK + 1); +static_assert!(align_of::() >= TAG_MASK + 1); + +static_assert!(@usize_eq: (TAG_MASK & TAG_SIMPLE_MESSAGE), TAG_SIMPLE_MESSAGE); +static_assert!(@usize_eq: (TAG_MASK & TAG_CUSTOM), TAG_CUSTOM); +static_assert!(@usize_eq: (TAG_MASK & TAG_OS), TAG_OS); +static_assert!(@usize_eq: (TAG_MASK & TAG_SIMPLE), TAG_SIMPLE); -// This is obviously true (`TAG_CUSTOM` is `0b01`), but our implementation of -// `Repr::new_custom` and such would be wrong if it were not, so we check. +// This is obviously true (`TAG_CUSTOM` is `0b01`), but in `Repr::new_custom` we +// offset a pointer by this value, and expect it to both be within the same +// object, and to not wrap around the address space. See the comment in that +// function for further details. +// +// Actually, at the moment we use `ptr::wrapping_add`, not `ptr::add`, so this +// check isn't needed for that one, although the assertion that we don't +// actually wrap around in that wrapping_add does simplify the safety reasoning +// elsewhere considerably. static_assert!(size_of::() >= TAG_CUSTOM); // These two store a payload which is allowed to be zero, so they must be diff --git a/library/std/src/io/error/tests.rs b/library/std/src/io/error/tests.rs index 81657033f5cb4..c2c51553b208c 100644 --- a/library/std/src/io/error/tests.rs +++ b/library/std/src/io/error/tests.rs @@ -1,4 +1,5 @@ -use super::{const_io_error, Custom, Error, ErrorKind, Repr}; +use super::{const_io_error, Custom, Error, ErrorData, ErrorKind, Repr}; +use crate::assert_matches::assert_matches; use crate::error; use crate::fmt; use crate::mem::size_of; @@ -69,16 +70,74 @@ fn test_const() { } #[test] -fn test_error_packing() { +fn test_os_packing() { for code in -20i32..20i32 { let e = Error::from_raw_os_error(code); assert_eq!(e.raw_os_error(), Some(code)); + assert_matches!( + e.repr.data(), + ErrorData::Os(c) if c == code, + ); } +} + +#[test] +fn test_errorkind_packing() { assert_eq!(Error::from(ErrorKind::NotFound).kind(), ErrorKind::NotFound); + assert_eq!(Error::from(ErrorKind::PermissionDenied).kind(), ErrorKind::PermissionDenied); assert_eq!(Error::from(ErrorKind::Uncategorized).kind(), ErrorKind::Uncategorized); - assert_eq!(Error::from(ErrorKind::NotFound).kind(), ErrorKind::NotFound); - assert_eq!(Error::from(ErrorKind::Uncategorized).kind(), ErrorKind::Uncategorized); - let dunno = const_io_error!(ErrorKind::Uncategorized, "dunno"); - assert_eq!(dunno.kind(), ErrorKind::Uncategorized); - assert!(format!("{:?}", dunno).contains("dunno")) + // Check that the innards look like like what we want. + assert_matches!( + Error::from(ErrorKind::OutOfMemory).repr.data(), + ErrorData::Simple(ErrorKind::OutOfMemory), + ); +} + +#[test] +fn test_simple_message_packing() { + use super::{ErrorKind::*, SimpleMessage}; + macro_rules! check_simple_msg { + ($err:expr, $kind:ident, $msg:literal) => {{ + let e = &$err; + // Check that the public api is right. + assert_eq!(e.kind(), $kind); + assert!(format!("{:?}", e).contains($msg)); + // and we got what we expected + assert_matches!( + e.repr.data(), + ErrorData::SimpleMessage(SimpleMessage { kind: $kind, message: $msg }) + ); + }}; + } + + let not_static = const_io_error!(Uncategorized, "not a constant!"); + check_simple_msg!(not_static, Uncategorized, "not a constant!"); + + const CONST: Error = const_io_error!(NotFound, "definitely a constant!"); + check_simple_msg!(CONST, NotFound, "definitely a constant!"); + + static STATIC: Error = const_io_error!(BrokenPipe, "a constant, sort of!"); + check_simple_msg!(STATIC, BrokenPipe, "a constant, sort of!"); +} + +#[derive(Debug, PartialEq)] +struct Bojji(bool); +impl error::Error for Bojji {} +impl fmt::Display for Bojji { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "ah! {:?}", self) + } +} + +#[test] +fn test_custom_error_packing() { + use super::Custom; + let test = Error::new(ErrorKind::Uncategorized, Bojji(true)); + assert_matches!( + test.repr.data(), + ErrorData::Custom(Custom { + kind: ErrorKind::Uncategorized, + error, + }) if error.downcast_ref::().as_deref() == Some(&Bojji(true)), + ); }