Skip to content

Commit

Permalink
Rollup merge of rust-lang#83353 - m-ou-se:io-error-avoid-alloc, r=nagisa
Browse files Browse the repository at this point in the history
Add internal io::Error::new_const to avoid allocations.

This makes it possible to have a io::Error containing a message with zero allocations, and uses that everywhere to avoid the *three* allocations involved in `io::Error::new(kind, "message")`.

The function signature isn't perfect, because it needs a reference to the `&str`. So for now, this is just a `pub(crate)` function. Later, we'll be able to use `fn new_const<MSG: &'static str>(kind: ErrorKind)` to make that a bit better. (Then we'll also be able to use some ZST trickery if that would result in more efficient code.)

See rust-lang#83352
  • Loading branch information
Dylan-DPC committed Mar 23, 2021
2 parents e1ca3f0 + 6bbcc5b commit ac33062
Show file tree
Hide file tree
Showing 41 changed files with 272 additions and 189 deletions.
2 changes: 1 addition & 1 deletion library/std/src/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ impl fmt::Display for NulError {
impl From<NulError> for io::Error {
/// Converts a [`NulError`] into a [`io::Error`].
fn from(_: NulError) -> io::Error {
io::Error::new(io::ErrorKind::InvalidInput, "data provided contains a nul byte")
io::Error::new_const(io::ErrorKind::InvalidInput, &"data provided contains a nul byte")
}
}

Expand Down
5 changes: 4 additions & 1 deletion library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2188,7 +2188,10 @@ impl DirBuilder {
match path.parent() {
Some(p) => self.create_dir_all(p)?,
None => {
return Err(io::Error::new(io::ErrorKind::Other, "failed to create whole tree"));
return Err(io::Error::new_const(
io::ErrorKind::Other,
&"failed to create whole tree",
));
}
}
match self.inner.mkdir(path) {
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/io/buffered/bufwriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ impl<W: Write> BufWriter<W> {

match r {
Ok(0) => {
return Err(Error::new(
return Err(Error::new_const(
ErrorKind::WriteZero,
"failed to write the buffered data",
&"failed to write the buffered data",
));
}
Ok(n) => guard.consume(n),
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/io/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,9 @@ where
self.pos = n;
Ok(self.pos)
}
None => Err(Error::new(
None => Err(Error::new_const(
ErrorKind::InvalidInput,
"invalid seek to a negative or overflowing position",
&"invalid seek to a negative or overflowing position",
)),
}
}
Expand Down Expand Up @@ -328,9 +328,9 @@ fn slice_write_vectored(
// Resizing write implementation
fn vec_write(pos_mut: &mut u64, vec: &mut Vec<u8>, buf: &[u8]) -> io::Result<usize> {
let pos: usize = (*pos_mut).try_into().map_err(|_| {
Error::new(
Error::new_const(
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
Expand Down
26 changes: 26 additions & 0 deletions library/std/src/io/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ impl fmt::Debug for Error {
enum Repr {
Os(i32),
Simple(ErrorKind),
// &str is a fat pointer, but &&str is a thin pointer.
SimpleMessage(ErrorKind, &'static &'static str),
Custom(Box<Custom>),
}

Expand Down Expand Up @@ -259,6 +261,18 @@ impl Error {
Error { repr: Repr::Custom(Box::new(Custom { kind, error })) }
}

/// 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<const MSG: &'static str>(kind: ErrorKind)`
/// in the future, when const generics allow that.
pub(crate) const fn new_const(kind: ErrorKind, message: &'static &'static str) -> Error {
Self { repr: Repr::SimpleMessage(kind, message) }
}

/// Returns an error representing the last OS error which occurred.
///
/// This function reads the value of `errno` for the target platform (e.g.
Expand Down Expand Up @@ -342,6 +356,7 @@ impl Error {
Repr::Os(i) => Some(i),
Repr::Custom(..) => None,
Repr::Simple(..) => None,
Repr::SimpleMessage(..) => None,
}
}

Expand Down Expand Up @@ -377,6 +392,7 @@ impl Error {
match self.repr {
Repr::Os(..) => None,
Repr::Simple(..) => None,
Repr::SimpleMessage(..) => None,
Repr::Custom(ref c) => Some(&*c.error),
}
}
Expand Down Expand Up @@ -448,6 +464,7 @@ impl Error {
match self.repr {
Repr::Os(..) => None,
Repr::Simple(..) => None,
Repr::SimpleMessage(..) => None,
Repr::Custom(ref mut c) => Some(&mut *c.error),
}
}
Expand Down Expand Up @@ -484,6 +501,7 @@ impl Error {
match self.repr {
Repr::Os(..) => None,
Repr::Simple(..) => None,
Repr::SimpleMessage(..) => None,
Repr::Custom(c) => Some(c.error),
}
}
Expand Down Expand Up @@ -512,6 +530,7 @@ impl Error {
Repr::Os(code) => sys::decode_error_kind(code),
Repr::Custom(ref c) => c.kind,
Repr::Simple(kind) => kind,
Repr::SimpleMessage(kind, _) => kind,
}
}
}
Expand All @@ -527,6 +546,9 @@ impl fmt::Debug for Repr {
.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()
}
}
}
}
Expand All @@ -541,6 +563,7 @@ impl fmt::Display for Error {
}
Repr::Custom(ref c) => c.error.fmt(fmt),
Repr::Simple(kind) => write!(fmt, "{}", kind.as_str()),
Repr::SimpleMessage(_, &msg) => msg.fmt(fmt),
}
}
}
Expand All @@ -551,6 +574,7 @@ impl error::Error for Error {
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(),
}
}
Expand All @@ -560,6 +584,7 @@ impl error::Error for Error {
match self.repr {
Repr::Os(..) => None,
Repr::Simple(..) => None,
Repr::SimpleMessage(..) => None,
Repr::Custom(ref c) => c.error.cause(),
}
}
Expand All @@ -568,6 +593,7 @@ impl error::Error for Error {
match self.repr {
Repr::Os(..) => None,
Repr::Simple(..) => None,
Repr::SimpleMessage(..) => None,
Repr::Custom(ref c) => c.error.source(),
}
}
Expand Down
16 changes: 16 additions & 0 deletions library/std/src/io/error/tests.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
use super::{Custom, Error, ErrorKind, Repr};
use crate::error;
use crate::fmt;
use crate::mem::size_of;
use crate::sys::decode_error_kind;
use crate::sys::os::error_string;

#[test]
fn test_size() {
assert!(size_of::<Error>() <= size_of::<[usize; 2]>());
}

#[test]
fn test_debug_error() {
let code = 6;
Expand Down Expand Up @@ -51,3 +57,13 @@ fn test_downcasting() {
let extracted = err.into_inner().unwrap();
extracted.downcast::<TestError>().unwrap();
}

#[test]
fn test_const() {
const E: Error = Error::new_const(ErrorKind::NotFound, &"hello");

assert_eq!(E.kind(), ErrorKind::NotFound);
assert_eq!(E.to_string(), "hello");
assert!(format!("{:?}", E).contains("\"hello\""));
assert!(format!("{:?}", E).contains("NotFound"));
}
4 changes: 2 additions & 2 deletions library/std/src/io/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ impl Read for &[u8] {
#[inline]
fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
if buf.len() > self.len() {
return Err(Error::new(ErrorKind::UnexpectedEof, "failed to fill whole buffer"));
return Err(Error::new_const(ErrorKind::UnexpectedEof, &"failed to fill whole buffer"));
}
let (a, b) = self.split_at(buf.len());

Expand Down Expand Up @@ -345,7 +345,7 @@ impl Write for &mut [u8] {
if self.write(data)? == data.len() {
Ok(())
} else {
Err(Error::new(ErrorKind::WriteZero, "failed to write whole buffer"))
Err(Error::new_const(ErrorKind::WriteZero, &"failed to write whole buffer"))
}
}

Expand Down
16 changes: 11 additions & 5 deletions library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ where
let ret = f(g.buf);
if str::from_utf8(&g.buf[g.len..]).is_err() {
ret.and_then(|_| {
Err(Error::new(ErrorKind::InvalidData, "stream did not contain valid UTF-8"))
Err(Error::new_const(ErrorKind::InvalidData, &"stream did not contain valid UTF-8"))
})
} else {
g.len = g.buf.len();
Expand Down Expand Up @@ -429,7 +429,7 @@ pub(crate) fn default_read_exact<R: Read + ?Sized>(this: &mut R, mut buf: &mut [
}
}
if !buf.is_empty() {
Err(Error::new(ErrorKind::UnexpectedEof, "failed to fill whole buffer"))
Err(Error::new_const(ErrorKind::UnexpectedEof, &"failed to fill whole buffer"))
} else {
Ok(())
}
Expand Down Expand Up @@ -1437,7 +1437,10 @@ pub trait Write {
while !buf.is_empty() {
match self.write(buf) {
Ok(0) => {
return Err(Error::new(ErrorKind::WriteZero, "failed to write whole buffer"));
return Err(Error::new_const(
ErrorKind::WriteZero,
&"failed to write whole buffer",
));
}
Ok(n) => buf = &buf[n..],
Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
Expand Down Expand Up @@ -1502,7 +1505,10 @@ pub trait Write {
while !bufs.is_empty() {
match self.write_vectored(bufs) {
Ok(0) => {
return Err(Error::new(ErrorKind::WriteZero, "failed to write whole buffer"));
return Err(Error::new_const(
ErrorKind::WriteZero,
&"failed to write whole buffer",
));
}
Ok(n) => bufs = IoSlice::advance(bufs, n),
Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
Expand Down Expand Up @@ -1576,7 +1582,7 @@ pub trait Write {
if output.error.is_err() {
output.error
} else {
Err(Error::new(ErrorKind::Other, "formatter error"))
Err(Error::new_const(ErrorKind::Other, &"formatter error"))
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/io/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,12 @@ fn take_eof() {

impl Read for R {
fn read(&mut self, _: &mut [u8]) -> io::Result<usize> {
Err(io::Error::new(io::ErrorKind::Other, ""))
Err(io::Error::new_const(io::ErrorKind::Other, &""))
}
}
impl BufRead for R {
fn fill_buf(&mut self) -> io::Result<&[u8]> {
Err(io::Error::new(io::ErrorKind::Other, ""))
Err(io::Error::new_const(io::ErrorKind::Other, &""))
}
fn consume(&mut self, _amt: usize) {}
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/net/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ where
}
}
Err(last_err.unwrap_or_else(|| {
Error::new(ErrorKind::InvalidInput, "could not resolve to any addresses")
Error::new_const(ErrorKind::InvalidInput, &"could not resolve to any addresses")
}))
}
2 changes: 1 addition & 1 deletion library/std/src/net/udp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl UdpSocket {
pub fn send_to<A: ToSocketAddrs>(&self, buf: &[u8], addr: A) -> io::Result<usize> {
match addr.to_socket_addrs()?.next() {
Some(addr) => self.0.send_to(buf, &addr),
None => Err(Error::new(ErrorKind::InvalidInput, "no addresses to send data to")),
None => Err(Error::new_const(ErrorKind::InvalidInput, &"no addresses to send data to")),
}
}

Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/hermit/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ impl FileDesc {
self.duplicate_path(&[])
}
pub fn duplicate_path(&self, _path: &[u8]) -> io::Result<FileDesc> {
Err(io::Error::new(ErrorKind::Other, "duplicate isn't supported"))
Err(io::Error::new_const(ErrorKind::Other, &"duplicate isn't supported"))
}

pub fn nonblocking(&self) -> io::Result<bool> {
Ok(false)
}

pub fn set_cloexec(&self) -> io::Result<()> {
Err(io::Error::new(ErrorKind::Other, "cloexec isn't supported"))
Err(io::Error::new_const(ErrorKind::Other, &"cloexec isn't supported"))
}

pub fn set_nonblocking(&self, _nonblocking: bool) -> io::Result<()> {
Err(io::Error::new(ErrorKind::Other, "nonblocking isn't supported"))
Err(io::Error::new_const(ErrorKind::Other, &"nonblocking isn't supported"))
}
}

Expand Down
12 changes: 9 additions & 3 deletions library/std/src/sys/hermit/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(ErrorKind::InvalidInput, "invalid access mode"))
Err(io::Error::new_const(ErrorKind::InvalidInput, &"invalid access mode"))
}
}
}
Expand All @@ -236,12 +236,18 @@ impl OpenOptions {
(true, false) => {}
(false, false) => {
if self.truncate || self.create || self.create_new {
return Err(io::Error::new(ErrorKind::InvalidInput, "invalid creation mode"));
return Err(io::Error::new_const(
ErrorKind::InvalidInput,
&"invalid creation mode",
));
}
}
(_, true) => {
if self.truncate && !self.create_new {
return Err(io::Error::new(ErrorKind::InvalidInput, "invalid creation mode"));
return Err(io::Error::new_const(
ErrorKind::InvalidInput,
&"invalid creation mode",
));
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion library/std/src/sys/hermit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ pub fn unsupported<T>() -> crate::io::Result<T> {
}

pub fn unsupported_err() -> crate::io::Error {
crate::io::Error::new(crate::io::ErrorKind::Other, "operation not supported on HermitCore yet")
crate::io::Error::new_const(
crate::io::ErrorKind::Other,
&"operation not supported on HermitCore yet",
)
}

// This enum is used as the storage for a bunch of types which can't actually
Expand Down
Loading

0 comments on commit ac33062

Please sign in to comment.