From 5dcc418f62e14580a319b2b8ec24645abbc1569e Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Fri, 17 Jun 2022 19:46:01 -0700 Subject: [PATCH 01/17] Document the conditional existence of `alloc::sync` and `alloc::task`. The wording is copied from `std::sync::atomic::AtomicPtr`, with additional advice on how to `#[cfg]` for it. --- library/alloc/src/sync.rs | 9 +++++++++ library/alloc/src/task.rs | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 55d51e0a3c4cf..d91217f341abd 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -3,6 +3,10 @@ //! Thread-safe reference-counting pointers. //! //! See the [`Arc`][Arc] documentation for more details. +//! +//! **Note**: This module is only available on platforms that support atomic +//! loads and stores of pointers. This may be detected at compile time using +//! `#[cfg(target_has_atomic = "ptr")]`. use core::any::Any; use core::borrow; @@ -82,6 +86,11 @@ macro_rules! acquire { /// [`Mutex`][mutex], [`RwLock`][rwlock], or one of the [`Atomic`][atomic] /// types. /// +/// **Note**: This type is only available on platforms that support atomic +/// loads and stores of pointers, which includes all platforms that support +/// the `std` crate but not all those which only support [`alloc`](crate). +/// This may be detected at compile time using `#[cfg(target_has_atomic = "ptr")]`. +/// /// ## Thread Safety /// /// Unlike [`Rc`], `Arc` uses atomic operations for its reference diff --git a/library/alloc/src/task.rs b/library/alloc/src/task.rs index 528ee4ff1542c..9d8e309a978d9 100644 --- a/library/alloc/src/task.rs +++ b/library/alloc/src/task.rs @@ -1,5 +1,11 @@ #![stable(feature = "wake_trait", since = "1.51.0")] + //! Types and Traits for working with asynchronous tasks. +//! +//! **Note**: This module is only available on platforms that support atomic +//! loads and stores of pointers. This may be detected at compile time using +//! `#[cfg(target_has_atomic = "ptr")]`. + use core::mem::ManuallyDrop; use core::task::{RawWaker, RawWakerVTable, Waker}; From 9cd66be2351099d2a74aeb67ccb13e148dfdf114 Mon Sep 17 00:00:00 2001 From: Duarte Nunes Date: Wed, 13 Jul 2022 14:22:50 -0300 Subject: [PATCH 02/17] docs: be less harsh in wording for Vec::from_raw_parts In particular, be clear that it is sound to specify memory not originating from a previous `Vec` allocation. That is already suggested in other parts of the documentation about zero-alloc conversions to Box<[T]>. Incorporate a constraint from `slice::from_raw_parts` that was missing but needs to be fulfilled, since a `Vec` can be converted into a slice. --- library/alloc/src/vec/mod.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index fa9f2131c0c1d..aa822b1f923df 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -490,8 +490,6 @@ impl Vec { /// This is highly unsafe, due to the number of invariants that aren't /// checked: /// - /// * `ptr` needs to have been previously allocated via [`String`]/`Vec` - /// (at least, it's highly likely to be incorrect if it wasn't). /// * `T` needs to have the same alignment as what `ptr` was allocated with. /// (`T` having a less strict alignment is not sufficient, the alignment really /// needs to be equal to satisfy the [`dealloc`] requirement that memory must be @@ -500,6 +498,12 @@ impl Vec { /// to be the same size as the pointer was allocated with. (Because similar to /// alignment, [`dealloc`] must be called with the same layout `size`.) /// * `length` needs to be less than or equal to `capacity`. + /// * `capacity` needs to be the capacity that the pointer was allocated with. + /// * The allocated size in bytes must be no larger than `isize::MAX`. + /// See the safety documentation of [`pointer::offset`]. + /// + /// To ensure these requirements are easily met, ensure `ptr` has previously + /// been allocated via `Vec`. /// /// Violating these may cause problems like corrupting the allocator's /// internal data structures. For example it is normally **not** safe @@ -648,14 +652,20 @@ impl Vec { /// This is highly unsafe, due to the number of invariants that aren't /// checked: /// - /// * `ptr` needs to have been previously allocated via [`String`]/`Vec` - /// (at least, it's highly likely to be incorrect if it wasn't). - /// * `T` needs to have the same size and alignment as what `ptr` was allocated with. + /// * `T` needs to have the same alignment as what `ptr` was allocated with. /// (`T` having a less strict alignment is not sufficient, the alignment really /// needs to be equal to satisfy the [`dealloc`] requirement that memory must be /// allocated and deallocated with the same layout.) + /// * The size of `T` times the `capacity` (ie. the allocated size in bytes) needs + /// to be the same size as the pointer was allocated with. (Because similar to + /// alignment, [`dealloc`] must be called with the same layout `size`.) /// * `length` needs to be less than or equal to `capacity`. /// * `capacity` needs to be the capacity that the pointer was allocated with. + /// * The allocated size in bytes must be no larger than `isize::MAX`. + /// See the safety documentation of [`pointer::offset`]. + /// + /// To ensure these requirements are easily met, ensure `ptr` has previously + /// been allocated via `Vec`. /// /// Violating these may cause problems like corrupting the allocator's /// internal data structures. For example it is **not** safe From c9ec7aa0d7013bab63477adc77a94163a0df2c32 Mon Sep 17 00:00:00 2001 From: Duarte Nunes Date: Wed, 13 Jul 2022 19:46:04 -0300 Subject: [PATCH 03/17] changes to wording --- library/alloc/src/vec/mod.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index aa822b1f923df..17342cacd760e 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -483,7 +483,7 @@ impl Vec { Self::with_capacity_in(capacity, Global) } - /// Creates a `Vec` directly from the raw components of another vector. + /// Creates a `Vec` directly from a pointer, a capacity, and a length. /// /// # Safety /// @@ -498,12 +498,14 @@ impl Vec { /// to be the same size as the pointer was allocated with. (Because similar to /// alignment, [`dealloc`] must be called with the same layout `size`.) /// * `length` needs to be less than or equal to `capacity`. + /// * The first `length` values must be properly initialized values of type `T`. /// * `capacity` needs to be the capacity that the pointer was allocated with. - /// * The allocated size in bytes must be no larger than `isize::MAX`. + /// * The allocated size in bytes must be no larger than `isize::MAX`. /// See the safety documentation of [`pointer::offset`]. /// - /// To ensure these requirements are easily met, ensure `ptr` has previously - /// been allocated via `Vec`. + /// These requirements are always uphead by any `ptr` that has been allocated + /// via `Vec`. Other allocation sources are allowed if the invariants are + /// upheld. /// /// Violating these may cause problems like corrupting the allocator's /// internal data structures. For example it is normally **not** safe @@ -645,7 +647,8 @@ impl Vec { Vec { buf: RawVec::with_capacity_in(capacity, alloc), len: 0 } } - /// Creates a `Vec` directly from the raw components of another vector. + /// Creates a `Vec` directly from a pointer, a capacity, a length, + /// and an allocator. /// /// # Safety /// @@ -660,12 +663,14 @@ impl Vec { /// to be the same size as the pointer was allocated with. (Because similar to /// alignment, [`dealloc`] must be called with the same layout `size`.) /// * `length` needs to be less than or equal to `capacity`. - /// * `capacity` needs to be the capacity that the pointer was allocated with. - /// * The allocated size in bytes must be no larger than `isize::MAX`. + /// * The first `length` values must be properly initialized values of type `T`. + /// * `capacity` needs to [fit] the layout size that the pointer was allocated with. + /// * The allocated size in bytes must be no larger than `isize::MAX`. /// See the safety documentation of [`pointer::offset`]. /// - /// To ensure these requirements are easily met, ensure `ptr` has previously - /// been allocated via `Vec`. + /// These requirements are always uphead by any `ptr` that has been allocated + /// via `Vec`. Other allocation sources are allowed if the invariants are + /// upheld. /// /// Violating these may cause problems like corrupting the allocator's /// internal data structures. For example it is **not** safe @@ -683,6 +688,7 @@ impl Vec { /// /// [`String`]: crate::string::String /// [`dealloc`]: crate::alloc::GlobalAlloc::dealloc + /// [*fit*]: crate::alloc::Allocator#memory-fitting /// /// # Examples /// From 050115c0d4115fb31c056ae858384ef48a77239d Mon Sep 17 00:00:00 2001 From: Duarte Nunes Date: Wed, 13 Jul 2022 21:49:31 -0300 Subject: [PATCH 04/17] typo --- library/alloc/src/vec/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 17342cacd760e..6b544c7025007 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -503,7 +503,7 @@ impl Vec { /// * The allocated size in bytes must be no larger than `isize::MAX`. /// See the safety documentation of [`pointer::offset`]. /// - /// These requirements are always uphead by any `ptr` that has been allocated + /// These requirements are always upheld by any `ptr` that has been allocated /// via `Vec`. Other allocation sources are allowed if the invariants are /// upheld. /// @@ -668,7 +668,7 @@ impl Vec { /// * The allocated size in bytes must be no larger than `isize::MAX`. /// See the safety documentation of [`pointer::offset`]. /// - /// These requirements are always uphead by any `ptr` that has been allocated + /// These requirements are always upheld by any `ptr` that has been allocated /// via `Vec`. Other allocation sources are allowed if the invariants are /// upheld. /// From 8d35ab38067ede0b2816e0177d1d0e0869e89136 Mon Sep 17 00:00:00 2001 From: Duarte Nunes Date: Wed, 13 Jul 2022 22:04:15 -0300 Subject: [PATCH 05/17] rustdoc --- library/alloc/src/vec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 6b544c7025007..fcfb88b18c7e9 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -664,7 +664,7 @@ impl Vec { /// alignment, [`dealloc`] must be called with the same layout `size`.) /// * `length` needs to be less than or equal to `capacity`. /// * The first `length` values must be properly initialized values of type `T`. - /// * `capacity` needs to [fit] the layout size that the pointer was allocated with. + /// * `capacity` needs to [*fit*] the layout size that the pointer was allocated with. /// * The allocated size in bytes must be no larger than `isize::MAX`. /// See the safety documentation of [`pointer::offset`]. /// From a85ee3ed9130de6b263de0b05e67b04e644f269e Mon Sep 17 00:00:00 2001 From: Duarte Nunes Date: Thu, 14 Jul 2022 11:47:06 -0300 Subject: [PATCH 06/17] add code examples --- library/alloc/src/vec/mod.rs | 51 +++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index fcfb88b18c7e9..ae99772450e21 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -557,6 +557,32 @@ impl Vec { /// assert_eq!(rebuilt, [4, 5, 6]); /// } /// ``` + /// + /// Using memory that was allocated elsewhere: + /// + /// ```rust + /// #![feature(allocator_api)] + /// + /// use std::alloc::{AllocError, Allocator, Global, Layout}; + /// + /// fn main() { + /// let layout = Layout::array::(16).expect("overflow cannot happen"); + /// + /// let vec = unsafe { + /// let mem = match Global.allocate(layout) { + /// Ok(mem) => mem.cast::().as_ptr(), + /// Err(AllocError) => return, + /// }; + /// + /// mem.write(1_000_000); + /// + /// Vec::from_raw_parts_in(mem, 1, 16, Global) + /// }; + /// + /// assert_eq!(vec, &[1_000_000]); + /// assert_eq!(vec.capacity(), 16); + /// } + /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn from_raw_parts(ptr: *mut T, length: usize, capacity: usize) -> Self { @@ -669,7 +695,7 @@ impl Vec { /// See the safety documentation of [`pointer::offset`]. /// /// These requirements are always upheld by any `ptr` that has been allocated - /// via `Vec`. Other allocation sources are allowed if the invariants are + /// via `Vec`. Other allocation sources are allowed if the invariants are /// upheld. /// /// Violating these may cause problems like corrupting the allocator's @@ -727,6 +753,29 @@ impl Vec { /// assert_eq!(rebuilt, [4, 5, 6]); /// } /// ``` + /// + /// Using memory that was allocated elsewhere: + /// + /// ```rust + /// use std::alloc::{alloc, Layout}; + /// + /// fn main() { + /// let layout = Layout::array::(16).expect("overflow cannot happen"); + /// let vec = unsafe { + /// let mem = alloc(layout).cast::(); + /// if mem.is_null() { + /// return; + /// } + /// + /// mem.write(1_000_000); + /// + /// Vec::from_raw_parts(mem, 1, 16) + /// }; + /// + /// assert_eq!(vec, &[1_000_000]); + /// assert_eq!(vec.capacity(), 16); + /// } + /// ``` #[inline] #[unstable(feature = "allocator_api", issue = "32838")] pub unsafe fn from_raw_parts_in(ptr: *mut T, length: usize, capacity: usize, alloc: A) -> Self { From 551d921de0f13c54093ab4f6a3adbe69292e091a Mon Sep 17 00:00:00 2001 From: Jan Behrens Date: Tue, 19 Jul 2022 11:40:40 +0200 Subject: [PATCH 07/17] docs: Improve AsRef / AsMut docs on blanket impls - Explicitly mention that `AsRef` and `AsMut` do not auto-dereference generally for all dereferencable types (but only if inner type is a shared and/or mutable reference) - Give advice to not use `AsRef` or `AsMut` for the sole purpose of dereferencing - Suggest providing a transitive `AsRef` or `AsMut` implementation for types which implement `Deref` - Add new section "Reflexivity" in documentation comments for `AsRef` and `AsMut` - Provide better example for `AsMut` - Added heading "Relation to `Borrow`" in `AsRef`'s docs to improve structure Issue #45742 and a corresponding FIXME in the libcore suggest that `AsRef` and `AsMut` should provide a blanket implementation over `Deref`. As that is difficult to realize at the moment, this commit updates the documentation to better describe the status-quo and to give advice on how to use `AsRef` and `AsMut`. --- library/core/src/convert/mod.rs | 177 ++++++++++++++++++++++++++++---- 1 file changed, 159 insertions(+), 18 deletions(-) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index b30c8a4aeabdd..dd0d6478c3a20 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -25,6 +25,7 @@ //! # Generic Implementations //! //! - [`AsRef`] and [`AsMut`] auto-dereference if the inner type is a reference +//! (but not generally for all [dereferenceable types][core::ops::Deref]) //! - [`From`]` for T` implies [`Into`]` for U` //! - [`TryFrom`]` for T` implies [`TryInto`]` for U` //! - [`From`] and [`Into`] are reflexive, which means that all types can @@ -108,10 +109,12 @@ pub const fn identity(x: T) -> T { /// If you need to do a costly conversion it is better to implement [`From`] with type /// `&T` or write a custom function. /// +/// # Relation to `Borrow` +/// /// `AsRef` has the same signature as [`Borrow`], but [`Borrow`] is different in a few aspects: /// /// - Unlike `AsRef`, [`Borrow`] has a blanket impl for any `T`, and can be used to accept either -/// a reference or a value. +/// a reference or a value. (See also note on `AsRef`'s reflexibility below.) /// - [`Borrow`] also requires that [`Hash`], [`Eq`] and [`Ord`] for a borrowed value are /// equivalent to those of the owned value. For this reason, if you want to /// borrow only a single field of a struct you can implement `AsRef`, but not [`Borrow`]. @@ -121,9 +124,55 @@ pub const fn identity(x: T) -> T { /// /// # Generic Implementations /// -/// - `AsRef` auto-dereferences if the inner type is a reference or a mutable -/// reference (e.g.: `foo.as_ref()` will work the same if `foo` has type -/// `&mut Foo` or `&&mut Foo`) +/// `AsRef` auto-dereferences if the inner type is a reference or a mutable reference +/// (e.g.: `foo.as_ref()` will work the same if `foo` has type `&mut Foo` or `&&mut Foo`). +/// +/// Note that due to historic reasons, the above currently does not hold generally for all +/// [dereferenceable types], e.g. `foo.as_ref()` will *not* work the same as +/// `Box::new(foo).as_ref()`. Instead, many smart pointers provide an `as_ref` implementation which +/// simply returns a reference to the [pointed-to value] (but do not perform a cheap +/// reference-to-reference conversion for that value). However, [`AsRef::as_ref`] should not be +/// used for the sole purpose of dereferencing; instead ['`Deref` coercion'] can be used: +/// +/// [dereferenceable types]: core::ops::Deref +/// [pointed-to value]: core::ops::Deref::Target +/// ['`Deref` coercion']: core::ops::Deref#more-on-deref-coercion +/// +/// ``` +/// let x = Box::new(5i32); +/// // Avoid this: +/// // let y: &i32 = x.as_ref(); +/// // Better just write: +/// let y: &i32 = &x; +/// ``` +/// +/// Types which implement [`Deref`][core::ops::Deref] should consider implementing `AsRef` as +/// follows: +/// +/// ``` +/// impl AsRef for SomeType +/// where +/// T: ?Sized, +/// ::Target: AsRef, +/// { +/// fn as_ref(&self) -> &T { +/// self.deref().as_ref() +/// } +/// } +/// ``` +/// +/// # Reflexivity +/// +/// Ideally, `AsRef` would be reflexive, that is there is an `impl AsRef for T`, with +/// [`as_ref`][AsRef::as_ref] simply returning its argument unchanged. +/// Such a blanket implementation is currently *not* provided due to technical restrictions of +/// Rust's type system (it would be overlapping with another existing blanket implementation for +/// `&T where T: AsRef` which allows `AsRef` to auto-dereference, see "Generic Implementations" +/// above). +/// +/// A trivial implementation of `AsRef for T` must be added explicitly for a particular type `T` +/// where needed or desired. Note, however, that not all types from `std` contain such an +/// implementation, and those cannot be added by external code due to orphan rules. /// /// # Examples /// @@ -170,29 +219,121 @@ pub trait AsRef { /// /// # Generic Implementations /// -/// - `AsMut` auto-dereferences if the inner type is a mutable reference -/// (e.g.: `foo.as_mut()` will work the same if `foo` has type `&mut Foo` -/// or `&mut &mut Foo`) +/// `AsMut` auto-dereferences if the inner type is a mutable reference +/// (e.g.: `foo.as_mut()` will work the same if `foo` has type `&mut Foo` or `&mut &mut Foo`). +/// +/// Note that due to historic reasons, the above currently does not hold generally for all +/// [mutably dereferenceable types], e.g. `foo.as_mut()` will *not* work the same as +/// `Box::new(foo).as_mut()`. Instead, many smart pointers provide an `as_mut` implementation which +/// simply returns a reference to the [pointed-to value] (but do not perform a cheap +/// reference-to-reference conversion for that value). However, [`AsMut::as_mut`] should not be +/// used for the sole purpose of mutable dereferencing; instead ['`Deref` coercion'] can be used: +/// +/// [mutably dereferenceable types]: core::ops::DerefMut +/// [pointed-to value]: core::ops::Deref::Target +/// ['`Deref` coercion']: core::ops::DerefMut#more-on-deref-coercion +/// +/// ``` +/// let mut x = Box::new(5i32); +/// // Avoid this: +/// // let y: &mut i32 = x.as_mut(); +/// // Better just write: +/// let y: &mut i32 = &mut x; +/// ``` +/// +/// Types which implement [`DerefMut`](core::ops::DerefMut) should consider to add an +/// implementation of `AsMut` as follows: +/// +/// ``` +/// impl AsMut for SomeType +/// where +/// ::Target: AsMut, +/// { +/// fn as_mut(&mut self) -> &mut T { +/// self.deref_mut().as_mut() +/// } +/// } +/// ``` +/// +/// # Reflexivity +/// +/// Ideally, `AsMut` would be reflexive, that is there is an `impl AsMut for T`, with +/// [`as_mut`][AsMut::as_mut] simply returning its argument unchanged. +/// Such a blanket implementation is currently *not* provided due to technical restrictions of +/// Rust's type system (it would be overlapping with another existing blanket implementation for +/// `&mut T where T: AsMut` which allows `AsMut` to auto-dereference, see "Generic +/// Implementations" above). +/// +/// A trivial implementation of `AsMut for T` must be added explicitly for a particular type `T` +/// where needed or desired. Note, however, that not all types from `std` contain such an +/// implementation, and those cannot be added by external code due to orphan rules. /// /// # Examples /// -/// Using `AsMut` as trait bound for a generic function we can accept all mutable references -/// that can be converted to type `&mut T`. Because [`Box`] implements `AsMut` we can -/// write a function `add_one` that takes all arguments that can be converted to `&mut u64`. -/// Because [`Box`] implements `AsMut`, `add_one` accepts arguments of type -/// `&mut Box` as well: +/// Using `AsMut` as trait bound for a generic function, we can accept all mutable references that +/// can be converted to type `&mut T`. Unlike [dereference], which has a single [target type], +/// there can be multiple implementations of `AsMut` for a type. In particular, `Vec` implements +/// both `AsMut>` and `AsMut<[T]>`. +/// +/// In the following, the example functions `caesar` and `null_terminate` provide a generic +/// interface which work with any type that can be converted by cheap mutable-to-mutable conversion +/// into a byte slice or byte `Vec`, respectively. +/// +/// [dereference]: core::ops::DerefMut +/// [target type]: core::ops::Deref::Target /// /// ``` -/// fn add_one>(num: &mut T) { -/// *num.as_mut() += 1; +/// struct Document { +/// info: String, +/// content: Vec, /// } /// -/// let mut boxed_num = Box::new(0); -/// add_one(&mut boxed_num); -/// assert_eq!(*boxed_num, 1); +/// impl AsMut for Document +/// where +/// Vec: AsMut, +/// { +/// fn as_mut(&mut self) -> &mut T { +/// self.content.as_mut() +/// } +/// } +/// +/// fn caesar>(data: &mut T, key: u8) { +/// for byte in data.as_mut() { +/// *byte = byte.wrapping_add(key); +/// } +/// } +/// +/// fn null_terminate>>(data: &mut T) { +/// // Using a non-generic inner function, which contains most of the +/// // functionality, helps to minimize monomorphization overhead. +/// fn doit(data: &mut Vec) { +/// let len = data.len(); +/// if len == 0 || data[len-1] != 0 { +/// data.push(0); +/// } +/// } +/// doit(data.as_mut()); +/// } +/// +/// fn main() { +/// let mut v: Vec = vec![1, 2, 3]; +/// caesar(&mut v, 5); +/// assert_eq!(v, [6, 7, 8]); +/// null_terminate(&mut v); +/// assert_eq!(v, [6, 7, 8, 0]); +/// let mut doc = Document { +/// info: String::from("Example"), +/// content: vec![17, 19, 8], +/// }; +/// caesar(&mut doc, 1); +/// assert_eq!(doc.content, [18, 20, 9]); +/// null_terminate(&mut doc); +/// assert_eq!(doc.content, [18, 20, 9, 0]); +/// } /// ``` /// -/// [`Box`]: ../../std/boxed/struct.Box.html +/// Note, however, that APIs don't need to be generic. In many cases taking a `&mut [u8]` or +/// `&mut Vec`, for example, is the better choice (callers need to pass the correct type then). #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "AsMut")] pub trait AsMut { From 9f68e3ef1b3bc0a7f1ce4d7ff7fc74bcaa2d42ad Mon Sep 17 00:00:00 2001 From: Jan Behrens Date: Tue, 19 Jul 2022 23:24:51 +0200 Subject: [PATCH 08/17] fixup! docs: Improve AsRef / AsMut docs on blanket impls Fixed examples in sections "Generic Implementations" of `AsRef`'s and `AsMut`'s doc comments, which failed tests. --- library/core/src/convert/mod.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index dd0d6478c3a20..e75e32855d1bb 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -150,6 +150,14 @@ pub const fn identity(x: T) -> T { /// follows: /// /// ``` +/// # use core::ops::Deref; +/// # struct SomeType; +/// # impl Deref for SomeType { +/// # type Target = [u8]; +/// # fn deref(&self) -> &[u8] { +/// # &[] +/// # } +/// # } /// impl AsRef for SomeType /// where /// T: ?Sized, @@ -245,6 +253,19 @@ pub trait AsRef { /// implementation of `AsMut` as follows: /// /// ``` +/// # use core::ops::{Deref, DerefMut}; +/// # struct SomeType; +/// # impl Deref for SomeType { +/// # type Target = [u8]; +/// # fn deref(&self) -> &[u8] { +/// # &[] +/// # } +/// # } +/// # impl DerefMut for SomeType { +/// # fn deref_mut(&mut self) -> &mut [u8] { +/// # &mut [] +/// # } +/// # } /// impl AsMut for SomeType /// where /// ::Target: AsMut, From e4a259b5e480ea5b444384f9ff9cd862a46a9d16 Mon Sep 17 00:00:00 2001 From: Jan Behrens Date: Tue, 19 Jul 2022 23:53:40 +0200 Subject: [PATCH 09/17] fixup! docs: Improve AsRef / AsMut docs on blanket impls Better conform to Rust API Documentation Conventions --- library/core/src/convert/mod.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index e75e32855d1bb..bdd862f39c801 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -146,8 +146,9 @@ pub const fn identity(x: T) -> T { /// let y: &i32 = &x; /// ``` /// -/// Types which implement [`Deref`][core::ops::Deref] should consider implementing `AsRef` as -/// follows: +/// Types which implement [`Deref`] should consider implementing `AsRef` as follows: +/// +/// [`Deref`]: core::ops::Deref /// /// ``` /// # use core::ops::Deref; @@ -172,12 +173,14 @@ pub const fn identity(x: T) -> T { /// # Reflexivity /// /// Ideally, `AsRef` would be reflexive, that is there is an `impl AsRef for T`, with -/// [`as_ref`][AsRef::as_ref] simply returning its argument unchanged. +/// [`as_ref`] simply returning its argument unchanged. /// Such a blanket implementation is currently *not* provided due to technical restrictions of /// Rust's type system (it would be overlapping with another existing blanket implementation for /// `&T where T: AsRef` which allows `AsRef` to auto-dereference, see "Generic Implementations" /// above). /// +/// [`as_ref`]: AsRef::as_ref +/// /// A trivial implementation of `AsRef for T` must be added explicitly for a particular type `T` /// where needed or desired. Note, however, that not all types from `std` contain such an /// implementation, and those cannot be added by external code due to orphan rules. @@ -249,8 +252,10 @@ pub trait AsRef { /// let y: &mut i32 = &mut x; /// ``` /// -/// Types which implement [`DerefMut`](core::ops::DerefMut) should consider to add an -/// implementation of `AsMut` as follows: +/// Types which implement [`DerefMut`] should consider to add an implementation of `AsMut` as +/// follows: +/// +/// [`DerefMut`]: core::ops::DerefMut /// /// ``` /// # use core::ops::{Deref, DerefMut}; @@ -279,12 +284,14 @@ pub trait AsRef { /// # Reflexivity /// /// Ideally, `AsMut` would be reflexive, that is there is an `impl AsMut for T`, with -/// [`as_mut`][AsMut::as_mut] simply returning its argument unchanged. +/// [`as_mut`] simply returning its argument unchanged. /// Such a blanket implementation is currently *not* provided due to technical restrictions of /// Rust's type system (it would be overlapping with another existing blanket implementation for /// `&mut T where T: AsMut` which allows `AsMut` to auto-dereference, see "Generic /// Implementations" above). /// +/// [`as_mut`]: AsMut::as_mut +/// /// A trivial implementation of `AsMut for T` must be added explicitly for a particular type `T` /// where needed or desired. Note, however, that not all types from `std` contain such an /// implementation, and those cannot be added by external code due to orphan rules. @@ -298,7 +305,7 @@ pub trait AsRef { /// /// In the following, the example functions `caesar` and `null_terminate` provide a generic /// interface which work with any type that can be converted by cheap mutable-to-mutable conversion -/// into a byte slice or byte `Vec`, respectively. +/// into a byte slice (`[u8]`) or byte vector (`Vec`), respectively. /// /// [dereference]: core::ops::DerefMut /// [target type]: core::ops::Deref::Target From e6b761b902fe80ba454368d078fb834554981109 Mon Sep 17 00:00:00 2001 From: Jan Behrens Date: Thu, 21 Jul 2022 16:40:14 +0200 Subject: [PATCH 10/17] fixup! docs: Improve AsRef / AsMut docs on blanket impls Changed wording in sections on "Reflexivity": replaced "that is there is" with "i.e. there would be" and removed comma before "with" Reason: "there is" somewhat contradicted the "would be" hypothetical. A slightly redundant wording has now been chosen for better clarity. The comma seemed to be superfluous. --- library/core/src/convert/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index bdd862f39c801..e2be262e311d6 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -172,8 +172,8 @@ pub const fn identity(x: T) -> T { /// /// # Reflexivity /// -/// Ideally, `AsRef` would be reflexive, that is there is an `impl AsRef for T`, with -/// [`as_ref`] simply returning its argument unchanged. +/// Ideally, `AsRef` would be reflexive, i.e. there would be an `impl AsRef for T` +/// with [`as_ref`] simply returning its argument unchanged. /// Such a blanket implementation is currently *not* provided due to technical restrictions of /// Rust's type system (it would be overlapping with another existing blanket implementation for /// `&T where T: AsRef` which allows `AsRef` to auto-dereference, see "Generic Implementations" @@ -283,8 +283,8 @@ pub trait AsRef { /// /// # Reflexivity /// -/// Ideally, `AsMut` would be reflexive, that is there is an `impl AsMut for T`, with -/// [`as_mut`] simply returning its argument unchanged. +/// Ideally, `AsMut` would be reflexive, i.e. there would be an `impl AsMut for T` +/// with [`as_mut`] simply returning its argument unchanged. /// Such a blanket implementation is currently *not* provided due to technical restrictions of /// Rust's type system (it would be overlapping with another existing blanket implementation for /// `&mut T where T: AsMut` which allows `AsMut` to auto-dereference, see "Generic From 0ae1df7dfc9cc3ecaa45b8879889f2b45eda078b Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Tue, 9 Aug 2022 07:07:29 +0000 Subject: [PATCH 11/17] Rename LinesAnyMap to LinesMap lines_any method was replaced with lines method, so it makes sense to rename this structure to match new name. Co-authored-by: Ian Jackson --- library/core/src/str/iter.rs | 4 ++-- library/core/src/str/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/str/iter.rs b/library/core/src/str/iter.rs index 24083ee6af44f..660a3b091b3d1 100644 --- a/library/core/src/str/iter.rs +++ b/library/core/src/str/iter.rs @@ -13,7 +13,7 @@ use super::from_utf8_unchecked; use super::pattern::Pattern; use super::pattern::{DoubleEndedSearcher, ReverseSearcher, Searcher}; use super::validations::{next_code_point, next_code_point_reverse}; -use super::LinesAnyMap; +use super::LinesMap; use super::{BytesIsNotEmpty, UnsafeBytesToStr}; use super::{CharEscapeDebugContinue, CharEscapeDefault, CharEscapeUnicode}; use super::{IsAsciiWhitespace, IsNotEmpty, IsWhitespace}; @@ -1091,7 +1091,7 @@ generate_pattern_iterators! { #[stable(feature = "rust1", since = "1.0.0")] #[must_use = "iterators are lazy and do nothing unless consumed"] #[derive(Clone, Debug)] -pub struct Lines<'a>(pub(super) Map, LinesAnyMap>); +pub struct Lines<'a>(pub(super) Map, LinesMap>); #[stable(feature = "rust1", since = "1.0.0")] impl<'a> Iterator for Lines<'a> { diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index ee66895a7ed2a..296fb4792eae6 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -995,7 +995,7 @@ impl str { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn lines(&self) -> Lines<'_> { - Lines(self.split_terminator('\n').map(LinesAnyMap)) + Lines(self.split_terminator('\n').map(LinesMap)) } /// An iterator over the lines of a string. @@ -2588,7 +2588,7 @@ impl Default for &mut str { impl_fn_for_zst! { /// A nameable, cloneable fn type #[derive(Clone)] - struct LinesAnyMap impl<'a> Fn = |line: &'a str| -> &'a str { + struct LinesMap impl<'a> Fn = |line: &'a str| -> &'a str { let l = line.len(); if l > 0 && line.as_bytes()[l - 1] == b'\r' { &line[0 .. l - 1] } else { line } From e5377776044ee6c45453fa8fc83d37c130397b2b Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Tue, 9 Aug 2022 07:12:15 +0000 Subject: [PATCH 12/17] Fix handling of trailing bare CR in str::lines Previously "bare\r" was split into ["bare"] even though the documentation said that only LF and CRLF count as newlines. This fix is a behavioural change, even though it brings the behaviour into line with the documentation, and into line with that of `std::io::BufRead::lines()`. This is an alternative to #91051, which proposes to document rather than fix the behaviour. Fixes #94435. Co-authored-by: Ian Jackson --- library/alloc/tests/str.rs | 26 +++++++++++++++++++------- library/core/src/lib.rs | 1 + library/core/src/str/iter.rs | 2 +- library/core/src/str/mod.rs | 8 ++++---- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/library/alloc/tests/str.rs b/library/alloc/tests/str.rs index 7379569dd68fe..193d46ed20b73 100644 --- a/library/alloc/tests/str.rs +++ b/library/alloc/tests/str.rs @@ -1499,13 +1499,25 @@ fn test_split_whitespace() { #[test] fn test_lines() { - let data = "\nMäry häd ä little lämb\n\r\nLittle lämb\n"; - let lines: Vec<&str> = data.lines().collect(); - assert_eq!(lines, ["", "Märy häd ä little lämb", "", "Little lämb"]); - - let data = "\r\nMäry häd ä little lämb\n\nLittle lämb"; // no trailing \n - let lines: Vec<&str> = data.lines().collect(); - assert_eq!(lines, ["", "Märy häd ä little lämb", "", "Little lämb"]); + fn t(data: &str, expected: &[&str]) { + let lines: Vec<&str> = data.lines().collect(); + assert_eq!(lines, expected); + } + t("", &[]); + t("\n", &[""]); + t("\n2nd", &["", "2nd"]); + t("\r\n", &[""]); + t("bare\r", &["bare\r"]); + t("bare\rcr", &["bare\rcr"]); + t("Text\n\r", &["Text", "\r"]); + t( + "\nMäry häd ä little lämb\n\r\nLittle lämb\n", + &["", "Märy häd ä little lämb", "", "Little lämb"], + ); + t( + "\r\nMäry häd ä little lämb\n\nLittle lämb", + &["", "Märy häd ä little lämb", "", "Little lämb"], + ); } #[test] diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 24742bb49b9a5..afa22b7ece3da 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -184,6 +184,7 @@ #![feature(intra_doc_pointers)] #![feature(intrinsics)] #![feature(lang_items)] +#![feature(let_else)] #![feature(link_llvm_intrinsics)] #![feature(macro_metavar_expr)] #![feature(min_specialization)] diff --git a/library/core/src/str/iter.rs b/library/core/src/str/iter.rs index 660a3b091b3d1..579f5806b1cd6 100644 --- a/library/core/src/str/iter.rs +++ b/library/core/src/str/iter.rs @@ -1091,7 +1091,7 @@ generate_pattern_iterators! { #[stable(feature = "rust1", since = "1.0.0")] #[must_use = "iterators are lazy and do nothing unless consumed"] #[derive(Clone, Debug)] -pub struct Lines<'a>(pub(super) Map, LinesMap>); +pub struct Lines<'a>(pub(super) Map, LinesMap>); #[stable(feature = "rust1", since = "1.0.0")] impl<'a> Iterator for Lines<'a> { diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index 296fb4792eae6..4cf83ffd9d2a1 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -995,7 +995,7 @@ impl str { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn lines(&self) -> Lines<'_> { - Lines(self.split_terminator('\n').map(LinesMap)) + Lines(self.split_inclusive('\n').map(LinesMap)) } /// An iterator over the lines of a string. @@ -2589,9 +2589,9 @@ impl_fn_for_zst! { /// A nameable, cloneable fn type #[derive(Clone)] struct LinesMap impl<'a> Fn = |line: &'a str| -> &'a str { - let l = line.len(); - if l > 0 && line.as_bytes()[l - 1] == b'\r' { &line[0 .. l - 1] } - else { line } + let Some(line) = line.strip_suffix('\n') else { return line }; + let Some(line) = line.strip_suffix('\r') else { return line }; + line }; #[derive(Clone)] From 698a3c679811fbde878148e742a6db591a13d89e Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 13 Aug 2022 02:08:24 +0200 Subject: [PATCH 13/17] Tweak `FpCategory` example order. --- library/core/src/num/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index f481399fdcf92..fce52a54949b6 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -936,8 +936,8 @@ impl usize { /// assert_eq!(num.classify(), FpCategory::Normal); /// assert_eq!(inf.classify(), FpCategory::Infinite); /// assert_eq!(zero.classify(), FpCategory::Zero); -/// assert_eq!(nan.classify(), FpCategory::Nan); /// assert_eq!(sub.classify(), FpCategory::Subnormal); +/// assert_eq!(nan.classify(), FpCategory::Nan); /// ``` #[derive(Copy, Clone, PartialEq, Eq, Debug)] #[stable(feature = "rust1", since = "1.0.0")] From cb86c38cdbf29a400dd48ec8450ce71b81c8dd45 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Fri, 26 Aug 2022 14:59:12 +0200 Subject: [PATCH 14/17] Fix `#[derive(Default)]` on a generic `#[default]` enum adding unnecessary `Default` bounds --- .../src/deriving/bounds.rs | 1 + .../src/deriving/clone.rs | 1 + .../src/deriving/cmp/eq.rs | 1 + .../src/deriving/cmp/ord.rs | 1 + .../src/deriving/cmp/partial_eq.rs | 1 + .../src/deriving/cmp/partial_ord.rs | 1 + .../src/deriving/debug.rs | 1 + .../src/deriving/decodable.rs | 1 + .../src/deriving/default.rs | 20 +++++++++++++++++++ .../src/deriving/encodable.rs | 1 + .../src/deriving/generic/mod.rs | 6 +++++- .../rustc_builtin_macros/src/deriving/hash.rs | 1 + 12 files changed, 35 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/bounds.rs b/compiler/rustc_builtin_macros/src/deriving/bounds.rs index 77e0b6c55a80e..7bd344467d032 100644 --- a/compiler/rustc_builtin_macros/src/deriving/bounds.rs +++ b/compiler/rustc_builtin_macros/src/deriving/bounds.rs @@ -16,6 +16,7 @@ pub fn expand_deriving_copy( let trait_def = TraitDef { span, path: path_std!(marker::Copy), + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: true, diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index c7f2d95e72f0c..fa8685f5f4e56 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/compiler/rustc_builtin_macros/src/deriving/clone.rs @@ -72,6 +72,7 @@ pub fn expand_deriving_clone( let trait_def = TraitDef { span, path: path_std!(clone::Clone), + skip_path_as_bound: false, additional_bounds: bounds, generics: Bounds::empty(), supports_unions: true, diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs index 5b556c5c9b9d1..eab67b0d354cf 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs @@ -25,6 +25,7 @@ pub fn expand_deriving_eq( let trait_def = TraitDef { span, path: path_std!(cmp::Eq), + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: true, diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs index 7262586955811..7f117981a9a2f 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs @@ -19,6 +19,7 @@ pub fn expand_deriving_ord( let trait_def = TraitDef { span, path: path_std!(cmp::Ord), + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs index 42ee65b570a2a..236cbccaf9fee 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs @@ -83,6 +83,7 @@ pub fn expand_deriving_partial_eq( let trait_def = TraitDef { span, path: path_std!(cmp::PartialEq), + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs index 516892aeda96f..4173403a1b84a 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs @@ -37,6 +37,7 @@ pub fn expand_deriving_partial_ord( let trait_def = TraitDef { span, path: path_std!(cmp::PartialOrd), + skip_path_as_bound: false, additional_bounds: vec![], generics: Bounds::empty(), supports_unions: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/debug.rs b/compiler/rustc_builtin_macros/src/deriving/debug.rs index 4af7fd8165388..2cf614ed9476c 100644 --- a/compiler/rustc_builtin_macros/src/deriving/debug.rs +++ b/compiler/rustc_builtin_macros/src/deriving/debug.rs @@ -20,6 +20,7 @@ pub fn expand_deriving_debug( let trait_def = TraitDef { span, path: path_std!(fmt::Debug), + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/decodable.rs b/compiler/rustc_builtin_macros/src/deriving/decodable.rs index 7174dbbe7ea8b..d669f616802fe 100644 --- a/compiler/rustc_builtin_macros/src/deriving/decodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/decodable.rs @@ -23,6 +23,7 @@ pub fn expand_deriving_rustc_decodable( let trait_def = TraitDef { span, path: Path::new_(vec![krate, sym::Decodable], vec![], PathKind::Global), + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index a94c8a996e642..17df9fb279ad6 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -24,6 +24,7 @@ pub fn expand_deriving_default( let trait_def = TraitDef { span, path: Path::new(vec![kw::Default, sym::Default]), + skip_path_as_bound: has_a_default_variant(item), additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: false, @@ -262,3 +263,22 @@ impl<'a, 'b> rustc_ast::visit::Visitor<'a> for DetectNonVariantDefaultAttr<'a, ' } } } + +fn has_a_default_variant(item: &Annotatable) -> bool { + struct HasDefaultAttrOnVariant { + found: bool, + } + + impl<'ast> rustc_ast::visit::Visitor<'ast> for HasDefaultAttrOnVariant { + fn visit_variant(&mut self, v: &'ast rustc_ast::Variant) { + if v.attrs.iter().any(|attr| attr.has_name(kw::Default)) { + self.found = true; + } + // no need to subrecurse. + } + } + + let mut visitor = HasDefaultAttrOnVariant { found: false }; + item.visit_with(&mut visitor); + visitor.found +} diff --git a/compiler/rustc_builtin_macros/src/deriving/encodable.rs b/compiler/rustc_builtin_macros/src/deriving/encodable.rs index b220e54238f46..f83f58b97d38f 100644 --- a/compiler/rustc_builtin_macros/src/deriving/encodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/encodable.rs @@ -107,6 +107,7 @@ pub fn expand_deriving_rustc_encodable( let trait_def = TraitDef { span, path: Path::new_(vec![krate, sym::Encodable], vec![], PathKind::Global), + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: false, diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index adffebd3fd285..cd53050c61e68 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -172,6 +172,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::Span; use std::cell::RefCell; use std::iter; +use std::ops::Not; use std::vec; use thin_vec::thin_vec; use ty::{Bounds, Path, Ref, Self_, Ty}; @@ -185,6 +186,9 @@ pub struct TraitDef<'a> { /// Path of the trait, including any type parameters pub path: Path, + /// Whether to skip adding the current trait as a bound to the type parameters of the type. + pub skip_path_as_bound: bool, + /// Additional bounds required of any type parameters of the type, /// other than the current trait pub additional_bounds: Vec, @@ -594,7 +598,7 @@ impl<'a> TraitDef<'a> { cx.trait_bound(p.to_path(cx, self.span, type_ident, generics)) }).chain( // require the current trait - iter::once(cx.trait_bound(trait_path.clone())) + self.skip_path_as_bound.not().then(|| cx.trait_bound(trait_path.clone())) ).chain( // also add in any bounds from the declaration param.bounds.iter().cloned() diff --git a/compiler/rustc_builtin_macros/src/deriving/hash.rs b/compiler/rustc_builtin_macros/src/deriving/hash.rs index f1f02e7ce7787..6e9d5f08b9443 100644 --- a/compiler/rustc_builtin_macros/src/deriving/hash.rs +++ b/compiler/rustc_builtin_macros/src/deriving/hash.rs @@ -22,6 +22,7 @@ pub fn expand_deriving_hash( let hash_trait_def = TraitDef { span, path, + skip_path_as_bound: false, additional_bounds: Vec::new(), generics: Bounds::empty(), supports_unions: false, From 975e72fc0f122e2c2b57d8d61a9c6553d29fb366 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Fri, 26 Aug 2022 15:00:38 +0200 Subject: [PATCH 15/17] Add corresponding regression test --- src/test/ui/deriving/deriving-default-enum.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/ui/deriving/deriving-default-enum.rs b/src/test/ui/deriving/deriving-default-enum.rs index d1a81c72c2fdc..1c7a501edc705 100644 --- a/src/test/ui/deriving/deriving-default-enum.rs +++ b/src/test/ui/deriving/deriving-default-enum.rs @@ -12,6 +12,16 @@ enum Foo { Beta(NotDefault), } +// #[default] on a generic enum does not add `Default` bounds to the type params. +#[derive(Default)] +enum MyOption { + #[default] + None, + #[allow(dead_code)] + Some(T), +} + fn main() { assert_eq!(Foo::default(), Foo::Alpha); + assert!(matches!(MyOption::::default(), MyOption::None)); } From 3d4980bc8d8a58df217a6b659b9353a11ce4cd29 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Thu, 15 Sep 2022 18:56:12 +0200 Subject: [PATCH 16/17] Future-proof against loose bounds if default variant is non-exhaustive. Co-Authored-By: Mark Rousskov --- src/test/ui/macros/macros-nonfatal-errors.rs | 21 +++++++++++++++++++ .../ui/macros/macros-nonfatal-errors.stderr | 12 ++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/test/ui/macros/macros-nonfatal-errors.rs b/src/test/ui/macros/macros-nonfatal-errors.rs index e7a01f105de0b..140cc5b0fd808 100644 --- a/src/test/ui/macros/macros-nonfatal-errors.rs +++ b/src/test/ui/macros/macros-nonfatal-errors.rs @@ -116,3 +116,24 @@ fn main() { trace_macros!(invalid); //~ ERROR } + +/// Check that `#[derive(Default)]` does use a `T : Default` bound when the +/// `#[default]` variant is `#[non_exhaustive]` (should this end up allowed). +const _: () = { + #[derive(Default)] + enum NonExhaustiveDefaultGeneric { + #[default] + #[non_exhaustive] + Foo, //~ ERROR default variant must be exhaustive + Bar(T), + } + + fn assert_impls_default() {} + + enum NotDefault {} + + // Note: the `derive(Default)` currently bails early enough for trait-checking + // not to happen. Should it bail late enough, or even pass, make sure to + // assert that the following line fails. + let _ = assert_impls_default::>; +}; diff --git a/src/test/ui/macros/macros-nonfatal-errors.stderr b/src/test/ui/macros/macros-nonfatal-errors.stderr index b3c6d07f96763..d42f6c179b7ef 100644 --- a/src/test/ui/macros/macros-nonfatal-errors.stderr +++ b/src/test/ui/macros/macros-nonfatal-errors.stderr @@ -215,11 +215,21 @@ error: trace_macros! accepts only `true` or `false` LL | trace_macros!(invalid); | ^^^^^^^^^^^^^^^^^^^^^^ +error: default variant must be exhaustive + --> $DIR/macros-nonfatal-errors.rs:127:9 + | +LL | #[non_exhaustive] + | ----------------- declared `#[non_exhaustive]` here +LL | Foo, + | ^^^ + | + = help: consider a manual implementation of `Default` + error: cannot find macro `llvm_asm` in this scope --> $DIR/macros-nonfatal-errors.rs:99:5 | LL | llvm_asm!(invalid); | ^^^^^^^^ -error: aborting due to 27 previous errors +error: aborting due to 28 previous errors From 591c1f25b297c446ac13211f955d06a859206f1a Mon Sep 17 00:00:00 2001 From: Akshay Date: Tue, 27 Sep 2022 11:55:13 +0530 Subject: [PATCH 17/17] introduce `{char, u8}::is_ascii_octdigit` --- library/core/src/char/methods.rs | 32 ++++++++++++++++++++++++++++++++ library/core/src/lib.rs | 1 + library/core/src/num/mod.rs | 32 ++++++++++++++++++++++++++++++++ library/core/tests/ascii.rs | 18 ++++++++++++++++++ library/core/tests/lib.rs | 1 + 5 files changed, 84 insertions(+) diff --git a/library/core/src/char/methods.rs b/library/core/src/char/methods.rs index b7a63b7c67566..224bc9effe61e 100644 --- a/library/core/src/char/methods.rs +++ b/library/core/src/char/methods.rs @@ -1444,6 +1444,38 @@ impl char { matches!(*self, '0'..='9') } + /// Checks if the value is an ASCII octal digit: + /// U+0030 '0' ..= U+0037 '7'. + /// + /// # Examples + /// + /// ``` + /// #![feature(is_ascii_octdigit)] + /// + /// let uppercase_a = 'A'; + /// let a = 'a'; + /// let zero = '0'; + /// let seven = '7'; + /// let nine = '9'; + /// let percent = '%'; + /// let lf = '\n'; + /// + /// assert!(!uppercase_a.is_ascii_octdigit()); + /// assert!(!a.is_ascii_octdigit()); + /// assert!(zero.is_ascii_octdigit()); + /// assert!(seven.is_ascii_octdigit()); + /// assert!(!nine.is_ascii_octdigit()); + /// assert!(!percent.is_ascii_octdigit()); + /// assert!(!lf.is_ascii_octdigit()); + /// ``` + #[must_use] + #[unstable(feature = "is_ascii_octdigit", issue = "101288")] + #[rustc_const_unstable(feature = "is_ascii_octdigit", issue = "101288")] + #[inline] + pub const fn is_ascii_octdigit(&self) -> bool { + matches!(*self, '0'..='7') + } + /// Checks if the value is an ASCII hexadecimal digit: /// /// - U+0030 '0' ..= U+0039 '9', or diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 5690b5256e88c..18ad0bdc8b20e 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -164,6 +164,7 @@ #![feature(const_slice_index)] #![feature(const_is_char_boundary)] #![feature(const_cstr_methods)] +#![feature(is_ascii_octdigit)] // // Language features: #![feature(abi_unadjusted)] diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index c0be235c120bf..01642a8dd31a8 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -622,6 +622,38 @@ impl u8 { matches!(*self, b'0'..=b'9') } + /// Checks if the value is an ASCII octal digit: + /// U+0030 '0' ..= U+0037 '7'. + /// + /// # Examples + /// + /// ``` + /// #![feature(is_ascii_octdigit)] + /// + /// let uppercase_a = b'A'; + /// let a = b'a'; + /// let zero = b'0'; + /// let seven = b'7'; + /// let nine = b'9'; + /// let percent = b'%'; + /// let lf = b'\n'; + /// + /// assert!(!uppercase_a.is_ascii_octdigit()); + /// assert!(!a.is_ascii_octdigit()); + /// assert!(zero.is_ascii_octdigit()); + /// assert!(seven.is_ascii_octdigit()); + /// assert!(!nine.is_ascii_octdigit()); + /// assert!(!percent.is_ascii_octdigit()); + /// assert!(!lf.is_ascii_octdigit()); + /// ``` + #[must_use] + #[unstable(feature = "is_ascii_octdigit", issue = "101288")] + #[rustc_const_unstable(feature = "is_ascii_octdigit", issue = "101288")] + #[inline] + pub const fn is_ascii_octdigit(&self) -> bool { + matches!(*self, b'0'..=b'7') + } + /// Checks if the value is an ASCII hexadecimal digit: /// /// - U+0030 '0' ..= U+0039 '9', or diff --git a/library/core/tests/ascii.rs b/library/core/tests/ascii.rs index 6d2cf3e83bce9..f5f2dd0477885 100644 --- a/library/core/tests/ascii.rs +++ b/library/core/tests/ascii.rs @@ -251,6 +251,23 @@ fn test_is_ascii_digit() { ); } +#[test] +fn test_is_ascii_octdigit() { + assert_all!(is_ascii_octdigit, "", "01234567"); + assert_none!( + is_ascii_octdigit, + "abcdefghijklmnopqrstuvwxyz", + "ABCDEFGHIJKLMNOQPRSTUVWXYZ", + "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~", + " \t\n\x0c\r", + "\x00\x01\x02\x03\x04\x05\x06\x07", + "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f", + "\x10\x11\x12\x13\x14\x15\x16\x17", + "\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f", + "\x7f", + ); +} + #[test] fn test_is_ascii_hexdigit() { assert_all!(is_ascii_hexdigit, "", "0123456789", "abcdefABCDEF",); @@ -454,6 +471,7 @@ fn ascii_ctype_const() { is_ascii_lowercase => [true, false, false, false, false]; is_ascii_alphanumeric => [true, true, true, false, false]; is_ascii_digit => [false, false, true, false, false]; + is_ascii_octdigit => [false, false, false, false, false]; is_ascii_hexdigit => [true, true, true, false, false]; is_ascii_punctuation => [false, false, false, true, false]; is_ascii_graphic => [true, true, true, true, false]; diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 46f603eaebaca..46538ddb20179 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -101,6 +101,7 @@ #![feature(slice_flatten)] #![feature(provide_any)] #![feature(utf8_chunks)] +#![feature(is_ascii_octdigit)] #![deny(unsafe_op_in_unsafe_fn)] extern crate test;