From 321419ec42dcff403a54cd1153fefdecf50161da Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Thu, 21 Jul 2022 13:15:29 -0400 Subject: [PATCH 01/19] Box::from(slice): Clarify that contents are copied A colleague mentioned that they interpreted the old text as saying that only the pointer and the length are copied. Add a clause so it is more clear that the pointed to contents are also copied. --- library/alloc/src/boxed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index c1ceeb0deb837..d2dca6cab0304 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1480,7 +1480,7 @@ impl From<&[T]> for Box<[T]> { /// Converts a `&[T]` into a `Box<[T]>` /// /// This conversion allocates on the heap - /// and performs a copy of `slice`. + /// and performs a copy of `slice` and its contents. /// /// # Examples /// ```rust From c1aae4d27902aaaa0d8e7d1a76d030b4fc90f329 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Fri, 13 May 2022 15:06:36 +0100 Subject: [PATCH 02/19] std::io: migrate ReadBuf to BorrowBuf/BorrowCursor Signed-off-by: Nick Cameron --- library/std/src/fs.rs | 10 +- library/std/src/io/buffered/bufreader.rs | 15 +- library/std/src/io/buffered/tests.rs | 32 +-- library/std/src/io/copy.rs | 34 ++-- library/std/src/io/cursor.rs | 10 +- library/std/src/io/impls.rs | 16 +- library/std/src/io/mod.rs | 83 ++++---- library/std/src/io/readbuf.rs | 243 +++++++++++------------ library/std/src/io/readbuf/tests.rs | 172 +++++----------- library/std/src/io/tests.rs | 20 +- library/std/src/io/util.rs | 14 +- library/std/src/io/util/tests.rs | 48 ++--- library/std/src/sys/unix/fd.rs | 11 +- library/std/src/sys/unix/fs.rs | 6 +- 14 files changed, 324 insertions(+), 390 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index c8e131b6ee14c..d41f32b5b3f21 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -13,7 +13,7 @@ mod tests; use crate::ffi::OsString; use crate::fmt; -use crate::io::{self, IoSlice, IoSliceMut, Read, ReadBuf, Seek, SeekFrom, Write}; +use crate::io::{self, BorrowCursor, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write}; use crate::path::{Path, PathBuf}; use crate::sys::fs as fs_imp; use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; @@ -703,8 +703,8 @@ impl Read for File { self.inner.read_vectored(bufs) } - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - self.inner.read_buf(buf) + fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + self.inner.read_buf(cursor) } #[inline] @@ -755,8 +755,8 @@ impl Read for &File { self.inner.read(buf) } - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - self.inner.read_buf(buf) + fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + self.inner.read_buf(cursor) } fn read_vectored(&mut self, bufs: &mut [IoSliceMut<'_>]) -> io::Result { diff --git a/library/std/src/io/buffered/bufreader.rs b/library/std/src/io/buffered/bufreader.rs index f7fbaa9c27649..1f19ac11bf101 100644 --- a/library/std/src/io/buffered/bufreader.rs +++ b/library/std/src/io/buffered/bufreader.rs @@ -2,7 +2,8 @@ mod buffer; use crate::fmt; use crate::io::{ - self, BufRead, IoSliceMut, Read, ReadBuf, Seek, SeekFrom, SizeHint, DEFAULT_BUF_SIZE, + self, BorrowBuf, BorrowCursor, BufRead, IoSliceMut, Read, Seek, SeekFrom, SizeHint, + DEFAULT_BUF_SIZE, }; use buffer::Buffer; @@ -266,21 +267,21 @@ impl Read for BufReader { Ok(nread) } - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { + fn read_buf(&mut self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { // If we don't have any buffered data and we're doing a massive read // (larger than our internal buffer), bypass our internal buffer // entirely. - if self.buf.pos() == self.buf.filled() && buf.remaining() >= self.capacity() { + if self.buf.pos() == self.buf.filled() && cursor.capacity() >= self.capacity() { self.discard_buffer(); - return self.inner.read_buf(buf); + return self.inner.read_buf(cursor); } - let prev = buf.filled_len(); + let prev = cursor.written(); let mut rem = self.fill_buf()?; - rem.read_buf(buf)?; + rem.read_buf(cursor.clone())?; - self.consume(buf.filled_len() - prev); //slice impl of read_buf known to never unfill buf + self.consume(cursor.written() - prev); //slice impl of read_buf known to never unfill buf Ok(()) } diff --git a/library/std/src/io/buffered/tests.rs b/library/std/src/io/buffered/tests.rs index fe45b13263844..c93b69bf1f7c1 100644 --- a/library/std/src/io/buffered/tests.rs +++ b/library/std/src/io/buffered/tests.rs @@ -1,5 +1,5 @@ use crate::io::prelude::*; -use crate::io::{self, BufReader, BufWriter, ErrorKind, IoSlice, LineWriter, ReadBuf, SeekFrom}; +use crate::io::{self, BorrowBuf, BufReader, BufWriter, ErrorKind, IoSlice, LineWriter, SeekFrom}; use crate::mem::MaybeUninit; use crate::panic; use crate::sync::atomic::{AtomicUsize, Ordering}; @@ -61,48 +61,48 @@ fn test_buffered_reader_read_buf() { let inner: &[u8] = &[5, 6, 7, 0, 1, 2, 3, 4]; let mut reader = BufReader::with_capacity(2, inner); - let mut buf = [MaybeUninit::uninit(); 3]; - let mut buf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 3]; + let mut buf: BorrowBuf<'_> = buf.into(); - reader.read_buf(&mut buf).unwrap(); + reader.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.filled(), [5, 6, 7]); assert_eq!(reader.buffer(), []); - let mut buf = [MaybeUninit::uninit(); 2]; - let mut buf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 2]; + let mut buf: BorrowBuf<'_> = buf.into(); - reader.read_buf(&mut buf).unwrap(); + reader.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.filled(), [0, 1]); assert_eq!(reader.buffer(), []); - let mut buf = [MaybeUninit::uninit(); 1]; - let mut buf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 1]; + let mut buf: BorrowBuf<'_> = buf.into(); - reader.read_buf(&mut buf).unwrap(); + reader.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.filled(), [2]); assert_eq!(reader.buffer(), [3]); - let mut buf = [MaybeUninit::uninit(); 3]; - let mut buf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 3]; + let mut buf: BorrowBuf<'_> = buf.into(); - reader.read_buf(&mut buf).unwrap(); + reader.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.filled(), [3]); assert_eq!(reader.buffer(), []); - reader.read_buf(&mut buf).unwrap(); + reader.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.filled(), [3, 4]); assert_eq!(reader.buffer(), []); buf.clear(); - reader.read_buf(&mut buf).unwrap(); + reader.read_buf(buf.unfilled()).unwrap(); - assert_eq!(buf.filled_len(), 0); + assert!(buf.filled().is_empty()); } #[test] diff --git a/library/std/src/io/copy.rs b/library/std/src/io/copy.rs index 1a10245e4a5e3..193bcd47467c1 100644 --- a/library/std/src/io/copy.rs +++ b/library/std/src/io/copy.rs @@ -1,4 +1,4 @@ -use super::{BufWriter, ErrorKind, Read, ReadBuf, Result, Write, DEFAULT_BUF_SIZE}; +use super::{BorrowBuf, BufWriter, ErrorKind, Read, Result, Write, DEFAULT_BUF_SIZE}; use crate::mem::MaybeUninit; /// Copies the entire contents of a reader into a writer. @@ -97,37 +97,39 @@ impl BufferedCopySpec for BufWriter { loop { let buf = writer.buffer_mut(); - let mut read_buf = ReadBuf::uninit(buf.spare_capacity_mut()); + let mut read_buf: BorrowBuf<'_> = buf.spare_capacity_mut().into(); - // SAFETY: init is either 0 or the initialized_len of the previous iteration unsafe { - read_buf.assume_init(init); + // SAFETY: init is either 0 or the init_len from the previous iteration. + read_buf.set_init(init); } if read_buf.capacity() >= DEFAULT_BUF_SIZE { - match reader.read_buf(&mut read_buf) { + let mut cursor = read_buf.unfilled(); + match reader.read_buf(cursor.clone()) { Ok(()) => { - let bytes_read = read_buf.filled_len(); + let bytes_read = cursor.written(); if bytes_read == 0 { return Ok(len); } - init = read_buf.initialized_len() - bytes_read; + init = read_buf.init_len() - bytes_read; + len += bytes_read as u64; - // SAFETY: ReadBuf guarantees all of its filled bytes are init + // SAFETY: BorrowBuf guarantees all of its filled bytes are init unsafe { buf.set_len(buf.len() + bytes_read) }; - len += bytes_read as u64; + // Read again if the buffer still has enough capacity, as BufWriter itself would do // This will occur if the reader returns short reads - continue; } - Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, + Err(ref e) if e.kind() == ErrorKind::Interrupted => {} Err(e) => return Err(e), } + } else { + writer.flush_buf()?; + init = 0; } - - writer.flush_buf()?; } } } @@ -136,13 +138,13 @@ fn stack_buffer_copy( reader: &mut R, writer: &mut W, ) -> Result { - let mut buf = [MaybeUninit::uninit(); DEFAULT_BUF_SIZE]; - let mut buf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::uninit(); DEFAULT_BUF_SIZE]; + let mut buf: BorrowBuf<'_> = buf.into(); let mut len = 0; loop { - match reader.read_buf(&mut buf) { + match reader.read_buf(buf.unfilled()) { Ok(()) => {} Err(e) if e.kind() == ErrorKind::Interrupted => continue, Err(e) => return Err(e), diff --git a/library/std/src/io/cursor.rs b/library/std/src/io/cursor.rs index f3fbfc4478951..460b1504ffbfc 100644 --- a/library/std/src/io/cursor.rs +++ b/library/std/src/io/cursor.rs @@ -5,7 +5,7 @@ use crate::io::prelude::*; use crate::alloc::Allocator; use crate::cmp; -use crate::io::{self, ErrorKind, IoSlice, IoSliceMut, ReadBuf, SeekFrom}; +use crate::io::{self, BorrowCursor, ErrorKind, IoSlice, IoSliceMut, SeekFrom}; /// A `Cursor` wraps an in-memory buffer and provides it with a /// [`Seek`] implementation. @@ -323,12 +323,12 @@ where Ok(n) } - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - let prev_filled = buf.filled_len(); + fn read_buf(&mut self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + let prev_written = cursor.written(); - Read::read_buf(&mut self.fill_buf()?, buf)?; + Read::read_buf(&mut self.fill_buf()?, cursor.clone())?; - self.pos += (buf.filled_len() - prev_filled) as u64; + self.pos += (cursor.written() - prev_written) as u64; Ok(()) } diff --git a/library/std/src/io/impls.rs b/library/std/src/io/impls.rs index 95072547302eb..eee5ab6ec1050 100644 --- a/library/std/src/io/impls.rs +++ b/library/std/src/io/impls.rs @@ -6,7 +6,7 @@ use crate::cmp; use crate::collections::VecDeque; use crate::fmt; use crate::io::{ - self, BufRead, ErrorKind, IoSlice, IoSliceMut, Read, ReadBuf, Seek, SeekFrom, Write, + self, BorrowCursor, BufRead, ErrorKind, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write, }; use crate::mem; @@ -21,8 +21,8 @@ impl Read for &mut R { } #[inline] - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - (**self).read_buf(buf) + fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + (**self).read_buf(cursor) } #[inline] @@ -125,8 +125,8 @@ impl Read for Box { } #[inline] - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - (**self).read_buf(buf) + fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + (**self).read_buf(cursor) } #[inline] @@ -249,11 +249,11 @@ impl Read for &[u8] { } #[inline] - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - let amt = cmp::min(buf.remaining(), self.len()); + fn read_buf(&mut self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + let amt = cmp::min(cursor.capacity(), self.len()); let (a, b) = self.split_at(amt); - buf.append(a); + cursor.append(a); *self = b; Ok(()) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 96addbd1a0558..b3218b2831d3f 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -278,7 +278,7 @@ pub use self::{ }; #[unstable(feature = "read_buf", issue = "78485")] -pub use self::readbuf::ReadBuf; +pub use self::readbuf::{BorrowBuf, BorrowCursor}; pub(crate) use error::const_io_error; mod buffered; @@ -362,29 +362,30 @@ pub(crate) fn default_read_to_end(r: &mut R, buf: &mut Vec buf.reserve(32); // buf is full, need more space } - let mut read_buf = ReadBuf::uninit(buf.spare_capacity_mut()); + let mut read_buf: BorrowBuf<'_> = buf.spare_capacity_mut().into(); // SAFETY: These bytes were initialized but not filled in the previous loop unsafe { - read_buf.assume_init(initialized); + read_buf.set_init(initialized); } - match r.read_buf(&mut read_buf) { + let mut cursor = read_buf.unfilled(); + match r.read_buf(cursor.clone()) { Ok(()) => {} Err(e) if e.kind() == ErrorKind::Interrupted => continue, Err(e) => return Err(e), } - if read_buf.filled_len() == 0 { + if cursor.written() == 0 { return Ok(buf.len() - start_len); } // store how much was initialized but not filled - initialized = read_buf.initialized_len() - read_buf.filled_len(); - let new_len = read_buf.filled_len() + buf.len(); + initialized = cursor.init_ref().len(); - // SAFETY: ReadBuf's invariants mean this much memory is init + // SAFETY: BorrowBuf's invariants mean this much memory is initialized. unsafe { + let new_len = read_buf.filled().len() + buf.len(); buf.set_len(new_len); } @@ -461,12 +462,15 @@ pub(crate) fn default_read_exact(this: &mut R, mut buf: &mut [ } } -pub(crate) fn default_read_buf(read: F, buf: &mut ReadBuf<'_>) -> Result<()> +pub(crate) fn default_read_buf(read: F, mut cursor: BorrowCursor<'_, '_>) -> Result<()> where F: FnOnce(&mut [u8]) -> Result, { - let n = read(buf.initialize_unfilled())?; - buf.add_filled(n); + let n = read(cursor.ensure_init().init_mut())?; + unsafe { + // SAFETY: we initialised using `ensure_init` so there is no uninit data to advance to. + cursor.advance(n); + } Ok(()) } @@ -801,32 +805,33 @@ pub trait Read { default_read_exact(self, buf) } + // TODO naming, if should the method be read_cursor? Or should we change the names of the data structures? /// Pull some bytes from this source into the specified buffer. /// - /// This is equivalent to the [`read`](Read::read) method, except that it is passed a [`ReadBuf`] rather than `[u8]` to allow use + /// This is equivalent to the [`read`](Read::read) method, except that it is passed a [`BorrowCursor`] rather than `[u8]` to allow use /// with uninitialized buffers. The new data will be appended to any existing contents of `buf`. /// /// The default implementation delegates to `read`. #[unstable(feature = "read_buf", issue = "78485")] - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> Result<()> { + fn read_buf(&mut self, buf: BorrowCursor<'_, '_>) -> Result<()> { default_read_buf(|b| self.read(b), buf) } - /// Read the exact number of bytes required to fill `buf`. + /// Read the exact number of bytes required to fill `cursor`. /// - /// This is equivalent to the [`read_exact`](Read::read_exact) method, except that it is passed a [`ReadBuf`] rather than `[u8]` to + /// This is equivalent to the [`read_exact`](Read::read_exact) method, except that it is passed a [`BorrowCursor`] rather than `[u8]` to /// allow use with uninitialized buffers. #[unstable(feature = "read_buf", issue = "78485")] - fn read_buf_exact(&mut self, buf: &mut ReadBuf<'_>) -> Result<()> { - while buf.remaining() > 0 { - let prev_filled = buf.filled().len(); - match self.read_buf(buf) { + fn read_buf_exact(&mut self, mut cursor: BorrowCursor<'_, '_>) -> Result<()> { + while cursor.capacity() > 0 { + let prev_written = cursor.written(); + match self.read_buf(cursor.clone()) { Ok(()) => {} Err(e) if e.kind() == ErrorKind::Interrupted => continue, Err(e) => return Err(e), } - if buf.filled().len() == prev_filled { + if cursor.written() == prev_written { return Err(Error::new(ErrorKind::UnexpectedEof, "failed to fill buffer")); } } @@ -2582,50 +2587,48 @@ impl Read for Take { Ok(n) } - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> Result<()> { + fn read_buf(&mut self, mut buf: BorrowCursor<'_, '_>) -> Result<()> { // Don't call into inner reader at all at EOF because it may still block if self.limit == 0 { return Ok(()); } - let prev_filled = buf.filled_len(); - - if self.limit <= buf.remaining() as u64 { + if self.limit <= buf.capacity() as u64 { // if we just use an as cast to convert, limit may wrap around on a 32 bit target let limit = cmp::min(self.limit, usize::MAX as u64) as usize; - let extra_init = cmp::min(limit as usize, buf.initialized_len() - buf.filled_len()); + let extra_init = cmp::min(limit as usize, buf.init_ref().len()); // SAFETY: no uninit data is written to ibuf - let ibuf = unsafe { &mut buf.unfilled_mut()[..limit] }; + let ibuf = unsafe { &mut buf.as_mut()[..limit] }; - let mut sliced_buf = ReadBuf::uninit(ibuf); + let mut sliced_buf: BorrowBuf<'_> = ibuf.into(); // SAFETY: extra_init bytes of ibuf are known to be initialized unsafe { - sliced_buf.assume_init(extra_init); + sliced_buf.set_init(extra_init); } - self.inner.read_buf(&mut sliced_buf)?; + let mut cursor = sliced_buf.unfilled(); + self.inner.read_buf(cursor.clone())?; - let new_init = sliced_buf.initialized_len(); - let filled = sliced_buf.filled_len(); + let new_init = cursor.init_ref().len(); + let filled = sliced_buf.len(); - // sliced_buf / ibuf must drop here + // cursor / sliced_buf / ibuf must drop here - // SAFETY: new_init bytes of buf's unfilled buffer have been initialized unsafe { - buf.assume_init(new_init); + // SAFETY: filled bytes have been filled and therefore initialized + buf.advance(filled); + // SAFETY: new_init bytes of buf's unfilled buffer have been initialized + buf.set_init(new_init); } - buf.add_filled(filled); - self.limit -= filled as u64; } else { - self.inner.read_buf(buf)?; - - //inner may unfill - self.limit -= buf.filled_len().saturating_sub(prev_filled) as u64; + let written = buf.written(); + self.inner.read_buf(buf.clone())?; + self.limit -= (buf.written() - written) as u64; } Ok(()) diff --git a/library/std/src/io/readbuf.rs b/library/std/src/io/readbuf.rs index 78d1113f8375a..4578433b22a11 100644 --- a/library/std/src/io/readbuf.rs +++ b/library/std/src/io/readbuf.rs @@ -7,6 +7,7 @@ use crate::cmp; use crate::fmt::{self, Debug, Formatter}; use crate::mem::MaybeUninit; +// TODO docs /// A wrapper around a byte buffer that is incrementally filled and initialized. /// /// This type is a sort of "double cursor". It tracks three regions in the buffer: a region at the beginning of the @@ -20,50 +21,66 @@ use crate::mem::MaybeUninit; /// [ filled | unfilled ] /// [ initialized | uninitialized ] /// ``` -pub struct ReadBuf<'a> { +pub struct BorrowBuf<'a> { buf: &'a mut [MaybeUninit], filled: usize, initialized: usize, } -impl Debug for ReadBuf<'_> { +impl Debug for BorrowBuf<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { f.debug_struct("ReadBuf") - .field("init", &self.initialized()) + .field("init", &self.initialized) .field("filled", &self.filled) .field("capacity", &self.capacity()) .finish() } } -impl<'a> ReadBuf<'a> { - /// Creates a new `ReadBuf` from a fully initialized buffer. +/// Creates a new `BorrowBuf` from a fully initialized slice. +impl<'a> From<&'a mut [u8]> for BorrowBuf<'a> { #[inline] - pub fn new(buf: &'a mut [u8]) -> ReadBuf<'a> { - let len = buf.len(); + fn from(slice: &'a mut [u8]) -> BorrowBuf<'a> { + let len = slice.len(); - ReadBuf { - //SAFETY: initialized data never becoming uninitialized is an invariant of ReadBuf - buf: unsafe { (buf as *mut [u8]).as_uninit_slice_mut().unwrap() }, + BorrowBuf { + //SAFETY: initialized data never becoming uninitialized is an invariant of BorrowBuf + buf: unsafe { (slice as *mut [u8]).as_uninit_slice_mut().unwrap() }, filled: 0, initialized: len, } } +} - /// Creates a new `ReadBuf` from a fully uninitialized buffer. - /// - /// Use `assume_init` if part of the buffer is known to be already initialized. +/// Creates a new `BorrowBuf` from a fully uninitialized buffer. +/// +/// Use `set_init` if part of the buffer is known to be already initialized. +impl<'a> From<&'a mut [MaybeUninit]> for BorrowBuf<'a> { #[inline] - pub fn uninit(buf: &'a mut [MaybeUninit]) -> ReadBuf<'a> { - ReadBuf { buf, filled: 0, initialized: 0 } + fn from(buf: &'a mut [MaybeUninit]) -> BorrowBuf<'a> { + BorrowBuf { buf, filled: 0, initialized: 0 } } +} +impl<'a> BorrowBuf<'a> { /// Returns the total capacity of the buffer. #[inline] pub fn capacity(&self) -> usize { self.buf.len() } + /// Returns the length of the filled part of the buffer. + #[inline] + pub fn len(&self) -> usize { + self.filled + } + + /// Returns the length of the initialized part of the buffer. + #[inline] + pub fn init_len(&self) -> usize { + self.initialized + } + /// Returns a shared reference to the filled portion of the buffer. #[inline] pub fn filled(&self) -> &[u8] { @@ -71,179 +88,157 @@ impl<'a> ReadBuf<'a> { unsafe { MaybeUninit::slice_assume_init_ref(&self.buf[0..self.filled]) } } - /// Returns a mutable reference to the filled portion of the buffer. + /// Returns a cursor over the unfilled part of the buffer. #[inline] - pub fn filled_mut(&mut self) -> &mut [u8] { - //SAFETY: We only slice the filled part of the buffer, which is always valid - unsafe { MaybeUninit::slice_assume_init_mut(&mut self.buf[0..self.filled]) } + pub fn unfilled<'b>(&'b mut self) -> BorrowCursor<'a, 'b> { + BorrowCursor { start: self.filled, buf: self } } - /// Returns a shared reference to the initialized portion of the buffer. + /// Clears the buffer, resetting the filled region to empty. /// - /// This includes the filled portion. + /// The number of initialized bytes is not changed, and the contents of the buffer are not modified. #[inline] - pub fn initialized(&self) -> &[u8] { - //SAFETY: We only slice the initialized part of the buffer, which is always valid - unsafe { MaybeUninit::slice_assume_init_ref(&self.buf[0..self.initialized]) } + pub fn clear(&mut self) -> &mut Self { + self.filled = 0; + self } - /// Returns a mutable reference to the initialized portion of the buffer. + /// Asserts that the first `n` bytes of the buffer are initialized. /// - /// This includes the filled portion. - #[inline] - pub fn initialized_mut(&mut self) -> &mut [u8] { - //SAFETY: We only slice the initialized part of the buffer, which is always valid - unsafe { MaybeUninit::slice_assume_init_mut(&mut self.buf[0..self.initialized]) } - } - - /// Returns a mutable reference to the unfilled part of the buffer without ensuring that it has been fully - /// initialized. + /// `BorrowBuf` assumes that bytes are never de-initialized, so this method does nothing when called with fewer + /// bytes than are already known to be initialized. /// /// # Safety /// - /// The caller must not de-initialize portions of the buffer that have already been initialized. + /// The caller must ensure that the first `n` unfilled bytes of the buffer have already been initialized. #[inline] - pub unsafe fn unfilled_mut(&mut self) -> &mut [MaybeUninit] { - &mut self.buf[self.filled..] + pub unsafe fn set_init(&mut self, n: usize) -> &mut Self { + self.initialized = cmp::max(self.initialized, n); + self } +} - /// Returns a mutable reference to the uninitialized part of the buffer. - /// - /// It is safe to uninitialize any of these bytes. +/// A cursor view of a [`BorrowBuf`](BorrowBuf). +/// +/// Provides mutable access to the unfilled portion (both initialised and uninitialised data) from +/// the buffer. +#[derive(Debug)] +pub struct BorrowCursor<'a, 'b> { + buf: &'b mut BorrowBuf<'a>, + start: usize, +} + +impl<'a, 'b> BorrowCursor<'a, 'b> { + /// Clone this cursor. #[inline] - pub fn uninitialized_mut(&mut self) -> &mut [MaybeUninit] { - &mut self.buf[self.initialized..] + pub fn clone<'c>(&'c mut self) -> BorrowCursor<'a, 'c> { + BorrowCursor { buf: self.buf, start: self.start } } - /// Returns a mutable reference to the unfilled part of the buffer, ensuring it is fully initialized. - /// - /// Since `ReadBuf` tracks the region of the buffer that has been initialized, this is effectively "free" after - /// the first use. + /// Returns the available space in the cursor. #[inline] - pub fn initialize_unfilled(&mut self) -> &mut [u8] { - // should optimize out the assertion - self.initialize_unfilled_to(self.remaining()) + pub fn capacity(&self) -> usize { + self.buf.capacity() - self.buf.filled } - /// Returns a mutable reference to the first `n` bytes of the unfilled part of the buffer, ensuring it is - /// fully initialized. - /// - /// # Panics - /// - /// Panics if `self.remaining()` is less than `n`. + /// Returns the number of bytes written to this cursor. + // TODO check for reuse uses #[inline] - pub fn initialize_unfilled_to(&mut self, n: usize) -> &mut [u8] { - assert!(self.remaining() >= n); - - let extra_init = self.initialized - self.filled; - // If we don't have enough initialized, do zeroing - if n > extra_init { - let uninit = n - extra_init; - let unfilled = &mut self.uninitialized_mut()[0..uninit]; - - for byte in unfilled.iter_mut() { - byte.write(0); - } + pub fn written(&self) -> usize { + self.buf.filled - self.start + } - // SAFETY: we just initialized uninit bytes, and the previous bytes were already init - unsafe { - self.assume_init(n); - } + /// Returns a shared reference to the initialized portion of the buffer. + #[inline] + pub fn init_ref(&self) -> &[u8] { + //SAFETY: We only slice the initialized part of the buffer, which is always valid + unsafe { + MaybeUninit::slice_assume_init_ref(&self.buf.buf[self.buf.filled..self.buf.initialized]) } - - let filled = self.filled; - - &mut self.initialized_mut()[filled..filled + n] } - /// Returns the number of bytes at the end of the slice that have not yet been filled. + /// Returns a mutable reference to the initialized portion of the buffer. #[inline] - pub fn remaining(&self) -> usize { - self.capacity() - self.filled + pub fn init_mut(&mut self) -> &mut [u8] { + //SAFETY: We only slice the initialized part of the buffer, which is always valid + unsafe { + MaybeUninit::slice_assume_init_mut( + &mut self.buf.buf[self.buf.filled..self.buf.initialized], + ) + } } - /// Clears the buffer, resetting the filled region to empty. + /// Returns a mutable reference to the uninitialized part of the buffer. /// - /// The number of initialized bytes is not changed, and the contents of the buffer are not modified. + /// It is safe to uninitialize any of these bytes. #[inline] - pub fn clear(&mut self) -> &mut Self { - self.set_filled(0) // The assertion in `set_filled` is optimized out + pub fn uninit_mut(&mut self) -> &mut [MaybeUninit] { + &mut self.buf.buf[self.buf.initialized..] + } + + /// A view of the cursor as a mutable slice of `MaybeUninit`. + #[inline] + pub unsafe fn as_mut(&mut self) -> &mut [MaybeUninit] { + &mut self.buf.buf[self.buf.filled..] } /// Increases the size of the filled region of the buffer. /// - /// The number of initialized bytes is not changed. - /// - /// # Panics + /// # Safety /// - /// Panics if the filled region of the buffer would become larger than the initialized region. + /// The caller must ensure that the first `n` elements of the cursor have been properly + /// initialised. #[inline] - pub fn add_filled(&mut self, n: usize) -> &mut Self { - self.set_filled(self.filled + n) + pub unsafe fn advance(&mut self, n: usize) -> &mut Self { + self.buf.filled += n; + self.buf.initialized = cmp::max(self.buf.initialized, self.buf.filled); + self } - /// Sets the size of the filled region of the buffer. - /// - /// The number of initialized bytes is not changed. - /// - /// Note that this can be used to *shrink* the filled region of the buffer in addition to growing it (for - /// example, by a `Read` implementation that compresses data in-place). - /// - /// # Panics - /// - /// Panics if the filled region of the buffer would become larger than the initialized region. + /// Initialised all bytes in the cursor. #[inline] - pub fn set_filled(&mut self, n: usize) -> &mut Self { - assert!(n <= self.initialized); + pub fn ensure_init(&mut self) -> &mut Self { + for byte in self.uninit_mut() { + byte.write(0); + } + self.buf.initialized = self.buf.capacity(); - self.filled = n; self } - /// Asserts that the first `n` unfilled bytes of the buffer are initialized. + /// Asserts that the first `n` unfilled bytes of the cursor are initialized. /// - /// `ReadBuf` assumes that bytes are never de-initialized, so this method does nothing when called with fewer + /// `BorrowBuf` assumes that bytes are never de-initialized, so this method does nothing when called with fewer /// bytes than are already known to be initialized. /// /// # Safety /// - /// The caller must ensure that the first `n` unfilled bytes of the buffer have already been initialized. + /// The caller must ensure that the first `n` bytes of the buffer have already been initialized. #[inline] - pub unsafe fn assume_init(&mut self, n: usize) -> &mut Self { - self.initialized = cmp::max(self.initialized, self.filled + n); + pub unsafe fn set_init(&mut self, n: usize) -> &mut Self { + self.buf.initialized = cmp::max(self.buf.initialized, self.buf.filled + n); self } - /// Appends data to the buffer, advancing the written position and possibly also the initialized position. + /// Appends data to the cursor, advancing the position within its buffer. /// /// # Panics /// - /// Panics if `self.remaining()` is less than `buf.len()`. + /// Panics if `self.capacity()` is less than `buf.len()`. #[inline] pub fn append(&mut self, buf: &[u8]) { - assert!(self.remaining() >= buf.len()); + assert!(self.capacity() >= buf.len()); // SAFETY: we do not de-initialize any of the elements of the slice unsafe { - MaybeUninit::write_slice(&mut self.unfilled_mut()[..buf.len()], buf); + MaybeUninit::write_slice(&mut self.as_mut()[..buf.len()], buf); } // SAFETY: We just added the entire contents of buf to the filled section. unsafe { - self.assume_init(buf.len()); - } - self.add_filled(buf.len()); - } - - /// Returns the amount of bytes that have been filled. - #[inline] - pub fn filled_len(&self) -> usize { - self.filled - } - /// Returns the amount of bytes that have been initialized. - #[inline] - pub fn initialized_len(&self) -> usize { - self.initialized + self.set_init(buf.len()); + } + self.buf.filled += buf.len(); } } diff --git a/library/std/src/io/readbuf/tests.rs b/library/std/src/io/readbuf/tests.rs index 3b7a5a56d2252..584e5de982e97 100644 --- a/library/std/src/io/readbuf/tests.rs +++ b/library/std/src/io/readbuf/tests.rs @@ -1,181 +1,117 @@ -use super::ReadBuf; +use super::BorrowBuf; use crate::mem::MaybeUninit; -/// Test that ReadBuf has the correct numbers when created with new +/// Test that BorrowBuf has the correct numbers when created with new #[test] fn new() { - let mut buf = [0; 16]; - let rbuf = ReadBuf::new(&mut buf); + let buf: &mut [_] = &mut [0; 16]; + let mut rbuf: BorrowBuf<'_> = buf.into(); - assert_eq!(rbuf.filled_len(), 0); - assert_eq!(rbuf.initialized_len(), 16); + assert_eq!(rbuf.filled().len(), 0); + assert_eq!(rbuf.init_len(), 16); assert_eq!(rbuf.capacity(), 16); - assert_eq!(rbuf.remaining(), 16); + assert_eq!(rbuf.unfilled().capacity(), 16); } -/// Test that ReadBuf has the correct numbers when created with uninit +/// Test that BorrowBuf has the correct numbers when created with uninit #[test] fn uninit() { - let mut buf = [MaybeUninit::uninit(); 16]; - let rbuf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 16]; + let mut rbuf: BorrowBuf<'_> = buf.into(); - assert_eq!(rbuf.filled_len(), 0); - assert_eq!(rbuf.initialized_len(), 0); + assert_eq!(rbuf.filled().len(), 0); + assert_eq!(rbuf.init_len(), 0); assert_eq!(rbuf.capacity(), 16); - assert_eq!(rbuf.remaining(), 16); + assert_eq!(rbuf.unfilled().capacity(), 16); } #[test] fn initialize_unfilled() { - let mut buf = [MaybeUninit::uninit(); 16]; - let mut rbuf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 16]; + let mut rbuf: BorrowBuf<'_> = buf.into(); - rbuf.initialize_unfilled(); + rbuf.unfilled().ensure_init(); - assert_eq!(rbuf.initialized_len(), 16); -} - -#[test] -fn initialize_unfilled_to() { - let mut buf = [MaybeUninit::uninit(); 16]; - let mut rbuf = ReadBuf::uninit(&mut buf); - - rbuf.initialize_unfilled_to(8); - - assert_eq!(rbuf.initialized_len(), 8); - - rbuf.initialize_unfilled_to(4); - - assert_eq!(rbuf.initialized_len(), 8); - - rbuf.set_filled(8); - - rbuf.initialize_unfilled_to(6); - - assert_eq!(rbuf.initialized_len(), 14); - - rbuf.initialize_unfilled_to(8); - - assert_eq!(rbuf.initialized_len(), 16); + assert_eq!(rbuf.init_len(), 16); } #[test] fn add_filled() { - let mut buf = [0; 16]; - let mut rbuf = ReadBuf::new(&mut buf); - - rbuf.add_filled(1); - - assert_eq!(rbuf.filled_len(), 1); - assert_eq!(rbuf.remaining(), 15); -} + let buf: &mut [_] = &mut [0; 16]; + let mut rbuf: BorrowBuf<'_> = buf.into(); -#[test] -#[should_panic] -fn add_filled_panic() { - let mut buf = [MaybeUninit::uninit(); 16]; - let mut rbuf = ReadBuf::uninit(&mut buf); - - rbuf.add_filled(1); -} - -#[test] -fn set_filled() { - let mut buf = [0; 16]; - let mut rbuf = ReadBuf::new(&mut buf); - - rbuf.set_filled(16); - - assert_eq!(rbuf.filled_len(), 16); - assert_eq!(rbuf.remaining(), 0); - - rbuf.set_filled(6); - - assert_eq!(rbuf.filled_len(), 6); - assert_eq!(rbuf.remaining(), 10); -} - -#[test] -#[should_panic] -fn set_filled_panic() { - let mut buf = [MaybeUninit::uninit(); 16]; - let mut rbuf = ReadBuf::uninit(&mut buf); + unsafe { + rbuf.unfilled().advance(1); + } - rbuf.set_filled(16); + assert_eq!(rbuf.filled().len(), 1); + assert_eq!(rbuf.unfilled().capacity(), 15); } #[test] fn clear() { - let mut buf = [255; 16]; - let mut rbuf = ReadBuf::new(&mut buf); + let buf: &mut [_] = &mut [255; 16]; + let mut rbuf: BorrowBuf<'_> = buf.into(); - rbuf.set_filled(16); + unsafe { + rbuf.unfilled().advance(16); + } - assert_eq!(rbuf.filled_len(), 16); - assert_eq!(rbuf.remaining(), 0); + assert_eq!(rbuf.filled().len(), 16); + assert_eq!(rbuf.unfilled().capacity(), 0); rbuf.clear(); - assert_eq!(rbuf.filled_len(), 0); - assert_eq!(rbuf.remaining(), 16); + assert_eq!(rbuf.filled().len(), 0); + assert_eq!(rbuf.unfilled().capacity(), 16); - assert_eq!(rbuf.initialized(), [255; 16]); + assert_eq!(rbuf.unfilled().init_ref(), [255; 16]); } #[test] -fn assume_init() { - let mut buf = [MaybeUninit::uninit(); 16]; - let mut rbuf = ReadBuf::uninit(&mut buf); +fn set_init() { + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 16]; + let mut rbuf: BorrowBuf<'_> = buf.into(); unsafe { - rbuf.assume_init(8); + rbuf.set_init(8); } - assert_eq!(rbuf.initialized_len(), 8); + assert_eq!(rbuf.init_len(), 8); - rbuf.add_filled(4); + unsafe { + rbuf.unfilled().advance(4); + } unsafe { - rbuf.assume_init(2); + rbuf.set_init(2); } - assert_eq!(rbuf.initialized_len(), 8); + assert_eq!(rbuf.init_len(), 8); unsafe { - rbuf.assume_init(8); + rbuf.set_init(8); } - assert_eq!(rbuf.initialized_len(), 12); + assert_eq!(rbuf.init_len(), 8); } #[test] fn append() { - let mut buf = [MaybeUninit::new(255); 16]; - let mut rbuf = ReadBuf::uninit(&mut buf); + let buf: &mut [_] = &mut [MaybeUninit::new(255); 16]; + let mut rbuf: BorrowBuf<'_> = buf.into(); - rbuf.append(&[0; 8]); + rbuf.unfilled().append(&[0; 8]); - assert_eq!(rbuf.initialized_len(), 8); - assert_eq!(rbuf.filled_len(), 8); + assert_eq!(rbuf.init_len(), 8); + assert_eq!(rbuf.filled().len(), 8); assert_eq!(rbuf.filled(), [0; 8]); rbuf.clear(); - rbuf.append(&[1; 16]); + rbuf.unfilled().append(&[1; 16]); - assert_eq!(rbuf.initialized_len(), 16); - assert_eq!(rbuf.filled_len(), 16); + assert_eq!(rbuf.init_len(), 16); + assert_eq!(rbuf.filled().len(), 16); assert_eq!(rbuf.filled(), [1; 16]); } - -#[test] -fn filled_mut() { - let mut buf = [0; 16]; - let mut rbuf = ReadBuf::new(&mut buf); - - rbuf.add_filled(8); - - let filled = rbuf.filled().to_vec(); - - assert_eq!(&*filled, &*rbuf.filled_mut()); -} diff --git a/library/std/src/io/tests.rs b/library/std/src/io/tests.rs index f357f33ec52c5..a1322a185651e 100644 --- a/library/std/src/io/tests.rs +++ b/library/std/src/io/tests.rs @@ -1,4 +1,4 @@ -use super::{repeat, Cursor, ReadBuf, SeekFrom}; +use super::{repeat, BorrowBuf, Cursor, SeekFrom}; use crate::cmp::{self, min}; use crate::io::{self, IoSlice, IoSliceMut}; use crate::io::{BufRead, BufReader, Read, Seek, Write}; @@ -159,24 +159,24 @@ fn read_exact_slice() { #[test] fn read_buf_exact() { - let mut buf = [0; 4]; - let mut buf = ReadBuf::new(&mut buf); + let buf: &mut [_] = &mut [0; 4]; + let mut buf: BorrowBuf<'_> = buf.into(); let mut c = Cursor::new(&b""[..]); - assert_eq!(c.read_buf_exact(&mut buf).unwrap_err().kind(), io::ErrorKind::UnexpectedEof); + assert_eq!(c.read_buf_exact(buf.unfilled()).unwrap_err().kind(), io::ErrorKind::UnexpectedEof); let mut c = Cursor::new(&b"123456789"[..]); - c.read_buf_exact(&mut buf).unwrap(); + c.read_buf_exact(buf.unfilled()).unwrap(); assert_eq!(buf.filled(), b"1234"); buf.clear(); - c.read_buf_exact(&mut buf).unwrap(); + c.read_buf_exact(buf.unfilled()).unwrap(); assert_eq!(buf.filled(), b"5678"); buf.clear(); - assert_eq!(c.read_buf_exact(&mut buf).unwrap_err().kind(), io::ErrorKind::UnexpectedEof); + assert_eq!(c.read_buf_exact(buf.unfilled()).unwrap_err().kind(), io::ErrorKind::UnexpectedEof); } #[test] @@ -614,10 +614,10 @@ fn bench_take_read(b: &mut test::Bencher) { #[bench] fn bench_take_read_buf(b: &mut test::Bencher) { b.iter(|| { - let mut buf = [MaybeUninit::uninit(); 64]; + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 64]; - let mut rbuf = ReadBuf::uninit(&mut buf); + let mut buf: BorrowBuf<'_> = buf.into(); - [255; 128].take(64).read_buf(&mut rbuf).unwrap(); + [255; 128].take(64).read_buf(buf.unfilled()).unwrap(); }); } diff --git a/library/std/src/io/util.rs b/library/std/src/io/util.rs index c1300cd67c086..5149926fd519d 100644 --- a/library/std/src/io/util.rs +++ b/library/std/src/io/util.rs @@ -5,7 +5,7 @@ mod tests; use crate::fmt; use crate::io::{ - self, BufRead, IoSlice, IoSliceMut, Read, ReadBuf, Seek, SeekFrom, SizeHint, Write, + self, BorrowCursor, BufRead, IoSlice, IoSliceMut, Read, Seek, SeekFrom, SizeHint, Write, }; /// A reader which is always at EOF. @@ -47,7 +47,7 @@ impl Read for Empty { } #[inline] - fn read_buf(&mut self, _buf: &mut ReadBuf<'_>) -> io::Result<()> { + fn read_buf(&mut self, _cursor: BorrowCursor<'_, '_>) -> io::Result<()> { Ok(()) } } @@ -130,21 +130,19 @@ impl Read for Repeat { Ok(buf.len()) } - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { + fn read_buf(&mut self, mut buf: BorrowCursor<'_, '_>) -> io::Result<()> { // SAFETY: No uninit bytes are being written - for slot in unsafe { buf.unfilled_mut() } { + for slot in unsafe { buf.as_mut() } { slot.write(self.byte); } - let remaining = buf.remaining(); + let remaining = buf.capacity(); // SAFETY: the entire unfilled portion of buf has been initialized unsafe { - buf.assume_init(remaining); + buf.advance(remaining); } - buf.add_filled(remaining); - Ok(()) } diff --git a/library/std/src/io/util/tests.rs b/library/std/src/io/util/tests.rs index 08972a59a833f..025173c3f446c 100644 --- a/library/std/src/io/util/tests.rs +++ b/library/std/src/io/util/tests.rs @@ -1,7 +1,7 @@ use crate::cmp::{max, min}; use crate::io::prelude::*; use crate::io::{ - copy, empty, repeat, sink, BufWriter, Empty, ReadBuf, Repeat, Result, SeekFrom, Sink, + copy, empty, repeat, sink, BorrowBuf, BufWriter, Empty, Repeat, Result, SeekFrom, Sink, DEFAULT_BUF_SIZE, }; @@ -79,29 +79,29 @@ fn empty_reads() { assert_eq!(e.read(&mut [0; 1024]).unwrap(), 0); assert_eq!(e.by_ref().read(&mut [0; 1024]).unwrap(), 0); - let mut buf = []; - let mut buf = ReadBuf::uninit(&mut buf); - e.read_buf(&mut buf).unwrap(); - assert_eq!(buf.filled_len(), 0); - assert_eq!(buf.initialized_len(), 0); - - let mut buf = [MaybeUninit::uninit()]; - let mut buf = ReadBuf::uninit(&mut buf); - e.read_buf(&mut buf).unwrap(); - assert_eq!(buf.filled_len(), 0); - assert_eq!(buf.initialized_len(), 0); - - let mut buf = [MaybeUninit::uninit(); 1024]; - let mut buf = ReadBuf::uninit(&mut buf); - e.read_buf(&mut buf).unwrap(); - assert_eq!(buf.filled_len(), 0); - assert_eq!(buf.initialized_len(), 0); - - let mut buf = [MaybeUninit::uninit(); 1024]; - let mut buf = ReadBuf::uninit(&mut buf); - e.by_ref().read_buf(&mut buf).unwrap(); - assert_eq!(buf.filled_len(), 0); - assert_eq!(buf.initialized_len(), 0); + let buf: &mut [MaybeUninit<_>] = &mut []; + let mut buf: BorrowBuf<'_> = buf.into(); + e.read_buf(buf.unfilled()).unwrap(); + assert_eq!(buf.len(), 0); + assert_eq!(buf.init_len(), 0); + + let buf: &mut [_] = &mut [MaybeUninit::uninit()]; + let mut buf: BorrowBuf<'_> = buf.into(); + e.read_buf(buf.unfilled()).unwrap(); + assert_eq!(buf.len(), 0); + assert_eq!(buf.init_len(), 0); + + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 1024]; + let mut buf: BorrowBuf<'_> = buf.into(); + e.read_buf(buf.unfilled()).unwrap(); + assert_eq!(buf.len(), 0); + assert_eq!(buf.init_len(), 0); + + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 1024]; + let mut buf: BorrowBuf<'_> = buf.into(); + e.by_ref().read_buf(buf.unfilled()).unwrap(); + assert_eq!(buf.len(), 0); + assert_eq!(buf.init_len(), 0); } #[test] diff --git a/library/std/src/sys/unix/fd.rs b/library/std/src/sys/unix/fd.rs index 30812dabb4e0d..6adb734fb0a2f 100644 --- a/library/std/src/sys/unix/fd.rs +++ b/library/std/src/sys/unix/fd.rs @@ -4,7 +4,7 @@ mod tests; use crate::cmp; -use crate::io::{self, IoSlice, IoSliceMut, Read, ReadBuf}; +use crate::io::{self, BorrowCursor, IoSlice, IoSliceMut, Read}; use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use crate::sys::cvt; use crate::sys_common::{AsInner, FromInner, IntoInner}; @@ -131,20 +131,19 @@ impl FileDesc { } } - pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> { + pub fn read_buf(&self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { let ret = cvt(unsafe { libc::read( self.as_raw_fd(), - buf.unfilled_mut().as_mut_ptr() as *mut libc::c_void, - cmp::min(buf.remaining(), READ_LIMIT), + cursor.as_mut().as_mut_ptr() as *mut libc::c_void, + cmp::min(cursor.capacity(), READ_LIMIT), ) })?; // Safety: `ret` bytes were written to the initialized portion of the buffer unsafe { - buf.assume_init(ret as usize); + cursor.advance(ret as usize); } - buf.add_filled(ret as usize); Ok(()) } diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index b5cc8038ca44f..374f9f72d6d74 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -2,7 +2,7 @@ use crate::os::unix::prelude::*; use crate::ffi::{CStr, CString, OsStr, OsString}; use crate::fmt; -use crate::io::{self, Error, IoSlice, IoSliceMut, ReadBuf, SeekFrom}; +use crate::io::{self, BorrowCursor, Error, IoSlice, IoSliceMut, SeekFrom}; use crate::mem; use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd}; use crate::path::{Path, PathBuf}; @@ -1031,8 +1031,8 @@ impl File { self.0.read_at(buf, offset) } - pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - self.0.read_buf(buf) + pub fn read_buf(&self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + self.0.read_buf(cursor) } pub fn write(&self, buf: &[u8]) -> io::Result { From b56cf67ce14580111ffb07a08a293e217566e116 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Sun, 22 May 2022 20:57:58 -0500 Subject: [PATCH 03/19] Add some docs for BorrowBuf Signed-off-by: Nick Cameron --- library/std/src/io/readbuf.rs | 84 +++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 24 deletions(-) diff --git a/library/std/src/io/readbuf.rs b/library/std/src/io/readbuf.rs index 4578433b22a11..8783763fd425f 100644 --- a/library/std/src/io/readbuf.rs +++ b/library/std/src/io/readbuf.rs @@ -7,8 +7,7 @@ use crate::cmp; use crate::fmt::{self, Debug, Formatter}; use crate::mem::MaybeUninit; -// TODO docs -/// A wrapper around a byte buffer that is incrementally filled and initialized. +/// A borrowed byte buffer which is incrementally filled and initialized. /// /// This type is a sort of "double cursor". It tracks three regions in the buffer: a region at the beginning of the /// buffer that has been logically filled with data, a region that has been initialized at some point but not yet @@ -21,9 +20,20 @@ use crate::mem::MaybeUninit; /// [ filled | unfilled ] /// [ initialized | uninitialized ] /// ``` +/// +/// A `BorrowBuf` is created around some existing data (or capacity for data) via a unique reference +/// (`&mut`). The `BorrowBuf` can be configured (e.g., using `clear` or `set_init`), but otherwise +/// is read-only. To write into the buffer, use `unfilled` to create a `BorrowCursor`. The cursor +/// has write-only access to the unfilled portion of the buffer (you can think of it like a +/// write-only iterator). +/// +/// The lifetime `'a` is a bound on the lifetime of the underlying data. pub struct BorrowBuf<'a> { + /// The buffer's underlying data. buf: &'a mut [MaybeUninit], + /// The length of `self.buf` which is known to be filled. filled: usize, + /// The length of `self.buf` which is known to be initialized. initialized: usize, } @@ -37,7 +47,7 @@ impl Debug for BorrowBuf<'_> { } } -/// Creates a new `BorrowBuf` from a fully initialized slice. +/// Create a new `BorrowBuf` from a fully initialized slice. impl<'a> From<&'a mut [u8]> for BorrowBuf<'a> { #[inline] fn from(slice: &'a mut [u8]) -> BorrowBuf<'a> { @@ -52,7 +62,7 @@ impl<'a> From<&'a mut [u8]> for BorrowBuf<'a> { } } -/// Creates a new `BorrowBuf` from a fully uninitialized buffer. +/// Create a new `BorrowBuf` from an uninitialized buffer. /// /// Use `set_init` if part of the buffer is known to be already initialized. impl<'a> From<&'a mut [MaybeUninit]> for BorrowBuf<'a> { @@ -90,7 +100,7 @@ impl<'a> BorrowBuf<'a> { /// Returns a cursor over the unfilled part of the buffer. #[inline] - pub fn unfilled<'b>(&'b mut self) -> BorrowCursor<'a, 'b> { + pub fn unfilled<'this>(&'this mut self) -> BorrowCursor<'this, 'a> { BorrowCursor { start: self.filled, buf: self } } @@ -118,20 +128,36 @@ impl<'a> BorrowBuf<'a> { } } -/// A cursor view of a [`BorrowBuf`](BorrowBuf). +/// A writeable view of the unfilled portion of a [`BorrowBuf`](BorrowBuf). +/// +/// Provides access to the initialized and uninitialized parts of the underlying `BorrowBuf`. +/// Data can be written directly to the cursor by using [`append`](BorrowCursor::append) or +/// indirectly by getting a slice of part or all of the cursor and writing into the slice. In the +/// indirect case, the caller must call [`advance`](BorrowCursor::advance) after writing to inform +/// the cursor how many bytes have been written. /// -/// Provides mutable access to the unfilled portion (both initialised and uninitialised data) from -/// the buffer. +/// Once data is written to the cursor, it becomes part of the filled portion of the underlying +/// `BorrowBuf` and can no longer be accessed or re-written by the cursor. I.e., the cursor tracks +/// the unfilled part of the underlying `BorrowBuf`. +/// +/// The `'buf` lifetime is a bound on the lifetime of the underlying buffer. `'data` is a bound on +/// that buffer's underlying data. #[derive(Debug)] -pub struct BorrowCursor<'a, 'b> { - buf: &'b mut BorrowBuf<'a>, +pub struct BorrowCursor<'buf, 'data> { + /// The underlying buffer. + buf: &'buf mut BorrowBuf<'data>, + /// The length of the filled portion of the underlying buffer at the time of the cursor's + /// creation. start: usize, } -impl<'a, 'b> BorrowCursor<'a, 'b> { +impl<'buf, 'data> BorrowCursor<'buf, 'data> { /// Clone this cursor. + /// + /// Since a cursor maintains unique access to its underlying buffer, the cloned cursor is not + /// accessible while the clone is alive. #[inline] - pub fn clone<'c>(&'c mut self) -> BorrowCursor<'a, 'c> { + pub fn clone<'this>(&'this mut self) -> BorrowCursor<'this, 'data> { BorrowCursor { buf: self.buf, start: self.start } } @@ -141,14 +167,16 @@ impl<'a, 'b> BorrowCursor<'a, 'b> { self.buf.capacity() - self.buf.filled } - /// Returns the number of bytes written to this cursor. - // TODO check for reuse uses + /// Returns the number of bytes written to this cursor since it was created from a `BorrowBuf`. + /// + /// Note that if this cursor is a clone of another, then the count returned is the count written + /// via either cursor, not the count since the cursor was cloned. #[inline] pub fn written(&self) -> usize { self.buf.filled - self.start } - /// Returns a shared reference to the initialized portion of the buffer. + /// Returns a shared reference to the initialized portion of the cursor. #[inline] pub fn init_ref(&self) -> &[u8] { //SAFETY: We only slice the initialized part of the buffer, which is always valid @@ -157,7 +185,7 @@ impl<'a, 'b> BorrowCursor<'a, 'b> { } } - /// Returns a mutable reference to the initialized portion of the buffer. + /// Returns a mutable reference to the initialized portion of the cursor. #[inline] pub fn init_mut(&mut self) -> &mut [u8] { //SAFETY: We only slice the initialized part of the buffer, which is always valid @@ -168,7 +196,7 @@ impl<'a, 'b> BorrowCursor<'a, 'b> { } } - /// Returns a mutable reference to the uninitialized part of the buffer. + /// Returns a mutable reference to the uninitialized part of the cursor. /// /// It is safe to uninitialize any of these bytes. #[inline] @@ -176,17 +204,25 @@ impl<'a, 'b> BorrowCursor<'a, 'b> { &mut self.buf.buf[self.buf.initialized..] } - /// A view of the cursor as a mutable slice of `MaybeUninit`. + /// Returns a mutable reference to the whole cursor. + /// + /// # Safety + /// + /// The caller must not uninitialize any bytes in the initialized portion of the cursor. #[inline] pub unsafe fn as_mut(&mut self) -> &mut [MaybeUninit] { &mut self.buf.buf[self.buf.filled..] } - /// Increases the size of the filled region of the buffer. + /// Advance the cursor by asserting that `n` bytes have been filled. + /// + /// After advancing, the `n` bytes are no longer accessible via the cursor and can only be + /// accessed via the underlying buffer. I.e., the buffer's filled portion grows by `n` elements + /// and its unfilled portion (and the capacity of this cursor) shrinks by `n` elements. /// /// # Safety /// - /// The caller must ensure that the first `n` elements of the cursor have been properly + /// The caller must ensure that the first `n` bytes of the cursor have been properly /// initialised. #[inline] pub unsafe fn advance(&mut self, n: usize) -> &mut Self { @@ -195,7 +231,7 @@ impl<'a, 'b> BorrowCursor<'a, 'b> { self } - /// Initialised all bytes in the cursor. + /// Initializes all bytes in the cursor. #[inline] pub fn ensure_init(&mut self) -> &mut Self { for byte in self.uninit_mut() { @@ -208,8 +244,8 @@ impl<'a, 'b> BorrowCursor<'a, 'b> { /// Asserts that the first `n` unfilled bytes of the cursor are initialized. /// - /// `BorrowBuf` assumes that bytes are never de-initialized, so this method does nothing when called with fewer - /// bytes than are already known to be initialized. + /// `BorrowBuf` assumes that bytes are never de-initialized, so this method does nothing when + /// called with fewer bytes than are already known to be initialized. /// /// # Safety /// @@ -220,7 +256,7 @@ impl<'a, 'b> BorrowCursor<'a, 'b> { self } - /// Appends data to the cursor, advancing the position within its buffer. + /// Appends data to the cursor, advancing position within its buffer. /// /// # Panics /// From 1a2122fff015d1d7fb31fe3a55e49027d67d79af Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 7 Jun 2022 08:43:54 +0100 Subject: [PATCH 04/19] non-linux platforms Signed-off-by: Nick Cameron --- library/std/src/fs.rs | 6 +- library/std/src/io/buffered/bufreader.rs | 5 +- .../std/src/io/buffered/bufreader/buffer.rs | 12 +- library/std/src/io/buffered/tests.rs | 12 +- library/std/src/io/copy.rs | 8 +- library/std/src/io/cursor.rs | 4 +- library/std/src/io/impls.rs | 14 +-- library/std/src/io/mod.rs | 21 ++-- library/std/src/io/readbuf.rs | 107 ++++++++++-------- library/std/src/io/readbuf/tests.rs | 80 +++++++++++-- library/std/src/io/tests.rs | 6 +- library/std/src/io/util.rs | 6 +- library/std/src/io/util/tests.rs | 10 +- library/std/src/sys/hermit/fs.rs | 6 +- library/std/src/sys/solid/fs.rs | 12 +- library/std/src/sys/unix/fd.rs | 4 +- library/std/src/sys/unix/fs.rs | 4 +- library/std/src/sys/unsupported/fs.rs | 4 +- library/std/src/sys/wasi/fs.rs | 6 +- library/std/src/sys/windows/fs.rs | 6 +- library/std/src/sys/windows/handle.rs | 12 +- 21 files changed, 205 insertions(+), 140 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index d41f32b5b3f21..98aa40db32176 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -13,7 +13,7 @@ mod tests; use crate::ffi::OsString; use crate::fmt; -use crate::io::{self, BorrowCursor, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write}; +use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write}; use crate::path::{Path, PathBuf}; use crate::sys::fs as fs_imp; use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; @@ -703,7 +703,7 @@ impl Read for File { self.inner.read_vectored(bufs) } - fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { self.inner.read_buf(cursor) } @@ -755,7 +755,7 @@ impl Read for &File { self.inner.read(buf) } - fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { self.inner.read_buf(cursor) } diff --git a/library/std/src/io/buffered/bufreader.rs b/library/std/src/io/buffered/bufreader.rs index 1f19ac11bf101..dced922ea572e 100644 --- a/library/std/src/io/buffered/bufreader.rs +++ b/library/std/src/io/buffered/bufreader.rs @@ -2,8 +2,7 @@ mod buffer; use crate::fmt; use crate::io::{ - self, BorrowBuf, BorrowCursor, BufRead, IoSliceMut, Read, Seek, SeekFrom, SizeHint, - DEFAULT_BUF_SIZE, + self, BorrowedCursor, BufRead, IoSliceMut, Read, Seek, SeekFrom, SizeHint, DEFAULT_BUF_SIZE, }; use buffer::Buffer; @@ -267,7 +266,7 @@ impl Read for BufReader { Ok(nread) } - fn read_buf(&mut self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { // If we don't have any buffered data and we're doing a massive read // (larger than our internal buffer), bypass our internal buffer // entirely. diff --git a/library/std/src/io/buffered/bufreader/buffer.rs b/library/std/src/io/buffered/bufreader/buffer.rs index 8ae01f3b0ad8a..b122a6c0ccc57 100644 --- a/library/std/src/io/buffered/bufreader/buffer.rs +++ b/library/std/src/io/buffered/bufreader/buffer.rs @@ -9,7 +9,7 @@ /// that user code which wants to do reads from a `BufReader` via `buffer` + `consume` can do so /// without encountering any runtime bounds checks. use crate::cmp; -use crate::io::{self, Read, ReadBuf}; +use crate::io::{self, BorrowedBuf, Read}; use crate::mem::MaybeUninit; pub struct Buffer { @@ -93,11 +93,15 @@ impl Buffer { if self.pos >= self.filled { debug_assert!(self.pos == self.filled); - let mut readbuf = ReadBuf::uninit(&mut self.buf); + let mut buf: BorrowedBuf<'_> = (&mut *self.buf).into(); + // SAFETY: `self.filled` bytes will always have been initialized. + unsafe { + buf.set_init(self.filled); + } - reader.read_buf(&mut readbuf)?; + reader.read_buf(buf.unfilled())?; - self.filled = readbuf.filled_len(); + self.filled = buf.len(); self.pos = 0; } Ok(self.buffer()) diff --git a/library/std/src/io/buffered/tests.rs b/library/std/src/io/buffered/tests.rs index c93b69bf1f7c1..bd6d95242ad94 100644 --- a/library/std/src/io/buffered/tests.rs +++ b/library/std/src/io/buffered/tests.rs @@ -1,5 +1,7 @@ use crate::io::prelude::*; -use crate::io::{self, BorrowBuf, BufReader, BufWriter, ErrorKind, IoSlice, LineWriter, SeekFrom}; +use crate::io::{ + self, BorrowedBuf, BufReader, BufWriter, ErrorKind, IoSlice, LineWriter, SeekFrom, +}; use crate::mem::MaybeUninit; use crate::panic; use crate::sync::atomic::{AtomicUsize, Ordering}; @@ -62,7 +64,7 @@ fn test_buffered_reader_read_buf() { let mut reader = BufReader::with_capacity(2, inner); let buf: &mut [_] = &mut [MaybeUninit::uninit(); 3]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); reader.read_buf(buf.unfilled()).unwrap(); @@ -70,7 +72,7 @@ fn test_buffered_reader_read_buf() { assert_eq!(reader.buffer(), []); let buf: &mut [_] = &mut [MaybeUninit::uninit(); 2]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); reader.read_buf(buf.unfilled()).unwrap(); @@ -78,7 +80,7 @@ fn test_buffered_reader_read_buf() { assert_eq!(reader.buffer(), []); let buf: &mut [_] = &mut [MaybeUninit::uninit(); 1]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); reader.read_buf(buf.unfilled()).unwrap(); @@ -86,7 +88,7 @@ fn test_buffered_reader_read_buf() { assert_eq!(reader.buffer(), [3]); let buf: &mut [_] = &mut [MaybeUninit::uninit(); 3]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); reader.read_buf(buf.unfilled()).unwrap(); diff --git a/library/std/src/io/copy.rs b/library/std/src/io/copy.rs index 193bcd47467c1..1efd98b92aa00 100644 --- a/library/std/src/io/copy.rs +++ b/library/std/src/io/copy.rs @@ -1,4 +1,4 @@ -use super::{BorrowBuf, BufWriter, ErrorKind, Read, Result, Write, DEFAULT_BUF_SIZE}; +use super::{BorrowedBuf, BufWriter, ErrorKind, Read, Result, Write, DEFAULT_BUF_SIZE}; use crate::mem::MaybeUninit; /// Copies the entire contents of a reader into a writer. @@ -97,7 +97,7 @@ impl BufferedCopySpec for BufWriter { loop { let buf = writer.buffer_mut(); - let mut read_buf: BorrowBuf<'_> = buf.spare_capacity_mut().into(); + let mut read_buf: BorrowedBuf<'_> = buf.spare_capacity_mut().into(); unsafe { // SAFETY: init is either 0 or the init_len from the previous iteration. @@ -117,7 +117,7 @@ impl BufferedCopySpec for BufWriter { init = read_buf.init_len() - bytes_read; len += bytes_read as u64; - // SAFETY: BorrowBuf guarantees all of its filled bytes are init + // SAFETY: BorrowedBuf guarantees all of its filled bytes are init unsafe { buf.set_len(buf.len() + bytes_read) }; // Read again if the buffer still has enough capacity, as BufWriter itself would do @@ -139,7 +139,7 @@ fn stack_buffer_copy( writer: &mut W, ) -> Result { let buf: &mut [_] = &mut [MaybeUninit::uninit(); DEFAULT_BUF_SIZE]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); let mut len = 0; diff --git a/library/std/src/io/cursor.rs b/library/std/src/io/cursor.rs index 460b1504ffbfc..e00577b51073e 100644 --- a/library/std/src/io/cursor.rs +++ b/library/std/src/io/cursor.rs @@ -5,7 +5,7 @@ use crate::io::prelude::*; use crate::alloc::Allocator; use crate::cmp; -use crate::io::{self, BorrowCursor, ErrorKind, IoSlice, IoSliceMut, SeekFrom}; +use crate::io::{self, BorrowedCursor, ErrorKind, IoSlice, IoSliceMut, SeekFrom}; /// A `Cursor` wraps an in-memory buffer and provides it with a /// [`Seek`] implementation. @@ -323,7 +323,7 @@ where Ok(n) } - fn read_buf(&mut self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { let prev_written = cursor.written(); Read::read_buf(&mut self.fill_buf()?, cursor.clone())?; diff --git a/library/std/src/io/impls.rs b/library/std/src/io/impls.rs index eee5ab6ec1050..183c8c660b490 100644 --- a/library/std/src/io/impls.rs +++ b/library/std/src/io/impls.rs @@ -6,7 +6,7 @@ use crate::cmp; use crate::collections::VecDeque; use crate::fmt; use crate::io::{ - self, BorrowCursor, BufRead, ErrorKind, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write, + self, BorrowedCursor, BufRead, ErrorKind, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write, }; use crate::mem; @@ -21,7 +21,7 @@ impl Read for &mut R { } #[inline] - fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { (**self).read_buf(cursor) } @@ -125,7 +125,7 @@ impl Read for Box { } #[inline] - fn read_buf(&mut self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { (**self).read_buf(cursor) } @@ -249,7 +249,7 @@ impl Read for &[u8] { } #[inline] - fn read_buf(&mut self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { let amt = cmp::min(cursor.capacity(), self.len()); let (a, b) = self.split_at(amt); @@ -427,10 +427,10 @@ impl Read for VecDeque { } #[inline] - fn read_buf(&mut self, buf: &mut ReadBuf<'_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { let (ref mut front, _) = self.as_slices(); - let n = cmp::min(buf.remaining(), front.len()); - Read::read_buf(front, buf)?; + let n = cmp::min(cursor.capacity(), front.len()); + Read::read_buf(front, cursor)?; self.drain(..n); Ok(()) } diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index b3218b2831d3f..02f82a7e9957a 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -278,7 +278,7 @@ pub use self::{ }; #[unstable(feature = "read_buf", issue = "78485")] -pub use self::readbuf::{BorrowBuf, BorrowCursor}; +pub use self::readbuf::{BorrowedBuf, BorrowedCursor}; pub(crate) use error::const_io_error; mod buffered; @@ -362,7 +362,7 @@ pub(crate) fn default_read_to_end(r: &mut R, buf: &mut Vec buf.reserve(32); // buf is full, need more space } - let mut read_buf: BorrowBuf<'_> = buf.spare_capacity_mut().into(); + let mut read_buf: BorrowedBuf<'_> = buf.spare_capacity_mut().into(); // SAFETY: These bytes were initialized but not filled in the previous loop unsafe { @@ -383,7 +383,7 @@ pub(crate) fn default_read_to_end(r: &mut R, buf: &mut Vec // store how much was initialized but not filled initialized = cursor.init_ref().len(); - // SAFETY: BorrowBuf's invariants mean this much memory is initialized. + // SAFETY: BorrowedBuf's invariants mean this much memory is initialized. unsafe { let new_len = read_buf.filled().len() + buf.len(); buf.set_len(new_len); @@ -462,7 +462,7 @@ pub(crate) fn default_read_exact(this: &mut R, mut buf: &mut [ } } -pub(crate) fn default_read_buf(read: F, mut cursor: BorrowCursor<'_, '_>) -> Result<()> +pub(crate) fn default_read_buf(read: F, mut cursor: BorrowedCursor<'_, '_>) -> Result<()> where F: FnOnce(&mut [u8]) -> Result, { @@ -805,24 +805,23 @@ pub trait Read { default_read_exact(self, buf) } - // TODO naming, if should the method be read_cursor? Or should we change the names of the data structures? /// Pull some bytes from this source into the specified buffer. /// - /// This is equivalent to the [`read`](Read::read) method, except that it is passed a [`BorrowCursor`] rather than `[u8]` to allow use + /// This is equivalent to the [`read`](Read::read) method, except that it is passed a [`BorrowedCursor`] rather than `[u8]` to allow use /// with uninitialized buffers. The new data will be appended to any existing contents of `buf`. /// /// The default implementation delegates to `read`. #[unstable(feature = "read_buf", issue = "78485")] - fn read_buf(&mut self, buf: BorrowCursor<'_, '_>) -> Result<()> { + fn read_buf(&mut self, buf: BorrowedCursor<'_, '_>) -> Result<()> { default_read_buf(|b| self.read(b), buf) } /// Read the exact number of bytes required to fill `cursor`. /// - /// This is equivalent to the [`read_exact`](Read::read_exact) method, except that it is passed a [`BorrowCursor`] rather than `[u8]` to + /// This is equivalent to the [`read_exact`](Read::read_exact) method, except that it is passed a [`BorrowedCursor`] rather than `[u8]` to /// allow use with uninitialized buffers. #[unstable(feature = "read_buf", issue = "78485")] - fn read_buf_exact(&mut self, mut cursor: BorrowCursor<'_, '_>) -> Result<()> { + fn read_buf_exact(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> Result<()> { while cursor.capacity() > 0 { let prev_written = cursor.written(); match self.read_buf(cursor.clone()) { @@ -2587,7 +2586,7 @@ impl Read for Take { Ok(n) } - fn read_buf(&mut self, mut buf: BorrowCursor<'_, '_>) -> Result<()> { + fn read_buf(&mut self, mut buf: BorrowedCursor<'_, '_>) -> Result<()> { // Don't call into inner reader at all at EOF because it may still block if self.limit == 0 { return Ok(()); @@ -2602,7 +2601,7 @@ impl Read for Take { // SAFETY: no uninit data is written to ibuf let ibuf = unsafe { &mut buf.as_mut()[..limit] }; - let mut sliced_buf: BorrowBuf<'_> = ibuf.into(); + let mut sliced_buf: BorrowedBuf<'_> = ibuf.into(); // SAFETY: extra_init bytes of ibuf are known to be initialized unsafe { diff --git a/library/std/src/io/readbuf.rs b/library/std/src/io/readbuf.rs index 8783763fd425f..ae3fbcc6a2f14 100644 --- a/library/std/src/io/readbuf.rs +++ b/library/std/src/io/readbuf.rs @@ -5,6 +5,7 @@ mod tests; use crate::cmp; use crate::fmt::{self, Debug, Formatter}; +use crate::io::{Result, Write}; use crate::mem::MaybeUninit; /// A borrowed byte buffer which is incrementally filled and initialized. @@ -21,58 +22,58 @@ use crate::mem::MaybeUninit; /// [ initialized | uninitialized ] /// ``` /// -/// A `BorrowBuf` is created around some existing data (or capacity for data) via a unique reference -/// (`&mut`). The `BorrowBuf` can be configured (e.g., using `clear` or `set_init`), but otherwise -/// is read-only. To write into the buffer, use `unfilled` to create a `BorrowCursor`. The cursor +/// A `BorrowedBuf` is created around some existing data (or capacity for data) via a unique reference +/// (`&mut`). The `BorrowedBuf` can be configured (e.g., using `clear` or `set_init`), but otherwise +/// is read-only. To write into the buffer, use `unfilled` to create a `BorrowedCursor`. The cursor /// has write-only access to the unfilled portion of the buffer (you can think of it like a /// write-only iterator). /// -/// The lifetime `'a` is a bound on the lifetime of the underlying data. -pub struct BorrowBuf<'a> { +/// The lifetime `'data` is a bound on the lifetime of the underlying data. +pub struct BorrowedBuf<'data> { /// The buffer's underlying data. - buf: &'a mut [MaybeUninit], + buf: &'data mut [MaybeUninit], /// The length of `self.buf` which is known to be filled. filled: usize, /// The length of `self.buf` which is known to be initialized. - initialized: usize, + init: usize, } -impl Debug for BorrowBuf<'_> { +impl Debug for BorrowedBuf<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - f.debug_struct("ReadBuf") - .field("init", &self.initialized) + f.debug_struct("BorrowedBuf") + .field("init", &self.init) .field("filled", &self.filled) .field("capacity", &self.capacity()) .finish() } } -/// Create a new `BorrowBuf` from a fully initialized slice. -impl<'a> From<&'a mut [u8]> for BorrowBuf<'a> { +/// Create a new `BorrowedBuf` from a fully initialized slice. +impl<'data> From<&'data mut [u8]> for BorrowedBuf<'data> { #[inline] - fn from(slice: &'a mut [u8]) -> BorrowBuf<'a> { + fn from(slice: &'data mut [u8]) -> BorrowedBuf<'data> { let len = slice.len(); - BorrowBuf { - //SAFETY: initialized data never becoming uninitialized is an invariant of BorrowBuf + BorrowedBuf { + //SAFETY: initialized data never becoming uninitialized is an invariant of BorrowedBuf buf: unsafe { (slice as *mut [u8]).as_uninit_slice_mut().unwrap() }, filled: 0, - initialized: len, + init: len, } } } -/// Create a new `BorrowBuf` from an uninitialized buffer. +/// Create a new `BorrowedBuf` from an uninitialized buffer. /// /// Use `set_init` if part of the buffer is known to be already initialized. -impl<'a> From<&'a mut [MaybeUninit]> for BorrowBuf<'a> { +impl<'data> From<&'data mut [MaybeUninit]> for BorrowedBuf<'data> { #[inline] - fn from(buf: &'a mut [MaybeUninit]) -> BorrowBuf<'a> { - BorrowBuf { buf, filled: 0, initialized: 0 } + fn from(buf: &'data mut [MaybeUninit]) -> BorrowedBuf<'data> { + BorrowedBuf { buf, filled: 0, init: 0 } } } -impl<'a> BorrowBuf<'a> { +impl<'data> BorrowedBuf<'data> { /// Returns the total capacity of the buffer. #[inline] pub fn capacity(&self) -> usize { @@ -88,7 +89,7 @@ impl<'a> BorrowBuf<'a> { /// Returns the length of the initialized part of the buffer. #[inline] pub fn init_len(&self) -> usize { - self.initialized + self.init } /// Returns a shared reference to the filled portion of the buffer. @@ -100,8 +101,8 @@ impl<'a> BorrowBuf<'a> { /// Returns a cursor over the unfilled part of the buffer. #[inline] - pub fn unfilled<'this>(&'this mut self) -> BorrowCursor<'this, 'a> { - BorrowCursor { start: self.filled, buf: self } + pub fn unfilled<'this>(&'this mut self) -> BorrowedCursor<'this, 'data> { + BorrowedCursor { start: self.filled, buf: self } } /// Clears the buffer, resetting the filled region to empty. @@ -115,7 +116,7 @@ impl<'a> BorrowBuf<'a> { /// Asserts that the first `n` bytes of the buffer are initialized. /// - /// `BorrowBuf` assumes that bytes are never de-initialized, so this method does nothing when called with fewer + /// `BorrowedBuf` assumes that bytes are never de-initialized, so this method does nothing when called with fewer /// bytes than are already known to be initialized. /// /// # Safety @@ -123,42 +124,42 @@ impl<'a> BorrowBuf<'a> { /// The caller must ensure that the first `n` unfilled bytes of the buffer have already been initialized. #[inline] pub unsafe fn set_init(&mut self, n: usize) -> &mut Self { - self.initialized = cmp::max(self.initialized, n); + self.init = cmp::max(self.init, n); self } } -/// A writeable view of the unfilled portion of a [`BorrowBuf`](BorrowBuf). +/// A writeable view of the unfilled portion of a [`BorrowedBuf`](BorrowedBuf). /// -/// Provides access to the initialized and uninitialized parts of the underlying `BorrowBuf`. -/// Data can be written directly to the cursor by using [`append`](BorrowCursor::append) or +/// Provides access to the initialized and uninitialized parts of the underlying `BorrowedBuf`. +/// Data can be written directly to the cursor by using [`append`](BorrowedCursor::append) or /// indirectly by getting a slice of part or all of the cursor and writing into the slice. In the -/// indirect case, the caller must call [`advance`](BorrowCursor::advance) after writing to inform +/// indirect case, the caller must call [`advance`](BorrowedCursor::advance) after writing to inform /// the cursor how many bytes have been written. /// /// Once data is written to the cursor, it becomes part of the filled portion of the underlying -/// `BorrowBuf` and can no longer be accessed or re-written by the cursor. I.e., the cursor tracks -/// the unfilled part of the underlying `BorrowBuf`. +/// `BorrowedBuf` and can no longer be accessed or re-written by the cursor. I.e., the cursor tracks +/// the unfilled part of the underlying `BorrowedBuf`. /// /// The `'buf` lifetime is a bound on the lifetime of the underlying buffer. `'data` is a bound on /// that buffer's underlying data. #[derive(Debug)] -pub struct BorrowCursor<'buf, 'data> { +pub struct BorrowedCursor<'buf, 'data> { /// The underlying buffer. - buf: &'buf mut BorrowBuf<'data>, + buf: &'buf mut BorrowedBuf<'data>, /// The length of the filled portion of the underlying buffer at the time of the cursor's /// creation. start: usize, } -impl<'buf, 'data> BorrowCursor<'buf, 'data> { +impl<'buf, 'data> BorrowedCursor<'buf, 'data> { /// Clone this cursor. /// /// Since a cursor maintains unique access to its underlying buffer, the cloned cursor is not /// accessible while the clone is alive. #[inline] - pub fn clone<'this>(&'this mut self) -> BorrowCursor<'this, 'data> { - BorrowCursor { buf: self.buf, start: self.start } + pub fn clone<'this>(&'this mut self) -> BorrowedCursor<'this, 'data> { + BorrowedCursor { buf: self.buf, start: self.start } } /// Returns the available space in the cursor. @@ -167,7 +168,7 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { self.buf.capacity() - self.buf.filled } - /// Returns the number of bytes written to this cursor since it was created from a `BorrowBuf`. + /// Returns the number of bytes written to this cursor since it was created from a `BorrowedBuf`. /// /// Note that if this cursor is a clone of another, then the count returned is the count written /// via either cursor, not the count since the cursor was cloned. @@ -180,9 +181,7 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { #[inline] pub fn init_ref(&self) -> &[u8] { //SAFETY: We only slice the initialized part of the buffer, which is always valid - unsafe { - MaybeUninit::slice_assume_init_ref(&self.buf.buf[self.buf.filled..self.buf.initialized]) - } + unsafe { MaybeUninit::slice_assume_init_ref(&self.buf.buf[self.buf.filled..self.buf.init]) } } /// Returns a mutable reference to the initialized portion of the cursor. @@ -190,9 +189,7 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { pub fn init_mut(&mut self) -> &mut [u8] { //SAFETY: We only slice the initialized part of the buffer, which is always valid unsafe { - MaybeUninit::slice_assume_init_mut( - &mut self.buf.buf[self.buf.filled..self.buf.initialized], - ) + MaybeUninit::slice_assume_init_mut(&mut self.buf.buf[self.buf.filled..self.buf.init]) } } @@ -201,7 +198,7 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { /// It is safe to uninitialize any of these bytes. #[inline] pub fn uninit_mut(&mut self) -> &mut [MaybeUninit] { - &mut self.buf.buf[self.buf.initialized..] + &mut self.buf.buf[self.buf.init..] } /// Returns a mutable reference to the whole cursor. @@ -227,7 +224,7 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { #[inline] pub unsafe fn advance(&mut self, n: usize) -> &mut Self { self.buf.filled += n; - self.buf.initialized = cmp::max(self.buf.initialized, self.buf.filled); + self.buf.init = cmp::max(self.buf.init, self.buf.filled); self } @@ -237,14 +234,14 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { for byte in self.uninit_mut() { byte.write(0); } - self.buf.initialized = self.buf.capacity(); + self.buf.init = self.buf.capacity(); self } /// Asserts that the first `n` unfilled bytes of the cursor are initialized. /// - /// `BorrowBuf` assumes that bytes are never de-initialized, so this method does nothing when + /// `BorrowedBuf` assumes that bytes are never de-initialized, so this method does nothing when /// called with fewer bytes than are already known to be initialized. /// /// # Safety @@ -252,7 +249,7 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { /// The caller must ensure that the first `n` bytes of the buffer have already been initialized. #[inline] pub unsafe fn set_init(&mut self, n: usize) -> &mut Self { - self.buf.initialized = cmp::max(self.buf.initialized, self.buf.filled + n); + self.buf.init = cmp::max(self.buf.init, self.buf.filled + n); self } @@ -272,9 +269,19 @@ impl<'buf, 'data> BorrowCursor<'buf, 'data> { // SAFETY: We just added the entire contents of buf to the filled section. unsafe { - self.set_init(buf.len()); } self.buf.filled += buf.len(); } } + +impl<'buf, 'data> Write for BorrowedCursor<'buf, 'data> { + fn write(&mut self, buf: &[u8]) -> Result { + self.append(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> Result<()> { + Ok(()) + } +} diff --git a/library/std/src/io/readbuf/tests.rs b/library/std/src/io/readbuf/tests.rs index 584e5de982e97..8037a95790899 100644 --- a/library/std/src/io/readbuf/tests.rs +++ b/library/std/src/io/readbuf/tests.rs @@ -1,11 +1,11 @@ -use super::BorrowBuf; +use super::BorrowedBuf; use crate::mem::MaybeUninit; -/// Test that BorrowBuf has the correct numbers when created with new +/// Test that BorrowedBuf has the correct numbers when created with new #[test] fn new() { let buf: &mut [_] = &mut [0; 16]; - let mut rbuf: BorrowBuf<'_> = buf.into(); + let mut rbuf: BorrowedBuf<'_> = buf.into(); assert_eq!(rbuf.filled().len(), 0); assert_eq!(rbuf.init_len(), 16); @@ -13,11 +13,11 @@ fn new() { assert_eq!(rbuf.unfilled().capacity(), 16); } -/// Test that BorrowBuf has the correct numbers when created with uninit +/// Test that BorrowedBuf has the correct numbers when created with uninit #[test] fn uninit() { let buf: &mut [_] = &mut [MaybeUninit::uninit(); 16]; - let mut rbuf: BorrowBuf<'_> = buf.into(); + let mut rbuf: BorrowedBuf<'_> = buf.into(); assert_eq!(rbuf.filled().len(), 0); assert_eq!(rbuf.init_len(), 0); @@ -28,7 +28,7 @@ fn uninit() { #[test] fn initialize_unfilled() { let buf: &mut [_] = &mut [MaybeUninit::uninit(); 16]; - let mut rbuf: BorrowBuf<'_> = buf.into(); + let mut rbuf: BorrowedBuf<'_> = buf.into(); rbuf.unfilled().ensure_init(); @@ -36,9 +36,9 @@ fn initialize_unfilled() { } #[test] -fn add_filled() { +fn addvance_filled() { let buf: &mut [_] = &mut [0; 16]; - let mut rbuf: BorrowBuf<'_> = buf.into(); + let mut rbuf: BorrowedBuf<'_> = buf.into(); unsafe { rbuf.unfilled().advance(1); @@ -51,7 +51,7 @@ fn add_filled() { #[test] fn clear() { let buf: &mut [_] = &mut [255; 16]; - let mut rbuf: BorrowBuf<'_> = buf.into(); + let mut rbuf: BorrowedBuf<'_> = buf.into(); unsafe { rbuf.unfilled().advance(16); @@ -71,7 +71,7 @@ fn clear() { #[test] fn set_init() { let buf: &mut [_] = &mut [MaybeUninit::uninit(); 16]; - let mut rbuf: BorrowBuf<'_> = buf.into(); + let mut rbuf: BorrowedBuf<'_> = buf.into(); unsafe { rbuf.set_init(8); @@ -99,7 +99,7 @@ fn set_init() { #[test] fn append() { let buf: &mut [_] = &mut [MaybeUninit::new(255); 16]; - let mut rbuf: BorrowBuf<'_> = buf.into(); + let mut rbuf: BorrowedBuf<'_> = buf.into(); rbuf.unfilled().append(&[0; 8]); @@ -115,3 +115,61 @@ fn append() { assert_eq!(rbuf.filled().len(), 16); assert_eq!(rbuf.filled(), [1; 16]); } + +#[test] +fn clone_written() { + let buf: &mut [_] = &mut [MaybeUninit::new(0); 32]; + let mut buf: BorrowedBuf<'_> = buf.into(); + + let mut cursor = buf.unfilled(); + cursor.append(&[1; 16]); + + let mut cursor2 = cursor.clone(); + cursor2.append(&[2; 16]); + + assert_eq!(cursor2.written(), 32); + assert_eq!(cursor.written(), 32); + + assert_eq!(buf.unfilled().written(), 0); + assert_eq!(buf.init_len(), 32); + assert_eq!(buf.filled().len(), 32); + let filled = buf.filled(); + assert_eq!(&filled[..16], [1; 16]); + assert_eq!(&filled[16..], [2; 16]); +} + +#[test] +fn cursor_set_init() { + let buf: &mut [_] = &mut [MaybeUninit::uninit(); 16]; + let mut rbuf: BorrowedBuf<'_> = buf.into(); + + unsafe { + rbuf.unfilled().set_init(8); + } + + assert_eq!(rbuf.init_len(), 8); + assert_eq!(rbuf.unfilled().init_ref().len(), 8); + assert_eq!(rbuf.unfilled().init_mut().len(), 8); + assert_eq!(rbuf.unfilled().uninit_mut().len(), 8); + assert_eq!(unsafe { rbuf.unfilled().as_mut() }.len(), 16); + + unsafe { + rbuf.unfilled().advance(4); + } + + unsafe { + rbuf.unfilled().set_init(2); + } + + assert_eq!(rbuf.init_len(), 8); + + unsafe { + rbuf.unfilled().set_init(8); + } + + assert_eq!(rbuf.init_len(), 12); + assert_eq!(rbuf.unfilled().init_ref().len(), 8); + assert_eq!(rbuf.unfilled().init_mut().len(), 8); + assert_eq!(rbuf.unfilled().uninit_mut().len(), 4); + assert_eq!(unsafe { rbuf.unfilled().as_mut() }.len(), 12); +} diff --git a/library/std/src/io/tests.rs b/library/std/src/io/tests.rs index a1322a185651e..c5c476ec3bfee 100644 --- a/library/std/src/io/tests.rs +++ b/library/std/src/io/tests.rs @@ -1,4 +1,4 @@ -use super::{repeat, BorrowBuf, Cursor, SeekFrom}; +use super::{repeat, BorrowedBuf, Cursor, SeekFrom}; use crate::cmp::{self, min}; use crate::io::{self, IoSlice, IoSliceMut}; use crate::io::{BufRead, BufReader, Read, Seek, Write}; @@ -160,7 +160,7 @@ fn read_exact_slice() { #[test] fn read_buf_exact() { let buf: &mut [_] = &mut [0; 4]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); let mut c = Cursor::new(&b""[..]); assert_eq!(c.read_buf_exact(buf.unfilled()).unwrap_err().kind(), io::ErrorKind::UnexpectedEof); @@ -616,7 +616,7 @@ fn bench_take_read_buf(b: &mut test::Bencher) { b.iter(|| { let buf: &mut [_] = &mut [MaybeUninit::uninit(); 64]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); [255; 128].take(64).read_buf(buf.unfilled()).unwrap(); }); diff --git a/library/std/src/io/util.rs b/library/std/src/io/util.rs index 5149926fd519d..7475d71119a76 100644 --- a/library/std/src/io/util.rs +++ b/library/std/src/io/util.rs @@ -5,7 +5,7 @@ mod tests; use crate::fmt; use crate::io::{ - self, BorrowCursor, BufRead, IoSlice, IoSliceMut, Read, Seek, SeekFrom, SizeHint, Write, + self, BorrowedCursor, BufRead, IoSlice, IoSliceMut, Read, Seek, SeekFrom, SizeHint, Write, }; /// A reader which is always at EOF. @@ -47,7 +47,7 @@ impl Read for Empty { } #[inline] - fn read_buf(&mut self, _cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, _cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { Ok(()) } } @@ -130,7 +130,7 @@ impl Read for Repeat { Ok(buf.len()) } - fn read_buf(&mut self, mut buf: BorrowCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut buf: BorrowedCursor<'_, '_>) -> io::Result<()> { // SAFETY: No uninit bytes are being written for slot in unsafe { buf.as_mut() } { slot.write(self.byte); diff --git a/library/std/src/io/util/tests.rs b/library/std/src/io/util/tests.rs index 025173c3f446c..ce5e2c9da1dbf 100644 --- a/library/std/src/io/util/tests.rs +++ b/library/std/src/io/util/tests.rs @@ -1,7 +1,7 @@ use crate::cmp::{max, min}; use crate::io::prelude::*; use crate::io::{ - copy, empty, repeat, sink, BorrowBuf, BufWriter, Empty, Repeat, Result, SeekFrom, Sink, + copy, empty, repeat, sink, BorrowedBuf, BufWriter, Empty, Repeat, Result, SeekFrom, Sink, DEFAULT_BUF_SIZE, }; @@ -80,25 +80,25 @@ fn empty_reads() { assert_eq!(e.by_ref().read(&mut [0; 1024]).unwrap(), 0); let buf: &mut [MaybeUninit<_>] = &mut []; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); e.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.len(), 0); assert_eq!(buf.init_len(), 0); let buf: &mut [_] = &mut [MaybeUninit::uninit()]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); e.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.len(), 0); assert_eq!(buf.init_len(), 0); let buf: &mut [_] = &mut [MaybeUninit::uninit(); 1024]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); e.read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.len(), 0); assert_eq!(buf.init_len(), 0); let buf: &mut [_] = &mut [MaybeUninit::uninit(); 1024]; - let mut buf: BorrowBuf<'_> = buf.into(); + let mut buf: BorrowedBuf<'_> = buf.into(); e.by_ref().read_buf(buf.unfilled()).unwrap(); assert_eq!(buf.len(), 0); assert_eq!(buf.init_len(), 0); diff --git a/library/std/src/sys/hermit/fs.rs b/library/std/src/sys/hermit/fs.rs index fa9a7fb19e463..51321c51972ac 100644 --- a/library/std/src/sys/hermit/fs.rs +++ b/library/std/src/sys/hermit/fs.rs @@ -2,7 +2,7 @@ use crate::ffi::{CStr, CString, OsString}; use crate::fmt; use crate::hash::{Hash, Hasher}; use crate::io::{self, Error, ErrorKind}; -use crate::io::{IoSlice, IoSliceMut, ReadBuf, SeekFrom}; +use crate::io::{BorrowedCursor, IoSlice, IoSliceMut, SeekFrom}; use crate::os::unix::ffi::OsStrExt; use crate::path::{Path, PathBuf}; use crate::sys::cvt; @@ -312,8 +312,8 @@ impl File { false } - pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - crate::io::default_read_buf(|buf| self.read(buf), buf) + pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + crate::io::default_read_buf(|buf| self.read(buf), cursor) } pub fn write(&self, buf: &[u8]) -> io::Result { diff --git a/library/std/src/sys/solid/fs.rs b/library/std/src/sys/solid/fs.rs index a2cbee4dcf07b..0848d3d8f102d 100644 --- a/library/std/src/sys/solid/fs.rs +++ b/library/std/src/sys/solid/fs.rs @@ -2,7 +2,7 @@ use super::{abi, error}; use crate::{ ffi::{CStr, CString, OsStr, OsString}, fmt, - io::{self, IoSlice, IoSliceMut, ReadBuf, SeekFrom}, + io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom}, mem::MaybeUninit, os::raw::{c_int, c_short}, os::solid::ffi::OsStrExt, @@ -358,13 +358,13 @@ impl File { } } - pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> { + pub fn read_buf(&self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { unsafe { - let len = buf.remaining(); + let len = cursor.capacity(); let mut out_num_bytes = MaybeUninit::uninit(); error::SolidError::err_if_negative(abi::SOLID_FS_Read( self.fd.raw(), - buf.unfilled_mut().as_mut_ptr() as *mut u8, + cursor.as_mut().as_mut_ptr() as *mut u8, len, out_num_bytes.as_mut_ptr(), )) @@ -376,9 +376,7 @@ impl File { // Safety: `num_bytes_read` bytes were written to the unfilled // portion of the buffer - buf.assume_init(num_bytes_read); - - buf.add_filled(num_bytes_read); + cursor.advance(num_bytes_read); Ok(()) } diff --git a/library/std/src/sys/unix/fd.rs b/library/std/src/sys/unix/fd.rs index 6adb734fb0a2f..76a269bb9b59c 100644 --- a/library/std/src/sys/unix/fd.rs +++ b/library/std/src/sys/unix/fd.rs @@ -4,7 +4,7 @@ mod tests; use crate::cmp; -use crate::io::{self, BorrowCursor, IoSlice, IoSliceMut, Read}; +use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, Read}; use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use crate::sys::cvt; use crate::sys_common::{AsInner, FromInner, IntoInner}; @@ -131,7 +131,7 @@ impl FileDesc { } } - pub fn read_buf(&self, mut cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { let ret = cvt(unsafe { libc::read( self.as_raw_fd(), diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 374f9f72d6d74..5056134544279 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -2,7 +2,7 @@ use crate::os::unix::prelude::*; use crate::ffi::{CStr, CString, OsStr, OsString}; use crate::fmt; -use crate::io::{self, BorrowCursor, Error, IoSlice, IoSliceMut, SeekFrom}; +use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom}; use crate::mem; use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd}; use crate::path::{Path, PathBuf}; @@ -1031,7 +1031,7 @@ impl File { self.0.read_at(buf, offset) } - pub fn read_buf(&self, cursor: BorrowCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { self.0.read_buf(cursor) } diff --git a/library/std/src/sys/unsupported/fs.rs b/library/std/src/sys/unsupported/fs.rs index 0e1a6257ed763..41e39ce27cec9 100644 --- a/library/std/src/sys/unsupported/fs.rs +++ b/library/std/src/sys/unsupported/fs.rs @@ -1,7 +1,7 @@ use crate::ffi::OsString; use crate::fmt; use crate::hash::{Hash, Hasher}; -use crate::io::{self, IoSlice, IoSliceMut, ReadBuf, SeekFrom}; +use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom}; use crate::path::{Path, PathBuf}; use crate::sys::time::SystemTime; use crate::sys::unsupported; @@ -214,7 +214,7 @@ impl File { self.0 } - pub fn read_buf(&self, _buf: &mut ReadBuf<'_>) -> io::Result<()> { + pub fn read_buf(&self, _cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { self.0 } diff --git a/library/std/src/sys/wasi/fs.rs b/library/std/src/sys/wasi/fs.rs index 6614ae397b57f..b5b5eab1a24ec 100644 --- a/library/std/src/sys/wasi/fs.rs +++ b/library/std/src/sys/wasi/fs.rs @@ -3,7 +3,7 @@ use super::fd::WasiFd; use crate::ffi::{CStr, CString, OsStr, OsString}; use crate::fmt; -use crate::io::{self, IoSlice, IoSliceMut, ReadBuf, SeekFrom}; +use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom}; use crate::iter; use crate::mem::{self, ManuallyDrop}; use crate::os::raw::c_int; @@ -439,8 +439,8 @@ impl File { true } - pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - crate::io::default_read_buf(|buf| self.read(buf), buf) + pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + crate::io::default_read_buf(|buf| self.read(buf), cursor) } pub fn write(&self, buf: &[u8]) -> io::Result { diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index aed082b3e0abf..bfc2477dff46b 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -2,7 +2,7 @@ use crate::os::windows::prelude::*; use crate::ffi::OsString; use crate::fmt; -use crate::io::{self, Error, IoSlice, IoSliceMut, ReadBuf, SeekFrom}; +use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom}; use crate::mem; use crate::os::windows::io::{AsHandle, BorrowedHandle}; use crate::path::{Path, PathBuf}; @@ -415,8 +415,8 @@ impl File { self.handle.read_at(buf, offset) } - pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - self.handle.read_buf(buf) + pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + self.handle.read_buf(cursor) } pub fn write(&self, buf: &[u8]) -> io::Result { diff --git a/library/std/src/sys/windows/handle.rs b/library/std/src/sys/windows/handle.rs index e24b09cc96ec8..0ea6876af5b93 100644 --- a/library/std/src/sys/windows/handle.rs +++ b/library/std/src/sys/windows/handle.rs @@ -4,7 +4,7 @@ mod tests; use crate::cmp; -use crate::io::{self, ErrorKind, IoSlice, IoSliceMut, Read, ReadBuf}; +use crate::io::{self, BorrowedCursor, ErrorKind, IoSlice, IoSliceMut, Read}; use crate::mem; use crate::os::windows::io::{ AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, @@ -112,18 +112,16 @@ impl Handle { } } - pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> { - let res = unsafe { - self.synchronous_read(buf.unfilled_mut().as_mut_ptr(), buf.remaining(), None) - }; + pub fn read_buf(&self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + let res = + unsafe { self.synchronous_read(cursor.as_mut().as_mut_ptr(), cursor.capacity(), None) }; match res { Ok(read) => { // Safety: `read` bytes were written to the initialized portion of the buffer unsafe { - buf.assume_init(read as usize); + cursor.advance(read as usize); } - buf.add_filled(read as usize); Ok(()) } From 08c97323dee8b8558c8e1dc770885f0095360d32 Mon Sep 17 00:00:00 2001 From: BlackHoleFox Date: Mon, 8 Aug 2022 17:49:19 -0700 Subject: [PATCH 05/19] Add standard C error function aliases Aids the discoverability of `io::Error::last_os_error()` by linking to commonly used error number functions from C/C++. --- library/std/src/io/error.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index ff7fdcae16f53..4e541dda90692 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -564,6 +564,8 @@ impl Error { /// println!("last OS error: {os_error:?}"); /// ``` #[stable(feature = "rust1", since = "1.0.0")] + #[doc(alias = "GetLastError")] + #[doc(alias = "errno")] #[must_use] #[inline] pub fn last_os_error() -> Error { From 8509936e8f48c64c063b7ba858b881377007f416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20D=C4=85bek?= Date: Sun, 14 Aug 2022 11:01:04 +0200 Subject: [PATCH 06/19] Add mention of `BufReader` in `Read::bytes` docs --- library/std/src/io/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 96addbd1a0558..a16e2431ea9fc 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -883,6 +883,10 @@ pub trait Read { /// The yielded item is [`Ok`] if a byte was successfully read and [`Err`] /// otherwise. EOF is mapped to returning [`None`] from this iterator. /// + /// The default implementation calls `read` for each byte, + /// which can be very inefficient for data that's not in memory, + /// such as [`File`]. Consider using a [`BufReader`] in such cases. + /// /// # Examples /// /// [`File`]s implement `Read`: @@ -895,10 +899,11 @@ pub trait Read { /// ```no_run /// use std::io; /// use std::io::prelude::*; + /// use std::io::BufReader; /// use std::fs::File; /// /// fn main() -> io::Result<()> { - /// let f = File::open("foo.txt")?; + /// let f = BufReader::new(File::open("foo.txt")?); /// /// for byte in f.bytes() { /// println!("{}", byte.unwrap()); From ac70aea98509c33ec75208f7b42c8d905c74ebaf Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Thu, 11 Aug 2022 15:52:29 +0100 Subject: [PATCH 07/19] Address reviewer comments Signed-off-by: Nick Cameron --- library/std/src/fs.rs | 4 +- library/std/src/io/buffered/bufreader.rs | 4 +- .../std/src/io/buffered/bufreader/buffer.rs | 2 +- library/std/src/io/copy.rs | 2 +- library/std/src/io/cursor.rs | 4 +- library/std/src/io/impls.rs | 8 +-- library/std/src/io/mod.rs | 16 ++--- library/std/src/io/readbuf.rs | 65 ++++++++++++------- library/std/src/io/readbuf/tests.rs | 4 +- library/std/src/io/util.rs | 4 +- library/std/src/sys/hermit/fs.rs | 2 +- library/std/src/sys/solid/fs.rs | 2 +- library/std/src/sys/unix/fd.rs | 2 +- library/std/src/sys/unix/fs.rs | 2 +- library/std/src/sys/unsupported/fs.rs | 2 +- library/std/src/sys/wasi/fs.rs | 2 +- library/std/src/sys/windows/fs.rs | 2 +- library/std/src/sys/windows/handle.rs | 2 +- 18 files changed, 74 insertions(+), 55 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 98aa40db32176..e1ab06b0d0f69 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -703,7 +703,7 @@ impl Read for File { self.inner.read_vectored(bufs) } - fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_>) -> io::Result<()> { self.inner.read_buf(cursor) } @@ -755,7 +755,7 @@ impl Read for &File { self.inner.read(buf) } - fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_>) -> io::Result<()> { self.inner.read_buf(cursor) } diff --git a/library/std/src/io/buffered/bufreader.rs b/library/std/src/io/buffered/bufreader.rs index dced922ea572e..88ad92d8a9859 100644 --- a/library/std/src/io/buffered/bufreader.rs +++ b/library/std/src/io/buffered/bufreader.rs @@ -266,7 +266,7 @@ impl Read for BufReader { Ok(nread) } - fn read_buf(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> { // If we don't have any buffered data and we're doing a massive read // (larger than our internal buffer), bypass our internal buffer // entirely. @@ -278,7 +278,7 @@ impl Read for BufReader { let prev = cursor.written(); let mut rem = self.fill_buf()?; - rem.read_buf(cursor.clone())?; + rem.read_buf(cursor.reborrow())?; self.consume(cursor.written() - prev); //slice impl of read_buf known to never unfill buf diff --git a/library/std/src/io/buffered/bufreader/buffer.rs b/library/std/src/io/buffered/bufreader/buffer.rs index b122a6c0ccc57..867c22c6041e7 100644 --- a/library/std/src/io/buffered/bufreader/buffer.rs +++ b/library/std/src/io/buffered/bufreader/buffer.rs @@ -93,7 +93,7 @@ impl Buffer { if self.pos >= self.filled { debug_assert!(self.pos == self.filled); - let mut buf: BorrowedBuf<'_> = (&mut *self.buf).into(); + let mut buf = BorrowedBuf::from(&mut *self.buf); // SAFETY: `self.filled` bytes will always have been initialized. unsafe { buf.set_init(self.filled); diff --git a/library/std/src/io/copy.rs b/library/std/src/io/copy.rs index 1efd98b92aa00..38b98afffa168 100644 --- a/library/std/src/io/copy.rs +++ b/library/std/src/io/copy.rs @@ -106,7 +106,7 @@ impl BufferedCopySpec for BufWriter { if read_buf.capacity() >= DEFAULT_BUF_SIZE { let mut cursor = read_buf.unfilled(); - match reader.read_buf(cursor.clone()) { + match reader.read_buf(cursor.reborrow()) { Ok(()) => { let bytes_read = cursor.written(); diff --git a/library/std/src/io/cursor.rs b/library/std/src/io/cursor.rs index e00577b51073e..d98ab021cadb1 100644 --- a/library/std/src/io/cursor.rs +++ b/library/std/src/io/cursor.rs @@ -323,10 +323,10 @@ where Ok(n) } - fn read_buf(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> { let prev_written = cursor.written(); - Read::read_buf(&mut self.fill_buf()?, cursor.clone())?; + Read::read_buf(&mut self.fill_buf()?, cursor.reborrow())?; self.pos += (cursor.written() - prev_written) as u64; diff --git a/library/std/src/io/impls.rs b/library/std/src/io/impls.rs index 183c8c660b490..e5048dcc8acd9 100644 --- a/library/std/src/io/impls.rs +++ b/library/std/src/io/impls.rs @@ -21,7 +21,7 @@ impl Read for &mut R { } #[inline] - fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_>) -> io::Result<()> { (**self).read_buf(cursor) } @@ -125,7 +125,7 @@ impl Read for Box { } #[inline] - fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_>) -> io::Result<()> { (**self).read_buf(cursor) } @@ -249,7 +249,7 @@ impl Read for &[u8] { } #[inline] - fn read_buf(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> { let amt = cmp::min(cursor.capacity(), self.len()); let (a, b) = self.split_at(amt); @@ -427,7 +427,7 @@ impl Read for VecDeque { } #[inline] - fn read_buf(&mut self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, cursor: BorrowedCursor<'_>) -> io::Result<()> { let (ref mut front, _) = self.as_slices(); let n = cmp::min(cursor.capacity(), front.len()); Read::read_buf(front, cursor)?; diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 02f82a7e9957a..8b8ec32bf5b63 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -370,7 +370,7 @@ pub(crate) fn default_read_to_end(r: &mut R, buf: &mut Vec } let mut cursor = read_buf.unfilled(); - match r.read_buf(cursor.clone()) { + match r.read_buf(cursor.reborrow()) { Ok(()) => {} Err(e) if e.kind() == ErrorKind::Interrupted => continue, Err(e) => return Err(e), @@ -462,7 +462,7 @@ pub(crate) fn default_read_exact(this: &mut R, mut buf: &mut [ } } -pub(crate) fn default_read_buf(read: F, mut cursor: BorrowedCursor<'_, '_>) -> Result<()> +pub(crate) fn default_read_buf(read: F, mut cursor: BorrowedCursor<'_>) -> Result<()> where F: FnOnce(&mut [u8]) -> Result, { @@ -812,7 +812,7 @@ pub trait Read { /// /// The default implementation delegates to `read`. #[unstable(feature = "read_buf", issue = "78485")] - fn read_buf(&mut self, buf: BorrowedCursor<'_, '_>) -> Result<()> { + fn read_buf(&mut self, buf: BorrowedCursor<'_>) -> Result<()> { default_read_buf(|b| self.read(b), buf) } @@ -821,10 +821,10 @@ pub trait Read { /// This is equivalent to the [`read_exact`](Read::read_exact) method, except that it is passed a [`BorrowedCursor`] rather than `[u8]` to /// allow use with uninitialized buffers. #[unstable(feature = "read_buf", issue = "78485")] - fn read_buf_exact(&mut self, mut cursor: BorrowedCursor<'_, '_>) -> Result<()> { + fn read_buf_exact(&mut self, mut cursor: BorrowedCursor<'_>) -> Result<()> { while cursor.capacity() > 0 { let prev_written = cursor.written(); - match self.read_buf(cursor.clone()) { + match self.read_buf(cursor.reborrow()) { Ok(()) => {} Err(e) if e.kind() == ErrorKind::Interrupted => continue, Err(e) => return Err(e), @@ -2586,7 +2586,7 @@ impl Read for Take { Ok(n) } - fn read_buf(&mut self, mut buf: BorrowedCursor<'_, '_>) -> Result<()> { + fn read_buf(&mut self, mut buf: BorrowedCursor<'_>) -> Result<()> { // Don't call into inner reader at all at EOF because it may still block if self.limit == 0 { return Ok(()); @@ -2609,7 +2609,7 @@ impl Read for Take { } let mut cursor = sliced_buf.unfilled(); - self.inner.read_buf(cursor.clone())?; + self.inner.read_buf(cursor.reborrow())?; let new_init = cursor.init_ref().len(); let filled = sliced_buf.len(); @@ -2626,7 +2626,7 @@ impl Read for Take { self.limit -= filled as u64; } else { let written = buf.written(); - self.inner.read_buf(buf.clone())?; + self.inner.read_buf(buf.reborrow())?; self.limit -= (buf.written() - written) as u64; } diff --git a/library/std/src/io/readbuf.rs b/library/std/src/io/readbuf.rs index ae3fbcc6a2f14..b1a84095f13fa 100644 --- a/library/std/src/io/readbuf.rs +++ b/library/std/src/io/readbuf.rs @@ -6,7 +6,7 @@ mod tests; use crate::cmp; use crate::fmt::{self, Debug, Formatter}; use crate::io::{Result, Write}; -use crate::mem::MaybeUninit; +use crate::mem::{self, MaybeUninit}; /// A borrowed byte buffer which is incrementally filled and initialized. /// @@ -23,9 +23,9 @@ use crate::mem::MaybeUninit; /// ``` /// /// A `BorrowedBuf` is created around some existing data (or capacity for data) via a unique reference -/// (`&mut`). The `BorrowedBuf` can be configured (e.g., using `clear` or `set_init`), but otherwise -/// is read-only. To write into the buffer, use `unfilled` to create a `BorrowedCursor`. The cursor -/// has write-only access to the unfilled portion of the buffer (you can think of it like a +/// (`&mut`). The `BorrowedBuf` can be configured (e.g., using `clear` or `set_init`), but cannot be +/// directly written. To write into the buffer, use `unfilled` to create a `BorrowedCursor`. The cursor +/// has write-only access to the unfilled portion of the buffer (you can think of it as a /// write-only iterator). /// /// The lifetime `'data` is a bound on the lifetime of the underlying data. @@ -55,7 +55,7 @@ impl<'data> From<&'data mut [u8]> for BorrowedBuf<'data> { let len = slice.len(); BorrowedBuf { - //SAFETY: initialized data never becoming uninitialized is an invariant of BorrowedBuf + // SAFETY: initialized data never becoming uninitialized is an invariant of BorrowedBuf buf: unsafe { (slice as *mut [u8]).as_uninit_slice_mut().unwrap() }, filled: 0, init: len, @@ -95,14 +95,21 @@ impl<'data> BorrowedBuf<'data> { /// Returns a shared reference to the filled portion of the buffer. #[inline] pub fn filled(&self) -> &[u8] { - //SAFETY: We only slice the filled part of the buffer, which is always valid + // SAFETY: We only slice the filled part of the buffer, which is always valid unsafe { MaybeUninit::slice_assume_init_ref(&self.buf[0..self.filled]) } } /// Returns a cursor over the unfilled part of the buffer. #[inline] - pub fn unfilled<'this>(&'this mut self) -> BorrowedCursor<'this, 'data> { - BorrowedCursor { start: self.filled, buf: self } + pub fn unfilled<'this>(&'this mut self) -> BorrowedCursor<'this> { + BorrowedCursor { + start: self.filled, + // SAFETY: we never assign into `BorrowedCursor::buf`, so treating its + // lifetime covariantly is safe. + buf: unsafe { + mem::transmute::<&'this mut BorrowedBuf<'data>, &'this mut BorrowedBuf<'this>>(self) + }, + } } /// Clears the buffer, resetting the filled region to empty. @@ -141,25 +148,37 @@ impl<'data> BorrowedBuf<'data> { /// `BorrowedBuf` and can no longer be accessed or re-written by the cursor. I.e., the cursor tracks /// the unfilled part of the underlying `BorrowedBuf`. /// -/// The `'buf` lifetime is a bound on the lifetime of the underlying buffer. `'data` is a bound on -/// that buffer's underlying data. +/// The lifetime `'a` is a bound on the lifetime of the underlying buffer (which means it is a bound +/// on the data in that buffer by transitivity). #[derive(Debug)] -pub struct BorrowedCursor<'buf, 'data> { +pub struct BorrowedCursor<'a> { /// The underlying buffer. - buf: &'buf mut BorrowedBuf<'data>, + // Safety invariant: we treat the type of buf as covariant in the lifetime of `BorrowedBuf` when + // we create a `BorrowedCursor`. This is only safe if we never replace `buf` by assigning into + // it, so don't do that! + buf: &'a mut BorrowedBuf<'a>, /// The length of the filled portion of the underlying buffer at the time of the cursor's /// creation. start: usize, } -impl<'buf, 'data> BorrowedCursor<'buf, 'data> { - /// Clone this cursor. +impl<'a> BorrowedCursor<'a> { + /// Reborrow this cursor by cloning it with a smaller lifetime. /// - /// Since a cursor maintains unique access to its underlying buffer, the cloned cursor is not - /// accessible while the clone is alive. + /// Since a cursor maintains unique access to its underlying buffer, the borrowed cursor is + /// not accessible while the new cursor exists. #[inline] - pub fn clone<'this>(&'this mut self) -> BorrowedCursor<'this, 'data> { - BorrowedCursor { buf: self.buf, start: self.start } + pub fn reborrow<'this>(&'this mut self) -> BorrowedCursor<'this> { + BorrowedCursor { + // SAFETY: we never assign into `BorrowedCursor::buf`, so treating its + // lifetime covariantly is safe. + buf: unsafe { + mem::transmute::<&'this mut BorrowedBuf<'a>, &'this mut BorrowedBuf<'this>>( + self.buf, + ) + }, + start: self.start, + } } /// Returns the available space in the cursor. @@ -170,8 +189,8 @@ impl<'buf, 'data> BorrowedCursor<'buf, 'data> { /// Returns the number of bytes written to this cursor since it was created from a `BorrowedBuf`. /// - /// Note that if this cursor is a clone of another, then the count returned is the count written - /// via either cursor, not the count since the cursor was cloned. + /// Note that if this cursor is a reborrowed clone of another, then the count returned is the + /// count written via either cursor, not the count since the cursor was reborrowed. #[inline] pub fn written(&self) -> usize { self.buf.filled - self.start @@ -180,14 +199,14 @@ impl<'buf, 'data> BorrowedCursor<'buf, 'data> { /// Returns a shared reference to the initialized portion of the cursor. #[inline] pub fn init_ref(&self) -> &[u8] { - //SAFETY: We only slice the initialized part of the buffer, which is always valid + // SAFETY: We only slice the initialized part of the buffer, which is always valid unsafe { MaybeUninit::slice_assume_init_ref(&self.buf.buf[self.buf.filled..self.buf.init]) } } /// Returns a mutable reference to the initialized portion of the cursor. #[inline] pub fn init_mut(&mut self) -> &mut [u8] { - //SAFETY: We only slice the initialized part of the buffer, which is always valid + // SAFETY: We only slice the initialized part of the buffer, which is always valid unsafe { MaybeUninit::slice_assume_init_mut(&mut self.buf.buf[self.buf.filled..self.buf.init]) } @@ -275,7 +294,7 @@ impl<'buf, 'data> BorrowedCursor<'buf, 'data> { } } -impl<'buf, 'data> Write for BorrowedCursor<'buf, 'data> { +impl<'a> Write for BorrowedCursor<'a> { fn write(&mut self, buf: &[u8]) -> Result { self.append(buf); Ok(buf.len()) diff --git a/library/std/src/io/readbuf/tests.rs b/library/std/src/io/readbuf/tests.rs index 8037a95790899..cc1b423f2dd0d 100644 --- a/library/std/src/io/readbuf/tests.rs +++ b/library/std/src/io/readbuf/tests.rs @@ -117,14 +117,14 @@ fn append() { } #[test] -fn clone_written() { +fn reborrow_written() { let buf: &mut [_] = &mut [MaybeUninit::new(0); 32]; let mut buf: BorrowedBuf<'_> = buf.into(); let mut cursor = buf.unfilled(); cursor.append(&[1; 16]); - let mut cursor2 = cursor.clone(); + let mut cursor2 = cursor.reborrow(); cursor2.append(&[2; 16]); assert_eq!(cursor2.written(), 32); diff --git a/library/std/src/io/util.rs b/library/std/src/io/util.rs index 7475d71119a76..f076ee0923c80 100644 --- a/library/std/src/io/util.rs +++ b/library/std/src/io/util.rs @@ -47,7 +47,7 @@ impl Read for Empty { } #[inline] - fn read_buf(&mut self, _cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, _cursor: BorrowedCursor<'_>) -> io::Result<()> { Ok(()) } } @@ -130,7 +130,7 @@ impl Read for Repeat { Ok(buf.len()) } - fn read_buf(&mut self, mut buf: BorrowedCursor<'_, '_>) -> io::Result<()> { + fn read_buf(&mut self, mut buf: BorrowedCursor<'_>) -> io::Result<()> { // SAFETY: No uninit bytes are being written for slot in unsafe { buf.as_mut() } { slot.write(self.byte); diff --git a/library/std/src/sys/hermit/fs.rs b/library/std/src/sys/hermit/fs.rs index 51321c51972ac..1c5efa94bd36a 100644 --- a/library/std/src/sys/hermit/fs.rs +++ b/library/std/src/sys/hermit/fs.rs @@ -312,7 +312,7 @@ impl File { false } - pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, cursor: BorrowedCursor<'_>) -> io::Result<()> { crate::io::default_read_buf(|buf| self.read(buf), cursor) } diff --git a/library/std/src/sys/solid/fs.rs b/library/std/src/sys/solid/fs.rs index 0848d3d8f102d..8e23a7c7d884a 100644 --- a/library/std/src/sys/solid/fs.rs +++ b/library/std/src/sys/solid/fs.rs @@ -358,7 +358,7 @@ impl File { } } - pub fn read_buf(&self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> { unsafe { let len = cursor.capacity(); let mut out_num_bytes = MaybeUninit::uninit(); diff --git a/library/std/src/sys/unix/fd.rs b/library/std/src/sys/unix/fd.rs index 76a269bb9b59c..dbaa3c33e2e57 100644 --- a/library/std/src/sys/unix/fd.rs +++ b/library/std/src/sys/unix/fd.rs @@ -131,7 +131,7 @@ impl FileDesc { } } - pub fn read_buf(&self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> { let ret = cvt(unsafe { libc::read( self.as_raw_fd(), diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 5056134544279..b8fc2e8da2b75 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1031,7 +1031,7 @@ impl File { self.0.read_at(buf, offset) } - pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, cursor: BorrowedCursor<'_>) -> io::Result<()> { self.0.read_buf(cursor) } diff --git a/library/std/src/sys/unsupported/fs.rs b/library/std/src/sys/unsupported/fs.rs index 41e39ce27cec9..6ac1b5d2bcfca 100644 --- a/library/std/src/sys/unsupported/fs.rs +++ b/library/std/src/sys/unsupported/fs.rs @@ -214,7 +214,7 @@ impl File { self.0 } - pub fn read_buf(&self, _cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, _cursor: BorrowedCursor<'_>) -> io::Result<()> { self.0 } diff --git a/library/std/src/sys/wasi/fs.rs b/library/std/src/sys/wasi/fs.rs index b5b5eab1a24ec..510cf36b1bf48 100644 --- a/library/std/src/sys/wasi/fs.rs +++ b/library/std/src/sys/wasi/fs.rs @@ -439,7 +439,7 @@ impl File { true } - pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, cursor: BorrowedCursor<'_>) -> io::Result<()> { crate::io::default_read_buf(|buf| self.read(buf), cursor) } diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index bfc2477dff46b..9ac7cfebbebe0 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -415,7 +415,7 @@ impl File { self.handle.read_at(buf, offset) } - pub fn read_buf(&self, cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, cursor: BorrowedCursor<'_>) -> io::Result<()> { self.handle.read_buf(cursor) } diff --git a/library/std/src/sys/windows/handle.rs b/library/std/src/sys/windows/handle.rs index 0ea6876af5b93..ae33d48c612ee 100644 --- a/library/std/src/sys/windows/handle.rs +++ b/library/std/src/sys/windows/handle.rs @@ -112,7 +112,7 @@ impl Handle { } } - pub fn read_buf(&self, mut cursor: BorrowedCursor<'_, '_>) -> io::Result<()> { + pub fn read_buf(&self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> { let res = unsafe { self.synchronous_read(cursor.as_mut().as_mut_ptr(), cursor.capacity(), None) }; From 85b3df2630efc9179bbb027904fdc3c8912260ae Mon Sep 17 00:00:00 2001 From: Mohsen Zohrevandi Date: Mon, 22 Aug 2022 08:54:50 -0700 Subject: [PATCH 08/19] Export Cancel from std::os::fortanix_sgx::usercalls::raw This was missed in https://github.com/rust-lang/rust/pull/100642 --- library/std/src/os/fortanix_sgx/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/std/src/os/fortanix_sgx/mod.rs b/library/std/src/os/fortanix_sgx/mod.rs index da100b689db84..39a42f4e17fec 100644 --- a/library/std/src/os/fortanix_sgx/mod.rs +++ b/library/std/src/os/fortanix_sgx/mod.rs @@ -30,7 +30,9 @@ pub mod usercalls { // fortanix-sgx-abi re-exports pub use crate::sys::abi::usercalls::raw::Error; - pub use crate::sys::abi::usercalls::raw::{ByteBuffer, FifoDescriptor, Return, Usercall}; + pub use crate::sys::abi::usercalls::raw::{ + ByteBuffer, Cancel, FifoDescriptor, Return, Usercall, + }; pub use crate::sys::abi::usercalls::raw::{Fd, Result, Tcs}; pub use crate::sys::abi::usercalls::raw::{ EV_RETURNQ_NOT_EMPTY, EV_UNPARK, EV_USERCALLQ_NOT_FULL, FD_STDERR, FD_STDIN, FD_STDOUT, From 2a26987b36656a16ee6e9d65e972ca36b9a99449 Mon Sep 17 00:00:00 2001 From: Artem Mukhin Date: Mon, 20 Jun 2022 19:07:10 +0200 Subject: [PATCH 09/19] Add GDB/LLDB pretty-printers for NonZero types --- src/etc/gdb_lookup.py | 3 + src/etc/gdb_providers.py | 11 ++++ src/etc/lldb_commands | 1 + src/etc/lldb_lookup.py | 3 + src/etc/lldb_providers.py | 8 +++ src/etc/rust_types.py | 3 + src/test/debuginfo/numeric-types.rs | 87 +++++++++++++++++++++++++++- src/tools/compiletest/src/runtest.rs | 1 + 8 files changed, 116 insertions(+), 1 deletion(-) diff --git a/src/etc/gdb_lookup.py b/src/etc/gdb_lookup.py index 292e91b4d5d61..8171cb4e9a68a 100644 --- a/src/etc/gdb_lookup.py +++ b/src/etc/gdb_lookup.py @@ -89,4 +89,7 @@ def lookup(valobj): if rust_type == RustType.STD_REF_CELL: return StdRefCellProvider(valobj) + if rust_type == RustType.STD_NONZERO_NUMBER: + return StdNonZeroNumberProvider(valobj) + return None diff --git a/src/etc/gdb_providers.py b/src/etc/gdb_providers.py index 0a52b8c976f6a..c351c3450f582 100644 --- a/src/etc/gdb_providers.py +++ b/src/etc/gdb_providers.py @@ -231,6 +231,17 @@ def children(self): yield "borrow", self.borrow +class StdNonZeroNumberProvider: + def __init__(self, valobj): + fields = valobj.type.fields() + assert len(fields) == 1 + field = list(fields)[0] + self.value = str(valobj[field.name]) + + def to_string(self): + return self.value + + # Yields children (in a provider's sense of the word) for a BTreeMap. def children_of_btree_map(map): # Yields each key/value pair in the node and in any child nodes. diff --git a/src/etc/lldb_commands b/src/etc/lldb_commands index 4a1204ccc4be7..ed66ecf30729e 100644 --- a/src/etc/lldb_commands +++ b/src/etc/lldb_commands @@ -15,4 +15,5 @@ type summary add -F lldb_lookup.summary_lookup -e -x -h "^(core::([a-z_]+::)+)C type summary add -F lldb_lookup.summary_lookup -e -x -h "^(core::([a-z_]+::)+)Ref<.+>$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^(core::([a-z_]+::)+)RefMut<.+>$" --category Rust type summary add -F lldb_lookup.summary_lookup -e -x -h "^(core::([a-z_]+::)+)RefCell<.+>$" --category Rust +type summary add -F lldb_lookup.summary_lookup -e -x -h "^core::num::([a-z_]+::)*NonZero.+$" --category Rust type category enable Rust diff --git a/src/etc/lldb_lookup.py b/src/etc/lldb_lookup.py index 3cee51982ba9f..bca9c2ae192a7 100644 --- a/src/etc/lldb_lookup.py +++ b/src/etc/lldb_lookup.py @@ -55,6 +55,9 @@ def summary_lookup(valobj, dict): if rust_type == RustType.STD_REF_CELL: return StdRefSummaryProvider(valobj, dict) + if rust_type == RustType.STD_NONZERO_NUMBER: + return StdNonZeroNumberSummaryProvider(valobj, dict) + return "" diff --git a/src/etc/lldb_providers.py b/src/etc/lldb_providers.py index 35ac07f0db763..8a9927e7d96d7 100644 --- a/src/etc/lldb_providers.py +++ b/src/etc/lldb_providers.py @@ -739,3 +739,11 @@ def update(self): def has_children(self): # type: () -> bool return True + + +def StdNonZeroNumberSummaryProvider(valobj, _dict): + # type: (SBValue, dict) -> str + objtype = valobj.GetType() + field = objtype.GetFieldAtIndex(0) + element = valobj.GetChildMemberWithName(field.name) + return element.GetValue() diff --git a/src/etc/rust_types.py b/src/etc/rust_types.py index bbc945a7ddab0..bf512bc99b8f2 100644 --- a/src/etc/rust_types.py +++ b/src/etc/rust_types.py @@ -31,6 +31,7 @@ class RustType(object): STD_REF = "StdRef" STD_REF_MUT = "StdRefMut" STD_REF_CELL = "StdRefCell" + STD_NONZERO_NUMBER = "StdNonZeroNumber" STD_STRING_REGEX = re.compile(r"^(alloc::(\w+::)+)String$") @@ -49,6 +50,7 @@ class RustType(object): STD_REF_REGEX = re.compile(r"^(core::(\w+::)+)Ref<.+>$") STD_REF_MUT_REGEX = re.compile(r"^(core::(\w+::)+)RefMut<.+>$") STD_REF_CELL_REGEX = re.compile(r"^(core::(\w+::)+)RefCell<.+>$") +STD_NONZERO_NUMBER_REGEX = re.compile(r"^core::num::([a-z_]+::)*NonZero.+$") TUPLE_ITEM_REGEX = re.compile(r"__\d+$") @@ -72,6 +74,7 @@ class RustType(object): RustType.STD_REF_MUT: STD_REF_MUT_REGEX, RustType.STD_REF_CELL: STD_REF_CELL_REGEX, RustType.STD_CELL: STD_CELL_REGEX, + RustType.STD_NONZERO_NUMBER: STD_NONZERO_NUMBER_REGEX, } def is_tuple_fields(fields): diff --git a/src/test/debuginfo/numeric-types.rs b/src/test/debuginfo/numeric-types.rs index 2eae9239b6118..c41c9ee21df89 100644 --- a/src/test/debuginfo/numeric-types.rs +++ b/src/test/debuginfo/numeric-types.rs @@ -1,6 +1,7 @@ -// only-cdb // compile-flags:-g +// min-gdb-version: 8.1 + // Tests the visualizations for `NonZero{I,U}{8,16,32,64,128,size}`, `Wrapping` and // `Atomic{Bool,I8,I16,I32,I64,Isize,U8,U16,U32,U64,Usize}` located in `libcore.natvis`. @@ -153,6 +154,90 @@ // cdb-check:a_usize : 0x400 [Type: core::sync::atomic::AtomicUsize] // cdb-check: [] [Type: core::sync::atomic::AtomicUsize] + +// === GDB TESTS =================================================================================== + +// gdb-command:run + +// gdb-command:print/d nz_i8 +// gdb-check:[...]$1 = 11 + +// gdb-command:print nz_i16 +// gdb-check:[...]$2 = 22 + +// gdb-command:print nz_i32 +// gdb-check:[...]$3 = 33 + +// gdb-command:print nz_i64 +// gdb-check:[...]$4 = 44 + +// gdb-command:print nz_i128 +// gdb-check:[...]$5 = 55 + +// gdb-command:print nz_isize +// gdb-check:[...]$6 = 66 + +// gdb-command:print/d nz_u8 +// gdb-check:[...]$7 = 77 + +// gdb-command:print nz_u16 +// gdb-check:[...]$8 = 88 + +// gdb-command:print nz_u32 +// gdb-check:[...]$9 = 99 + +// gdb-command:print nz_u64 +// gdb-check:[...]$10 = 100 + +// gdb-command:print nz_u128 +// gdb-check:[...]$11 = 111 + +// gdb-command:print nz_usize +// gdb-check:[...]$12 = 122 + + + +// === LLDB TESTS ================================================================================== + +// lldb-command:run + +// lldb-command:print/d nz_i8 +// lldb-check:[...]$0 = 11 { __0 = 11 } + +// lldb-command:print nz_i16 +// lldb-check:[...]$1 = 22 { __0 = 22 } + +// lldb-command:print nz_i32 +// lldb-check:[...]$2 = 33 { __0 = 33 } + +// lldb-command:print nz_i64 +// lldb-check:[...]$3 = 44 { __0 = 44 } + +// lldb-command:print nz_i128 +// lldb-check:[...]$4 = 55 { __0 = 55 } + +// lldb-command:print nz_isize +// lldb-check:[...]$5 = 66 { __0 = 66 } + +// lldb-command:print/d nz_u8 +// lldb-check:[...]$6 = 77 { __0 = 77 } + +// lldb-command:print nz_u16 +// lldb-check:[...]$7 = 88 { __0 = 88 } + +// lldb-command:print nz_u32 +// lldb-check:[...]$8 = 99 { __0 = 99 } + +// lldb-command:print nz_u64 +// lldb-check:[...]$9 = 100 { __0 = 100 } + +// lldb-command:print nz_u128 +// lldb-check:[...]$10 = 111 { __0 = 111 } + +// lldb-command:print nz_usize +// lldb-check:[...]$11 = 122 { __0 = 122 } + + use std::num::*; use std::sync::atomic::*; diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index b758bb9cf6790..70da954a54827 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1094,6 +1094,7 @@ impl<'test> TestCx<'test> { "^(core::([a-z_]+::)+)Ref<.+>$", "^(core::([a-z_]+::)+)RefMut<.+>$", "^(core::([a-z_]+::)+)RefCell<.+>$", + "^core::num::([a-z_]+::)*NonZero.+$", ]; script_str From 80442f375aff4939afbf2b398cd2a663231a72b9 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Wed, 24 Aug 2022 15:14:51 +0100 Subject: [PATCH 10/19] error::Error: rename the chain method to sources Signed-off-by: Nick Cameron --- library/core/src/error.rs | 29 ++++++++++++++++++++------- library/std/src/error.rs | 41 ++++++++++++++++++++++++++------------- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/library/core/src/error.rs b/library/core/src/error.rs index d11debb34adca..37bd19c6f03c4 100644 --- a/library/core/src/error.rs +++ b/library/core/src/error.rs @@ -30,12 +30,12 @@ use crate::fmt::{Debug, Display}; /// assert_eq!(err.to_string(), "invalid digit found in string"); /// ``` /// -/// Errors may provide cause chain information. [`Error::source()`] is generally +/// Errors may provide cause information. [`Error::source()`] is generally /// used when errors cross "abstraction boundaries". If one module must report /// an error that is caused by an error from a lower-level module, it can allow /// accessing that error via [`Error::source()`]. This makes it possible for the /// high-level module to provide its own errors while also revealing some of the -/// implementation for debugging via `source` chains. +/// implementation for debugging. #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "Error")] #[rustc_has_incoherent_inherent_impls] @@ -397,7 +397,7 @@ impl dyn Error { /// // let err : Box = b.into(); // or /// let err = &b as &(dyn Error); /// - /// let mut iter = err.chain(); + /// let mut iter = err.sources(); /// /// assert_eq!("B".to_string(), iter.next().unwrap().to_string()); /// assert_eq!("A".to_string(), iter.next().unwrap().to_string()); @@ -406,8 +406,23 @@ impl dyn Error { /// ``` #[unstable(feature = "error_iter", issue = "58520")] #[inline] - pub fn chain(&self) -> Chain<'_> { - Chain { current: Some(self) } + pub fn sources(&self) -> Source<'_> { + // You may think this method would be better in the Error trait, and you'd be right. + // Unfortunately that doesn't work, not because of the object safety rules but because we + // save a reference to self in Sources below as a trait object. If this method was + // declared in Error, then self would have the type &T where T is some concrete type which + // implements Error. We would need to coerce self to have type &dyn Error, but that requires + // that Self has a known size (i.e., Self: Sized). We can't put that bound on Error + // since that would forbid Error trait objects, and we can't put that bound on the method + // because that means the method can't be called on trait objects (we'd also need the + // 'static bound, but that isn't allowed because methods with bounds on Self other than + // Sized are not object-safe). Requiring an Unsize bound is not backwards compatible. + // + // Two possible solutions are to start the iterator at self.source() instead of self (see + // discussion on the tracking issue), or to wait for dyn* to exist (which would then permit + // the coercion). + + Source { current: Some(self) } } } @@ -417,12 +432,12 @@ impl dyn Error { /// its sources, use `skip(1)`. #[unstable(feature = "error_iter", issue = "58520")] #[derive(Clone, Debug)] -pub struct Chain<'a> { +pub struct Source<'a> { current: Option<&'a (dyn Error + 'static)>, } #[unstable(feature = "error_iter", issue = "58520")] -impl<'a> Iterator for Chain<'a> { +impl<'a> Iterator for Source<'a> { type Item = &'a (dyn Error + 'static); fn next(&mut self) -> Option { diff --git a/library/std/src/error.rs b/library/std/src/error.rs index 914f6d6d2e3e9..70eeec557b1cb 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -69,12 +69,12 @@ pub use core::error::Error; /// assert_eq!(err.to_string(), "invalid digit found in string"); /// ``` /// -/// Errors may provide cause chain information. [`Error::source()`] is generally +/// Errors may provide cause information. [`Error::source()`] is generally /// used when errors cross "abstraction boundaries". If one module must report /// an error that is caused by an error from a lower-level module, it can allow /// accessing that error via [`Error::source()`]. This makes it possible for the /// high-level module to provide its own errors while also revealing some of the -/// implementation for debugging via `source` chains. +/// implementation for debugging. #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "Error")] #[cfg(bootstrap)] @@ -976,7 +976,7 @@ impl dyn Error { /// // let err : Box = b.into(); // or /// let err = &b as &(dyn Error); /// - /// let mut iter = err.chain(); + /// let mut iter = err.sources(); /// /// assert_eq!("B".to_string(), iter.next().unwrap().to_string()); /// assert_eq!("A".to_string(), iter.next().unwrap().to_string()); @@ -985,8 +985,23 @@ impl dyn Error { /// ``` #[unstable(feature = "error_iter", issue = "58520")] #[inline] - pub fn chain(&self) -> Chain<'_> { - Chain { current: Some(self) } + pub fn sources(&self) -> Sources<'_> { + // You may think this method would be better in the Error trait, and you'd be right. + // Unfortunately that doesn't work, not because of the object safety rules but because we + // save a reference to self in Sources below as a trait object. If this method was + // declared in Error, then self would have the type &T where T is some concrete type which + // implements Error. We would need to coerce self to have type &dyn Error, but that requires + // that Self has a known size (i.e., Self: Sized). We can't put that bound on Error + // since that would forbid Error trait objects, and we can't put that bound on the method + // because that means the method can't be called on trait objects (we'd also need the + // 'static bound, but that isn't allowed because methods with bounds on Self other than + // Sized are not object-safe). Requiring an Unsize bound is not backwards compatible. + // + // Two possible solutions are to start the iterator at self.source() instead of self (see + // discussion on the tracking issue), or to wait for dyn* to exist (which would then permit + // the coercion). + + Sources { current: Some(self) } } } @@ -997,13 +1012,13 @@ impl dyn Error { #[unstable(feature = "error_iter", issue = "58520")] #[derive(Clone, Debug)] #[cfg(bootstrap)] -pub struct Chain<'a> { +pub struct Sources<'a> { current: Option<&'a (dyn Error + 'static)>, } #[cfg(bootstrap)] #[unstable(feature = "error_iter", issue = "58520")] -impl<'a> Iterator for Chain<'a> { +impl<'a> Iterator for Sources<'a> { type Item = &'a (dyn Error + 'static); fn next(&mut self) -> Option { @@ -1043,8 +1058,8 @@ impl dyn Error + Send + Sync { /// An error reporter that prints an error and its sources. /// -/// Report also exposes configuration options for formatting the error chain, either entirely on a -/// single line, or in multi-line format with each cause in the error chain on a new line. +/// Report also exposes configuration options for formatting the error sources, either entirely on a +/// single line, or in multi-line format with each source on a new line. /// /// `Report` only requires that the wrapped error implement `Error`. It doesn't require that the /// wrapped error be `Send`, `Sync`, or `'static`. @@ -1389,7 +1404,7 @@ impl Report { /// /// **Note**: Report will search for the first `Backtrace` it can find starting from the /// outermost error. In this example it will display the backtrace from the second error in the - /// chain, `SuperErrorSideKick`. + /// sources, `SuperErrorSideKick`. /// /// ```rust /// #![feature(error_reporter)] @@ -1486,7 +1501,7 @@ where let backtrace = backtrace.or_else(|| { self.error .source() - .map(|source| source.chain().find_map(|source| source.request_ref())) + .map(|source| source.sources().find_map(|source| source.request_ref())) .flatten() }); backtrace @@ -1497,7 +1512,7 @@ where fn fmt_singleline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.error)?; - let sources = self.error.source().into_iter().flat_map(::chain); + let sources = self.error.source().into_iter().flat_map(::sources); for cause in sources { write!(f, ": {cause}")?; @@ -1518,7 +1533,7 @@ where let multiple = cause.source().is_some(); - for (ind, error) in cause.chain().enumerate() { + for (ind, error) in cause.sources().enumerate() { writeln!(f)?; let mut indented = Indented { inner: f }; if multiple { From b556a5be5a0425b453495e2eb636aca4faa37720 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Wed, 24 Aug 2022 15:17:17 +0100 Subject: [PATCH 11/19] error::Error: rename the Demand arguments from req to demand Signed-off-by: Nick Cameron --- library/core/src/error.rs | 14 +++++++------- library/std/src/error.rs | 23 +++++++++++------------ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/library/core/src/error.rs b/library/core/src/error.rs index 37bd19c6f03c4..a4ed2868c9109 100644 --- a/library/core/src/error.rs +++ b/library/core/src/error.rs @@ -182,8 +182,8 @@ pub trait Error: Debug + Display { /// } /// /// impl std::error::Error for Error { - /// fn provide<'a>(&'a self, req: &mut Demand<'a>) { - /// req + /// fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + /// demand /// .provide_ref::(&self.backtrace) /// .provide_ref::(&self.source); /// } @@ -201,7 +201,7 @@ pub trait Error: Debug + Display { /// ``` #[unstable(feature = "error_generic_member_access", issue = "99301")] #[allow(unused_variables)] - fn provide<'a>(&'a self, req: &mut Demand<'a>) {} + fn provide<'a>(&'a self, demand: &mut Demand<'a>) {} } #[unstable(feature = "error_generic_member_access", issue = "99301")] @@ -209,8 +209,8 @@ impl Provider for E where E: Error + ?Sized, { - fn provide<'a>(&'a self, req: &mut Demand<'a>) { - self.provide(req) + fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + self.provide(demand) } } @@ -463,8 +463,8 @@ impl<'a, T: Error + ?Sized> Error for &'a T { Error::source(&**self) } - fn provide<'b>(&'b self, req: &mut Demand<'b>) { - Error::provide(&**self, req); + fn provide<'b>(&'b self, demand: &mut Demand<'b>) { + Error::provide(&**self, demand); } } diff --git a/library/std/src/error.rs b/library/std/src/error.rs index 70eeec557b1cb..f36765c07078c 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -221,8 +221,8 @@ pub trait Error: Debug + Display { /// } /// /// impl std::error::Error for Error { - /// fn provide<'a>(&'a self, req: &mut Demand<'a>) { - /// req + /// fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + /// demand /// .provide_ref::(&self.backtrace) /// .provide_ref::(&self.source); /// } @@ -240,14 +240,14 @@ pub trait Error: Debug + Display { /// ``` #[unstable(feature = "error_generic_member_access", issue = "99301")] #[allow(unused_variables)] - fn provide<'a>(&'a self, req: &mut Demand<'a>) {} + fn provide<'a>(&'a self, demand: &mut Demand<'a>) {} } #[cfg(bootstrap)] #[unstable(feature = "error_generic_member_access", issue = "99301")] impl<'b> Provider for dyn Error + 'b { - fn provide<'a>(&'a self, req: &mut Demand<'a>) { - self.provide(req) + fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + self.provide(demand) } } @@ -659,8 +659,8 @@ impl<'a, T: Error + ?Sized> Error for &'a T { Error::source(&**self) } - fn provide<'b>(&'b self, req: &mut Demand<'b>) { - Error::provide(&**self, req); + fn provide<'b>(&'b self, demand: &mut Demand<'b>) { + Error::provide(&**self, demand); } } @@ -681,8 +681,8 @@ impl Error for Arc { Error::source(&**self) } - fn provide<'a>(&'a self, req: &mut Demand<'a>) { - Error::provide(&**self, req); + fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + Error::provide(&**self, demand); } } @@ -1442,9 +1442,8 @@ impl Report { /// } /// /// impl Error for SuperErrorSideKick { - /// fn provide<'a>(&'a self, req: &mut Demand<'a>) { - /// req - /// .provide_ref::(&self.backtrace); + /// fn provide<'a>(&'a self, demand: &mut Demand<'a>) { + /// demand.provide_ref::(&self.backtrace); /// } /// } /// From 9372c4f6acddce4ec3077a2f8e170549f35b6282 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Thu, 25 Aug 2022 07:42:07 +0100 Subject: [PATCH 12/19] error::Error: remove some comments Signed-off-by: Nick Cameron --- library/core/src/error.rs | 15 --------------- library/std/src/error.rs | 15 --------------- 2 files changed, 30 deletions(-) diff --git a/library/core/src/error.rs b/library/core/src/error.rs index a4ed2868c9109..4a8efe15e596b 100644 --- a/library/core/src/error.rs +++ b/library/core/src/error.rs @@ -1,17 +1,6 @@ #![doc = include_str!("error.md")] #![unstable(feature = "error_in_core", issue = "none")] -// A note about crates and the facade: -// -// Originally, the `Error` trait was defined in libcore, and the impls -// were scattered about. However, coherence objected to this -// arrangement, because to create the blanket impls for `Box` required -// knowing that `&str: !Error`, and we have no means to deal with that -// sort of conflict just now. Therefore, for the time being, we have -// moved the `Error` trait into libstd. As we evolve a sol'n to the -// coherence challenge (e.g., specialization, neg impls, etc) we can -// reconsider what crate these items belong in. - #[cfg(test)] mod tests; @@ -417,10 +406,6 @@ impl dyn Error { // because that means the method can't be called on trait objects (we'd also need the // 'static bound, but that isn't allowed because methods with bounds on Self other than // Sized are not object-safe). Requiring an Unsize bound is not backwards compatible. - // - // Two possible solutions are to start the iterator at self.source() instead of self (see - // discussion on the tracking issue), or to wait for dyn* to exist (which would then permit - // the coercion). Source { current: Some(self) } } diff --git a/library/std/src/error.rs b/library/std/src/error.rs index f36765c07078c..e45059595362f 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -1,17 +1,6 @@ #![doc = include_str!("../../core/src/error.md")] #![stable(feature = "rust1", since = "1.0.0")] -// A note about crates and the facade: -// -// Originally, the `Error` trait was defined in libcore, and the impls -// were scattered about. However, coherence objected to this -// arrangement, because to create the blanket impls for `Box` required -// knowing that `&str: !Error`, and we have no means to deal with that -// sort of conflict just now. Therefore, for the time being, we have -// moved the `Error` trait into libstd. As we evolve a sol'n to the -// coherence challenge (e.g., specialization, neg impls, etc) we can -// reconsider what crate these items belong in. - #[cfg(test)] mod tests; @@ -996,10 +985,6 @@ impl dyn Error { // because that means the method can't be called on trait objects (we'd also need the // 'static bound, but that isn't allowed because methods with bounds on Self other than // Sized are not object-safe). Requiring an Unsize bound is not backwards compatible. - // - // Two possible solutions are to start the iterator at self.source() instead of self (see - // discussion on the tracking issue), or to wait for dyn* to exist (which would then permit - // the coercion). Sources { current: Some(self) } } From 752902957bb6cfcdb17c0a9119763f4b8a8bb147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 25 Aug 2022 06:36:33 -0700 Subject: [PATCH 13/19] Provide structured suggestion for `hashmap[idx] = val` --- .../src/diagnostics/mutability_errors.rs | 116 +++++++++++++++++- src/test/ui/borrowck/index-mut-help.rs | 3 +- src/test/ui/borrowck/index-mut-help.stderr | 20 ++- .../ui/btreemap/btreemap-index-mut.stderr | 9 +- src/test/ui/hashmap/hashmap-index-mut.stderr | 9 +- src/test/ui/issues/issue-41726.stderr | 5 +- 6 files changed, 149 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 56721cc3f5c75..dd9590016b990 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1,5 +1,8 @@ -use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed}; +use rustc_errors::{ + Applicability, Diagnostic, DiagnosticBuilder, EmissionGuarantee, ErrorGuaranteed, +}; use rustc_hir as hir; +use rustc_hir::intravisit::Visitor; use rustc_hir::Node; use rustc_middle::hir::map::Map; use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem}; @@ -614,7 +617,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { "trait `IndexMut` is required to modify indexed content, \ but it is not implemented for `{ty}`", )); - self.suggest_map_index_mut_alternatives(ty, &mut err); + self.suggest_map_index_mut_alternatives(ty, &mut err, span); } _ => (), } @@ -632,13 +635,120 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { &self, ty: Ty<'_>, err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>, + span: Span, ) { let Some(adt) = ty.ty_adt_def() else { return }; let did = adt.did(); if self.infcx.tcx.is_diagnostic_item(sym::HashMap, did) || self.infcx.tcx.is_diagnostic_item(sym::BTreeMap, did) { - err.help(format!("to modify a `{ty}`, use `.get_mut()`, `.insert()` or the entry API")); + struct V<'a, 'b, 'tcx, G: EmissionGuarantee> { + assign_span: Span, + err: &'a mut DiagnosticBuilder<'b, G>, + ty: Ty<'tcx>, + suggested: bool, + } + impl<'a, 'b: 'a, 'hir, 'tcx, G: EmissionGuarantee> Visitor<'hir> for V<'a, 'b, 'tcx, G> { + fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'hir>) { + hir::intravisit::walk_stmt(self, stmt); + let expr = match stmt.kind { + hir::StmtKind::Semi(expr) | hir::StmtKind::Expr(expr) => expr, + hir::StmtKind::Local(hir::Local { init: Some(expr), .. }) => expr, + _ => { + return; + } + }; + if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind + && let hir::ExprKind::Index(val, index) = place.kind + && (expr.span == self.assign_span || place.span == self.assign_span) + { + // val[index] = rv; + // ---------- place + self.err.multipart_suggestions( + &format!( + "to modify a `{}`, use `.get_mut()`, `.insert()` or the entry API", + self.ty, + ), + vec![ + vec![ // val.insert(index, rv); + ( + val.span.shrink_to_hi().with_hi(index.span.lo()), + ".insert(".to_string(), + ), + ( + index.span.shrink_to_hi().with_hi(rv.span.lo()), + ", ".to_string(), + ), + (rv.span.shrink_to_hi(), ")".to_string()), + ], + vec![ // val.get_mut(index).map(|v| { *v = rv; }); + ( + val.span.shrink_to_hi().with_hi(index.span.lo()), + ".get_mut(".to_string(), + ), + ( + index.span.shrink_to_hi().with_hi(place.span.hi()), + ").map(|val| { *val".to_string(), + ), + ( + rv.span.shrink_to_hi(), + "; })".to_string(), + ), + ], + vec![ // let x = val.entry(index).or_insert(rv); + (val.span.shrink_to_lo(), "let val = ".to_string()), + ( + val.span.shrink_to_hi().with_hi(index.span.lo()), + ".entry(".to_string(), + ), + ( + index.span.shrink_to_hi().with_hi(rv.span.lo()), + ").or_insert(".to_string(), + ), + (rv.span.shrink_to_hi(), ")".to_string()), + ], + ].into_iter(), + Applicability::MachineApplicable, + ); + self.suggested = true; + } else if let hir::ExprKind::MethodCall(_path, args @ [_, ..], sp) = expr.kind + && let hir::ExprKind::Index(val, index) = args[0].kind + && expr.span == self.assign_span + { + // val[index].path(args..); + self.err.multipart_suggestion( + &format!("to modify a `{}` use `.get_mut()`", self.ty), + vec![ + ( + val.span.shrink_to_hi().with_hi(index.span.lo()), + ".get_mut(".to_string(), + ), + ( + index.span.shrink_to_hi().with_hi(args[0].span.hi()), + ").map(|val| val".to_string(), + ), + (sp.shrink_to_hi(), ")".to_string()), + ], + Applicability::MachineApplicable, + ); + self.suggested = true; + } + } + } + let hir_map = self.infcx.tcx.hir(); + let def_id = self.body.source.def_id(); + let hir_id = hir_map.local_def_id_to_hir_id(def_id.as_local().unwrap()); + let node = hir_map.find(hir_id); + let Some(hir::Node::Item(item)) = node else { return; }; + let hir::ItemKind::Fn(.., body_id) = item.kind else { return; }; + let body = self.infcx.tcx.hir().body(body_id); + let mut v = V { assign_span: span, err, ty, suggested: false }; + v.visit_body(body); + if !v.suggested { + err.help(&format!( + "to modify a `{ty}`, use `.get_mut()`, `.insert()` or the entry API", + )); + } } } diff --git a/src/test/ui/borrowck/index-mut-help.rs b/src/test/ui/borrowck/index-mut-help.rs index d57ef975d9634..35266e113a6de 100644 --- a/src/test/ui/borrowck/index-mut-help.rs +++ b/src/test/ui/borrowck/index-mut-help.rs @@ -1,10 +1,9 @@ // When mutably indexing a type that implements `Index` but not `IndexMut`, a // special 'help' message is added to the output. +use std::collections::HashMap; fn main() { - use std::collections::HashMap; - let mut map = HashMap::new(); map.insert("peter", "23".to_string()); diff --git a/src/test/ui/borrowck/index-mut-help.stderr b/src/test/ui/borrowck/index-mut-help.stderr index 0ce60e3eb1db8..f42d7e0155433 100644 --- a/src/test/ui/borrowck/index-mut-help.stderr +++ b/src/test/ui/borrowck/index-mut-help.stderr @@ -1,23 +1,33 @@ error[E0596]: cannot borrow data in an index of `HashMap<&str, String>` as mutable - --> $DIR/index-mut-help.rs:11:5 + --> $DIR/index-mut-help.rs:10:5 | LL | map["peter"].clear(); | ^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>` - = help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `HashMap<&str, String>` use `.get_mut()` + | +LL | map.get_mut("peter").map(|val| val.clear()); + | ~~~~~~~~~ ~~~~~~~~~~~~~~~ + error[E0594]: cannot assign to data in an index of `HashMap<&str, String>` - --> $DIR/index-mut-help.rs:12:5 + --> $DIR/index-mut-help.rs:11:5 | LL | map["peter"] = "0".to_string(); | ^^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>` - = help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API + | +LL | map.insert("peter", "0".to_string()); + | ~~~~~~~~ ~ + +LL | map.get_mut("peter").map(|val| { *val = "0".to_string(); }); + | ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++ +LL | let val = map.entry("peter").or_insert("0".to_string()); + | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + error[E0596]: cannot borrow data in an index of `HashMap<&str, String>` as mutable - --> $DIR/index-mut-help.rs:13:13 + --> $DIR/index-mut-help.rs:12:13 | LL | let _ = &mut map["peter"]; | ^^^^^^^^^^^^^^^^^ cannot borrow as mutable diff --git a/src/test/ui/btreemap/btreemap-index-mut.stderr b/src/test/ui/btreemap/btreemap-index-mut.stderr index 260f710007426..26f2a4c4b2914 100644 --- a/src/test/ui/btreemap/btreemap-index-mut.stderr +++ b/src/test/ui/btreemap/btreemap-index-mut.stderr @@ -5,7 +5,14 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap` - = help: to modify a `BTreeMap`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `BTreeMap`, use `.get_mut()`, `.insert()` or the entry API + | +LL | map.insert(&0, 1); + | ~~~~~~~~ ~ + +LL | map.get_mut(&0).map(|val| { *val = 1; }); + | ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++ +LL | let val = map.entry(&0).or_insert(1); + | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + error: aborting due to previous error diff --git a/src/test/ui/hashmap/hashmap-index-mut.stderr b/src/test/ui/hashmap/hashmap-index-mut.stderr index c72b380f4666d..c1948ab627149 100644 --- a/src/test/ui/hashmap/hashmap-index-mut.stderr +++ b/src/test/ui/hashmap/hashmap-index-mut.stderr @@ -5,7 +5,14 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap` - = help: to modify a `HashMap`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `HashMap`, use `.get_mut()`, `.insert()` or the entry API + | +LL | map.insert(&0, 1); + | ~~~~~~~~ ~ + +LL | map.get_mut(&0).map(|val| { *val = 1; }); + | ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++ +LL | let val = map.entry(&0).or_insert(1); + | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + error: aborting due to previous error diff --git a/src/test/ui/issues/issue-41726.stderr b/src/test/ui/issues/issue-41726.stderr index 9c70ab7d9711d..b05c1fb14efc6 100644 --- a/src/test/ui/issues/issue-41726.stderr +++ b/src/test/ui/issues/issue-41726.stderr @@ -5,7 +5,10 @@ LL | things[src.as_str()].sort(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap>` - = help: to modify a `HashMap>`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `HashMap>` use `.get_mut()` + | +LL | things.get_mut(src.as_str()).map(|val| val.sort()); + | ~~~~~~~~~ ~~~~~~~~~~~~~~~ + error: aborting due to previous error From b85178a5fccb0d582fdbf23625c531af170cc882 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 26 Aug 2022 08:08:10 -0400 Subject: [PATCH 14/19] no alignment check during interning --- compiler/rustc_const_eval/src/const_eval/eval_queries.rs | 2 ++ compiler/rustc_const_eval/src/const_eval/machine.rs | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 5cfa63bd105c4..371f2cfab1e0f 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -74,7 +74,9 @@ fn eval_body_using_ecx<'mir, 'tcx>( None => InternKind::Constant, } }; + ecx.machine.check_alignment = false; // interning doesn't need to respect alignment intern_const_alloc_recursive(ecx, intern_kind, &ret)?; + // we leave alignment checks off, since this `ecx` will not be used for further evaluation anyway debug!("eval_body_using_ecx done: {:?}", *ret); Ok(ret) diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index f24b19089c113..7ad312a123ac8 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -89,10 +89,10 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> { /// exhaustion error. /// /// Setting this to `0` disables the limit and allows the interpreter to run forever. - pub steps_remaining: usize, + pub(super) steps_remaining: usize, /// The virtual call stack. - pub(crate) stack: Vec>, + pub(super) stack: Vec>, /// We need to make sure consts never point to anything mutable, even recursively. That is /// relied on for pattern matching on consts with references. @@ -103,7 +103,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> { pub(super) can_access_statics: bool, /// Whether to check alignment during evaluation. - check_alignment: bool, + pub(super) check_alignment: bool, } impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> { From 468c617c2124b0b50eedab634656d213fc1e033d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 26 Aug 2022 11:04:13 -0400 Subject: [PATCH 15/19] add a test --- .../ui/consts/extra-const-ub/issue-101034.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/test/ui/consts/extra-const-ub/issue-101034.rs diff --git a/src/test/ui/consts/extra-const-ub/issue-101034.rs b/src/test/ui/consts/extra-const-ub/issue-101034.rs new file mode 100644 index 0000000000000..e0de705c48846 --- /dev/null +++ b/src/test/ui/consts/extra-const-ub/issue-101034.rs @@ -0,0 +1,17 @@ +// check-pass +// compile-flags: -Zextra-const-ub-checks + +#[repr(packed)] +pub struct Foo { + bar: u8, + baa: [u32; 1], +} + +const FOOMP: Foo = Foo { + bar: 0, + baa: [69; 1], +}; + +fn main() { + let _val = FOOMP; +} From b33c3d6cd68788f62b79814726004da45546bfa7 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sat, 27 Aug 2022 02:23:21 +0900 Subject: [PATCH 16/19] use smaller span for suggestions --- compiler/rustc_attr/src/session_diagnostics.rs | 14 ++++++-------- compiler/rustc_typeck/src/check/method/suggest.rs | 11 +++++------ .../wrong_number_of_generic_args.rs | 11 ++++------- .../cfg-attr-syntax-validation.stderr | 4 +++- .../ui/const-generics/issues/issue-87493.stderr | 8 +++++--- src/test/ui/deprecation/deprecation-sanity.stderr | 4 +++- src/test/ui/error-codes/E0107.stderr | 2 +- src/test/ui/error-codes/E0565-2.stderr | 4 +++- .../method-on-ambiguous-numeric-type.stderr | 8 ++++---- .../use-type-argument-instead-of-assoc-type.stderr | 2 +- 10 files changed, 35 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_attr/src/session_diagnostics.rs b/compiler/rustc_attr/src/session_diagnostics.rs index a75e7409fba18..25cd960dbf1d0 100644 --- a/compiler/rustc_attr/src/session_diagnostics.rs +++ b/compiler/rustc_attr/src/session_diagnostics.rs @@ -223,14 +223,12 @@ impl<'a> SessionDiagnostic<'a> for UnsupportedLiteral { error_code!(E0565), ); if self.is_bytestr { - if let Ok(lint_str) = sess.source_map().span_to_snippet(self.span) { - diag.span_suggestion( - self.span, - fluent::attr::unsupported_literal_suggestion, - &lint_str[1..], - Applicability::MaybeIncorrect, - ); - } + diag.span_suggestion( + sess.source_map().start_point(self.span), + fluent::attr::unsupported_literal_suggestion, + "", + Applicability::MaybeIncorrect, + ); } diag } diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 441a62256de88..8bb8c7ac51559 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -1299,7 +1299,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // local binding if let hir::def::Res::Local(hir_id) = path.res { let span = tcx.hir().span(hir_id); - let snippet = tcx.sess.source_map().span_to_snippet(span); let filename = tcx.sess.source_map().span_to_filename(span); let parent_node = @@ -1309,7 +1308,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { concrete_type, ); - match (filename, parent_node, snippet) { + match (filename, parent_node) { ( FileName::Real(_), Node::Local(hir::Local { @@ -1317,14 +1316,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ty, .. }), - Ok(ref snippet), ) => { + let type_span = ty.map(|ty| ty.span.with_lo(span.hi())).unwrap_or(span.shrink_to_hi()); err.span_suggestion( // account for `let x: _ = 42;` - // ^^^^ - span.to(ty.as_ref().map(|ty| ty.span).unwrap_or(span)), + // ^^^ + type_span, &msg, - format!("{}: {}", snippet, concrete_type), + format!(": {concrete_type}"), Applicability::MaybeIncorrect, ); } diff --git a/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs b/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs index 99729391e02b0..df14a966d1581 100644 --- a/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs +++ b/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs @@ -763,16 +763,13 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { // If there is a single unbound associated type and a single excess generic param // suggest replacing the generic param with the associated type bound if provided_args_matches_unbound_traits && !unbound_types.is_empty() { - let mut suggestions = vec![]; let unused_generics = &self.gen_args.args[self.num_expected_type_or_const_args()..]; - for (potential, name) in iter::zip(unused_generics, &unbound_types) { - if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(potential.span()) { - suggestions.push((potential.span(), format!("{} = {}", name, snippet))); - } - } + let suggestions = iter::zip(unused_generics, &unbound_types) + .map(|(potential, name)| (potential.span().shrink_to_lo(), format!("{name} = "))) + .collect::>(); if !suggestions.is_empty() { - err.multipart_suggestion( + err.multipart_suggestion_verbose( &format!( "replace the generic bound{s} with the associated type{s}", s = pluralize!(unbound_types.len()) diff --git a/src/test/ui/conditional-compilation/cfg-attr-syntax-validation.stderr b/src/test/ui/conditional-compilation/cfg-attr-syntax-validation.stderr index a057fd19b16b9..d4bd673b84e1b 100644 --- a/src/test/ui/conditional-compilation/cfg-attr-syntax-validation.stderr +++ b/src/test/ui/conditional-compilation/cfg-attr-syntax-validation.stderr @@ -50,7 +50,9 @@ error[E0565]: literal in `cfg` predicate value must be a string --> $DIR/cfg-attr-syntax-validation.rs:25:11 | LL | #[cfg(a = b"hi")] - | ^^^^^ help: consider removing the prefix: `"hi"` + | -^^^^ + | | + | help: consider removing the prefix error: expected unsuffixed literal or identifier, found `concat!("nonexistent")` --> $DIR/cfg-attr-syntax-validation.rs:30:25 diff --git a/src/test/ui/const-generics/issues/issue-87493.stderr b/src/test/ui/const-generics/issues/issue-87493.stderr index f998c1187d810..653afae219115 100644 --- a/src/test/ui/const-generics/issues/issue-87493.stderr +++ b/src/test/ui/const-generics/issues/issue-87493.stderr @@ -13,15 +13,17 @@ error[E0107]: this trait takes 0 generic arguments but 1 generic argument was su --> $DIR/issue-87493.rs:8:8 | LL | T: MyTrait, - | ^^^^^^^ ----------------- help: replace the generic bound with the associated type: `Assoc = Assoc == S::Assoc` - | | - | expected 0 generic arguments + | ^^^^^^^ expected 0 generic arguments | note: trait defined here, with 0 generic parameters --> $DIR/issue-87493.rs:1:11 | LL | pub trait MyTrait { | ^^^^^^^ +help: replace the generic bound with the associated type + | +LL | T: MyTrait, + | +++++++ error: aborting due to 2 previous errors diff --git a/src/test/ui/deprecation/deprecation-sanity.stderr b/src/test/ui/deprecation/deprecation-sanity.stderr index 973c672df91c3..8b2b480d19586 100644 --- a/src/test/ui/deprecation/deprecation-sanity.stderr +++ b/src/test/ui/deprecation/deprecation-sanity.stderr @@ -44,7 +44,9 @@ error[E0565]: literal in `deprecated` value must be a string --> $DIR/deprecation-sanity.rs:19:25 | LL | #[deprecated(note = b"test")] - | ^^^^^^^ help: consider removing the prefix: `"test"` + | -^^^^^^ + | | + | help: consider removing the prefix error[E0565]: item in `deprecated` must be a key/value pair --> $DIR/deprecation-sanity.rs:22:18 diff --git a/src/test/ui/error-codes/E0107.stderr b/src/test/ui/error-codes/E0107.stderr index 5ca03b45d82b3..03430f8fa3a0b 100644 --- a/src/test/ui/error-codes/E0107.stderr +++ b/src/test/ui/error-codes/E0107.stderr @@ -142,7 +142,7 @@ LL | pub trait T { help: replace the generic bounds with the associated types | LL | fn trait_bound_generic>(_i: I) { - | ~~~~~~ ~~~~~~~ + | +++ +++ error: aborting due to 10 previous errors diff --git a/src/test/ui/error-codes/E0565-2.stderr b/src/test/ui/error-codes/E0565-2.stderr index bb30bd7befb38..097871bd31906 100644 --- a/src/test/ui/error-codes/E0565-2.stderr +++ b/src/test/ui/error-codes/E0565-2.stderr @@ -2,7 +2,9 @@ error[E0565]: literal in `deprecated` value must be a string --> $DIR/E0565-2.rs:2:22 | LL | #[deprecated(since = b"1.29", note = "hi")] - | ^^^^^^^ help: consider removing the prefix: `"1.29"` + | -^^^^^^ + | | + | help: consider removing the prefix error: aborting due to previous error diff --git a/src/test/ui/methods/method-on-ambiguous-numeric-type.stderr b/src/test/ui/methods/method-on-ambiguous-numeric-type.stderr index 0af58bc61f49e..91733411637d0 100644 --- a/src/test/ui/methods/method-on-ambiguous-numeric-type.stderr +++ b/src/test/ui/methods/method-on-ambiguous-numeric-type.stderr @@ -18,7 +18,7 @@ LL | let x = y.neg(); help: you must specify a type for this binding, like `f32` | LL | let y: f32 = 2.0; - | ~~~~~~ + | +++++ error[E0689]: can't call method `pow` on ambiguous numeric type `{integer}` --> $DIR/method-on-ambiguous-numeric-type.rs:19:26 @@ -37,7 +37,7 @@ LL | local_bar.pow(2); help: you must specify a type for this binding, like `i32` | LL | ($ident:ident) => { let $ident: i32 = 42; } - | ~~~~~~~~~~~ + | +++++ error[E0689]: can't call method `pow` on ambiguous numeric type `{integer}` --> $DIR/method-on-ambiguous-numeric-type.rs:30:9 @@ -46,10 +46,10 @@ LL | bar.pow(2); | ^^^ | help: you must specify a type for this binding, like `i32` - --> $DIR/auxiliary/macro-in-other-crate.rs:3:29 + --> $DIR/auxiliary/macro-in-other-crate.rs:3:35 | LL | ($ident:ident) => { let $ident: i32 = 42; } - | ~~~~~~~~~~~ + | +++++ error: aborting due to 5 previous errors diff --git a/src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.stderr b/src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.stderr index 7038a572bec07..75b9192328478 100644 --- a/src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.stderr +++ b/src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.stderr @@ -12,7 +12,7 @@ LL | pub trait T { help: replace the generic bounds with the associated types | LL | i: Box>, - | ~~~~~~~~~ ~~~~~~~~~ + | +++ +++ error[E0191]: the value of the associated types `A` (from trait `T`), `C` (from trait `T`) must be specified --> $DIR/use-type-argument-instead-of-assoc-type.rs:7:16 From aa76e135ff5777b32a20cdf9407564322b4b422d Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Sun, 28 Aug 2022 01:20:26 +0900 Subject: [PATCH 17/19] extend attrs if local_def_id exists --- src/librustdoc/passes/propagate_doc_cfg.rs | 5 +++-- src/test/rustdoc-ui/issue-101076.rs | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 src/test/rustdoc-ui/issue-101076.rs diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index 21d295bb1f88e..2b12d118bca0e 100644 --- a/src/librustdoc/passes/propagate_doc_cfg.rs +++ b/src/librustdoc/passes/propagate_doc_cfg.rs @@ -41,8 +41,9 @@ impl<'a, 'tcx> DocFolder for CfgPropagator<'a, 'tcx> { if self.parent != Some(expected_parent) { let mut attrs = Vec::new(); for (parent_hir_id, _) in hir.parent_iter(hir_id) { - let def_id = hir.local_def_id(parent_hir_id).to_def_id(); - attrs.extend_from_slice(load_attrs(self.cx, def_id)); + if let Some(def_id) = hir.opt_local_def_id(parent_hir_id) { + attrs.extend_from_slice(load_attrs(self.cx, def_id.to_def_id())); + } } let (_, cfg) = merge_attrs(self.cx, None, item.attrs.other_attrs.as_slice(), Some(&attrs)); diff --git a/src/test/rustdoc-ui/issue-101076.rs b/src/test/rustdoc-ui/issue-101076.rs new file mode 100644 index 0000000000000..648f9902908af --- /dev/null +++ b/src/test/rustdoc-ui/issue-101076.rs @@ -0,0 +1,14 @@ +// check-pass + +const _: () = { + #[macro_export] + macro_rules! first_macro { + () => {} + } + mod foo { + #[macro_export] + macro_rules! second_macro { + () => {} + } + } +}; From fc3f3c304bd6840f7c298a9d000c5ebddfe0b13c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 27 Aug 2022 18:36:57 +0300 Subject: [PATCH 18/19] rustc_middle: Remove `Visibility::Invisible` --- .../src/debuginfo/metadata.rs | 16 +++----- compiler/rustc_middle/src/middle/stability.rs | 2 +- .../rustc_middle/src/ty/inhabitedness/mod.rs | 4 -- compiler/rustc_middle/src/ty/mod.rs | 6 --- compiler/rustc_privacy/src/lib.rs | 1 - .../rustc_resolve/src/build_reduced_graph.rs | 10 ++--- compiler/rustc_resolve/src/check_unused.rs | 2 +- compiler/rustc_resolve/src/ident.rs | 10 ++++- compiler/rustc_resolve/src/imports.rs | 40 +++++++++++-------- src/librustdoc/clean/mod.rs | 5 --- 10 files changed, 43 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 25a989bdf0525..d0a6f216858b6 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1500,24 +1500,18 @@ fn vcall_visibility_metadata<'ll, 'tcx>( // If there is not LTO and the visibility in public, we have to assume that the vtable can // be seen from anywhere. With multiple CGUs, the vtable is quasi-public. (Lto::No | Lto::ThinLocal, Visibility::Public, _) - | (Lto::No, Visibility::Restricted(_) | Visibility::Invisible, false) => { - VCallVisibility::Public - } + | (Lto::No, Visibility::Restricted(_), false) => VCallVisibility::Public, // With LTO and a quasi-public visibility, the usages of the functions of the vtable are // all known by the `LinkageUnit`. // FIXME: LLVM only supports this optimization for `Lto::Fat` currently. Once it also // supports `Lto::Thin` the `VCallVisibility` may have to be adjusted for those. (Lto::Fat | Lto::Thin, Visibility::Public, _) - | ( - Lto::ThinLocal | Lto::Thin | Lto::Fat, - Visibility::Restricted(_) | Visibility::Invisible, - false, - ) => VCallVisibility::LinkageUnit, + | (Lto::ThinLocal | Lto::Thin | Lto::Fat, Visibility::Restricted(_), false) => { + VCallVisibility::LinkageUnit + } // If there is only one CGU, private vtables can only be seen by that CGU/translation unit // and therefore we know of all usages of functions in the vtable. - (_, Visibility::Restricted(_) | Visibility::Invisible, true) => { - VCallVisibility::TranslationUnit - } + (_, Visibility::Restricted(_), true) => VCallVisibility::TranslationUnit, }; let trait_ref_typeid = typeid_for_trait_ref(cx.tcx, trait_ref); diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 7a9ad44d1d9ae..d182929c40066 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -293,7 +293,7 @@ fn skip_stability_check_due_to_privacy(tcx: TyCtxt<'_>, def_id: DefId) -> bool { // These are not visible outside crate; therefore // stability markers are irrelevant, if even present. - ty::Visibility::Restricted(..) | ty::Visibility::Invisible => true, + ty::Visibility::Restricted(..) => true, } } diff --git a/compiler/rustc_middle/src/ty/inhabitedness/mod.rs b/compiler/rustc_middle/src/ty/inhabitedness/mod.rs index 3d22f5a04a2bf..aaa66deb2a3ec 100644 --- a/compiler/rustc_middle/src/ty/inhabitedness/mod.rs +++ b/compiler/rustc_middle/src/ty/inhabitedness/mod.rs @@ -169,14 +169,10 @@ impl<'tcx> FieldDef { param_env: ty::ParamEnv<'tcx>, ) -> DefIdForest<'tcx> { let data_uninhabitedness = move || self.ty(tcx, substs).uninhabited_from(tcx, param_env); - // FIXME(canndrew): Currently enum fields are (incorrectly) stored with - // `Visibility::Invisible` so we need to override `self.vis` if we're - // dealing with an enum. if is_enum { data_uninhabitedness() } else { match self.vis { - Visibility::Invisible => DefIdForest::empty(), Visibility::Restricted(from) => { let forest = DefIdForest::from_id(from); let iter = Some(forest).into_iter().chain(Some(data_uninhabitedness())); diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index a7833ab64310f..ed04e7660339e 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -268,8 +268,6 @@ pub enum Visibility { Public, /// Visible only in the given crate-local module. Restricted(DefId), - /// Not visible anywhere in the local crate. This is the visibility of private external items. - Invisible, } #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, TyEncodable, TyDecodable)] @@ -366,8 +364,6 @@ impl Visibility { let restriction = match self { // Public items are visible everywhere. Visibility::Public => return true, - // Private items from other crates are visible nowhere. - Visibility::Invisible => return false, // Restricted items are visible in an arbitrary local module. Visibility::Restricted(other) if other.krate != module.krate => return false, Visibility::Restricted(module) => module, @@ -380,7 +376,6 @@ impl Visibility { pub fn is_at_least(self, vis: Visibility, tree: T) -> bool { let vis_restriction = match vis { Visibility::Public => return self == Visibility::Public, - Visibility::Invisible => return true, Visibility::Restricted(module) => module, }; @@ -392,7 +387,6 @@ impl Visibility { match self { Visibility::Public => true, Visibility::Restricted(def_id) => def_id.is_local(), - Visibility::Invisible => false, } } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 27fa402edcec8..5d562f18a8158 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -1731,7 +1731,6 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { if !vis.is_at_least(self.required_visibility, self.tcx) { let vis_descr = match vis { ty::Visibility::Public => "public", - ty::Visibility::Invisible => "private", ty::Visibility::Restricted(vis_def_id) => { if vis_def_id == self.tcx.parent_module(hir_id).to_def_id() { "private" diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index e955a1798b735..8f3b6009bd6ef 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -380,7 +380,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { has_attributes: !item.attrs.is_empty(), root_span, root_id, - vis: Cell::new(vis), + vis: Cell::new(Some(vis)), used: Cell::new(false), }); @@ -588,7 +588,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { ast::UseTreeKind::Glob => { let kind = ImportKind::Glob { is_prelude: self.r.session.contains_name(&item.attrs, sym::prelude_import), - max_vis: Cell::new(ty::Visibility::Invisible), + max_vis: Cell::new(None), }; self.r.visibilities.insert(self.r.local_def_id(id), vis); self.add_import(prefix, kind, use_tree.span, id, item, root_span, item.id, vis); @@ -650,7 +650,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { true, // The whole `use` item item, - ty::Visibility::Invisible, + ty::Visibility::Restricted(self.parent_scope.module.nearest_parent_mod()), root_span, ); } @@ -885,7 +885,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { root_span: item.span, span: item.span, module_path: Vec::new(), - vis: Cell::new(vis), + vis: Cell::new(Some(vis)), used: Cell::new(used), }); self.r.potentially_unused_imports.push(import); @@ -1118,7 +1118,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { root_span: span, span, module_path: Vec::new(), - vis: Cell::new(ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id())), + vis: Cell::new(Some(ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id()))), used: Cell::new(false), }) }; diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index f2f6f1d895e32..8b58b32b65649 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -227,7 +227,7 @@ impl Resolver<'_> { for import in self.potentially_unused_imports.iter() { match import.kind { _ if import.used.get() - || import.vis.get().is_public() + || import.expect_vis().is_public() || import.span.is_dummy() => { if let ImportKind::MacroUse = import.kind { diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index 41a0c76d83a95..23c0ca108d385 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -953,7 +953,10 @@ impl<'a> Resolver<'a> { // Check if one of single imports can still define the name, // if it can then our result is not determined and can be invalidated. for single_import in &resolution.single_imports { - if !self.is_accessible_from(single_import.vis.get(), parent_scope.module) { + let Some(import_vis) = single_import.vis.get() else { + continue; + }; + if !self.is_accessible_from(import_vis, parent_scope.module) { continue; } let Some(module) = single_import.imported_module.get() else { @@ -1018,7 +1021,10 @@ impl<'a> Resolver<'a> { // Check if one of glob imports can still define the name, // if it can then our "no resolution" result is not determined and can be invalidated. for glob_import in module.globs.borrow().iter() { - if !self.is_accessible_from(glob_import.vis.get(), parent_scope.module) { + let Some(import_vis) = glob_import.vis.get() else { + continue; + }; + if !self.is_accessible_from(import_vis, parent_scope.module) { continue; } let module = match glob_import.imported_module.get() { diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index e0d57ded5bf24..c2491c6ebdec0 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -52,8 +52,8 @@ pub enum ImportKind<'a> { }, Glob { is_prelude: bool, - max_vis: Cell, // The visibility of the greatest re-export. - // n.b. `max_vis` is only used in `finalize_import` to check for re-export errors. + max_vis: Cell>, // The visibility of the greatest re-export. + // n.b. `max_vis` is only used in `finalize_import` to check for re-export errors. }, ExternCrate { source: Option, @@ -144,7 +144,7 @@ pub(crate) struct Import<'a> { pub module_path: Vec, /// The resolution of `module_path`. pub imported_module: Cell>>, - pub vis: Cell, + pub vis: Cell>, pub used: Cell, } @@ -159,6 +159,10 @@ impl<'a> Import<'a> { _ => false, } } + + pub(crate) fn expect_vis(&self) -> ty::Visibility { + self.vis.get().expect("encountered cleared import visibility") + } } /// Records information about the resolution of a name in a namespace of a module. @@ -199,7 +203,7 @@ fn pub_use_of_private_extern_crate_hack(import: &Import<'_>, binding: &NameBindi import: Import { kind: ImportKind::ExternCrate { .. }, .. }, .. }, - ) => import.vis.get().is_public(), + ) => import.expect_vis().is_public(), _ => false, } } @@ -212,17 +216,20 @@ impl<'a> Resolver<'a> { binding: &'a NameBinding<'a>, import: &'a Import<'a>, ) -> &'a NameBinding<'a> { - let vis = if binding.vis.is_at_least(import.vis.get(), self) + let import_vis = import.expect_vis(); + let vis = if binding.vis.is_at_least(import_vis, self) || pub_use_of_private_extern_crate_hack(import, binding) { - import.vis.get() + import_vis } else { binding.vis }; if let ImportKind::Glob { ref max_vis, .. } = import.kind { - if vis == import.vis.get() || vis.is_at_least(max_vis.get(), self) { - max_vis.set(vis) + if vis == import_vis + || max_vis.get().map_or(true, |max_vis| vis.is_at_least(max_vis, self)) + { + max_vis.set(Some(vis)) } } @@ -536,7 +543,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } else { // For better failure detection, pretend that the import will // not define any names while resolving its module path. - let orig_vis = import.vis.replace(ty::Visibility::Invisible); + let orig_vis = import.vis.take(); let path_res = self.r.maybe_resolve_path(&import.module_path, None, &import.parent_scope); import.vis.set(orig_vis); @@ -571,7 +578,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { if let Err(Undetermined) = source_bindings[ns].get() { // For better failure detection, pretend that the import will // not define any names while resolving its module path. - let orig_vis = import.vis.replace(ty::Visibility::Invisible); + let orig_vis = import.vis.take(); let binding = this.resolve_ident_in_module( module, source, @@ -620,7 +627,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { /// Optionally returns an unresolved import error. This error is buffered and used to /// consolidate multiple unresolved import errors into a single diagnostic. fn finalize_import(&mut self, import: &'b Import<'b>) -> Option { - let orig_vis = import.vis.replace(ty::Visibility::Invisible); + let orig_vis = import.vis.take(); let ignore_binding = match &import.kind { ImportKind::Single { target_bindings, .. } => target_bindings[TypeNS].get(), _ => None, @@ -727,9 +734,9 @@ impl<'a, 'b> ImportResolver<'a, 'b> { }); } } - if !is_prelude && - max_vis.get() != ty::Visibility::Invisible && // Allow empty globs. - !max_vis.get().is_at_least(import.vis.get(), &*self.r) + if !is_prelude + && let Some(max_vis) = max_vis.get() + && !max_vis.is_at_least(import.expect_vis(), &*self.r) { let msg = "glob import doesn't reexport anything because no candidate is public enough"; self.r.lint_buffer.buffer_lint(UNUSED_IMPORTS, import.id, import.span, msg); @@ -742,7 +749,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { let mut all_ns_err = true; self.r.per_ns(|this, ns| { if !type_ns_only || ns == TypeNS { - let orig_vis = import.vis.replace(ty::Visibility::Invisible); + let orig_vis = import.vis.take(); let binding = this.resolve_ident_in_module( module, ident, @@ -906,8 +913,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { let mut crate_private_reexport = false; self.r.per_ns(|this, ns| { if let Ok(binding) = source_bindings[ns].get() { - let vis = import.vis.get(); - if !binding.vis.is_at_least(vis, &*this) { + if !binding.vis.is_at_least(import.expect_vis(), &*this) { reexport_error = Some((ns, binding)); if let ty::Visibility::Restricted(binding_def_id) = binding.vis { if binding_def_id.is_top_level_module() { diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 5507ffb871b6e..420159b5a6751 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1776,11 +1776,6 @@ fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool { pub(crate) fn clean_visibility(vis: ty::Visibility) -> Visibility { match vis { ty::Visibility::Public => Visibility::Public, - // NOTE: this is not quite right: `ty` uses `Invisible` to mean 'private', - // while rustdoc really does mean inherited. That means that for enum variants, such as - // `pub enum E { V }`, `V` will be marked as `Public` by `ty`, but as `Inherited` by rustdoc. - // Various parts of clean override `tcx.visibility` explicitly to make sure this distinction is captured. - ty::Visibility::Invisible => Visibility::Inherited, ty::Visibility::Restricted(module) => Visibility::Restricted(module), } } From 4a82c2174a7e986f9a50d9ec806c4c47bd22eed8 Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 27 Aug 2022 17:32:04 +0200 Subject: [PATCH 19/19] unstable-book-gen: use std::fs::write I wrote this code in 2017 back when std::fs::write wasn't available. Also, use the t macro from tidy. --- src/tools/unstable-book-gen/src/main.rs | 33 ++++++++----------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/src/tools/unstable-book-gen/src/main.rs b/src/tools/unstable-book-gen/src/main.rs index 5a0477b4b941e..a9830a3840d1a 100644 --- a/src/tools/unstable-book-gen/src/main.rs +++ b/src/tools/unstable-book-gen/src/main.rs @@ -2,37 +2,23 @@ use std::collections::BTreeSet; use std::env; -use std::fs::{self, File}; -use std::io::Write; +use std::fs::{self, write}; use std::path::Path; use tidy::features::{collect_lang_features, collect_lib_features, Features}; +use tidy::t; use tidy::unstable_book::{ collect_unstable_book_section_file_names, collect_unstable_feature_names, LANG_FEATURES_DIR, LIB_FEATURES_DIR, PATH_STR, }; -/// A helper macro to `unwrap` a result except also print out details like: -/// -/// * The file/line of the panic -/// * The expression that failed -/// * The error itself -macro_rules! t { - ($e:expr) => { - match $e { - Ok(e) => e, - Err(e) => panic!("{} failed with {}", stringify!($e), e), - } - }; -} - fn generate_stub_issue(path: &Path, name: &str, issue: u32) { - let mut file = t!(File::create(path)); - t!(write!(file, include_str!("stub-issue.md"), name = name, issue = issue)); + let content = format!(include_str!("stub-issue.md"), name = name, issue = issue); + t!(write(path, content), path); } fn generate_stub_no_issue(path: &Path, name: &str) { - let mut file = t!(File::create(path)); - t!(write!(file, include_str!("stub-no-issue.md"), name = name)); + let content = format!(include_str!("stub-no-issue.md"), name = name); + t!(write(path, content), path); } fn set_to_summary_str(set: &BTreeSet, dir: &str) -> String { @@ -52,13 +38,14 @@ fn generate_summary(path: &Path, lang_features: &Features, lib_features: &Featur let lang_features_str = set_to_summary_str(&unstable_lang_features, "language-features"); let lib_features_str = set_to_summary_str(&unstable_lib_features, "library-features"); - let mut file = t!(File::create(&path.join("src/SUMMARY.md"))); - t!(file.write_fmt(format_args!( + let summary_path = path.join("src/SUMMARY.md"); + let content = format!( include_str!("SUMMARY.md"), compiler_flags = compiler_flags_str, language_features = lang_features_str, library_features = lib_features_str - ))); + ); + t!(write(&summary_path, content), summary_path); } fn generate_unstable_book_files(src: &Path, out: &Path, features: &Features) {