From 4be574e6c91e004c13e133ea3a441e13aa2439d3 Mon Sep 17 00:00:00 2001 From: Caio Date: Thu, 30 Sep 2021 07:49:32 -0300 Subject: [PATCH 1/6] Add 'core::array::from_fn' and 'core::array::try_from_fn' --- library/core/src/array/mod.rs | 98 ++++++++++++++++++++++++++++++++--- library/core/tests/array.rs | 81 ++++++++++++++++++++++++++++- library/core/tests/lib.rs | 1 + 3 files changed, 171 insertions(+), 9 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 8c33a43ab33..b823c33ee23 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -20,6 +20,69 @@ #[stable(feature = "array_value_iter", since = "1.51.0")] pub use iter::IntoIter; +/// Creates an array `[T; N]` where each array element `T` is returned by the `cb` call. +/// +/// # Arguments +/// +/// * `cb`: Callback where the passed argument is the current array index. +/// +/// # Example +/// +/// ```rust +/// #![feature(array_from_fn)] +/// +/// let array = core::array::from_fn(|i| i); +/// assert_eq!(array, [0, 1, 2, 3, 4]); +/// ``` +#[inline] +#[unstable(feature = "array_from_fn", issue = "89379")] +pub fn from_fn(mut cb: F) -> [T; N] +where + F: FnMut(usize) -> T, +{ + let mut idx = 0; + [(); N].map(|_| { + let res = cb(idx); + idx += 1; + res + }) +} + +/// Creates an array `[T; N]` where each fallible array element `T` is returned by the `cb` call. +/// Unlike `core::array::from_fn`, where the element creation can't fail, this version will return an error +/// if any element creation was unsuccessful. +/// +/// # Arguments +/// +/// * `cb`: Callback where the passed argument is the current array index. +/// +/// # Example +/// +/// ```rust +/// #![feature(array_from_fn)] +/// +/// #[derive(Debug, PartialEq)] +/// enum SomeError { +/// Foo, +/// } +/// +/// let array = core::array::try_from_fn(|i| Ok::<_, SomeError>(i)); +/// assert_eq!(array, Ok([0, 1, 2, 3, 4])); +/// +/// let another_array = core::array::try_from_fn::(|_| Err(SomeError::Foo)); +/// assert_eq!(another_array, Err(SomeError::Foo)); +/// ``` +#[inline] +#[unstable(feature = "array_from_fn", issue = "89379")] +pub fn try_from_fn(cb: F) -> Result<[T; N], E> +where + F: FnMut(usize) -> Result, +{ + // SAFETY: we know for certain that this iterator will yield exactly `N` + // items. + unsafe { collect_into_array_rslt_unchecked(&mut (0..N).map(cb)) } +} + /// Converts a reference to `T` into a reference to an array of length 1 (without copying). #[stable(feature = "array_from_ref", since = "1.53.0")] pub fn from_ref(s: &T) -> &[T; 1] { @@ -448,13 +511,15 @@ pub fn as_mut_slice(&mut self) -> &mut [T] { /// /// It is up to the caller to guarantee that `iter` yields at least `N` items. /// Violating this condition causes undefined behavior. -unsafe fn collect_into_array_unchecked(iter: &mut I) -> [I::Item; N] +unsafe fn collect_into_array_rslt_unchecked( + iter: &mut I, +) -> Result<[T; N], E> where // Note: `TrustedLen` here is somewhat of an experiment. This is just an // internal function, so feel free to remove if this bound turns out to be a // bad idea. In that case, remember to also remove the lower bound // `debug_assert!` below! - I: Iterator + TrustedLen, + I: Iterator> + TrustedLen, { debug_assert!(N <= iter.size_hint().1.unwrap_or(usize::MAX)); debug_assert!(N <= iter.size_hint().0); @@ -463,6 +528,18 @@ pub fn as_mut_slice(&mut self) -> &mut [T] { unsafe { collect_into_array(iter).unwrap_unchecked() } } +// Infallible version of `collect_into_array_rslt_unchecked`. +unsafe fn collect_into_array_unchecked(iter: &mut I) -> [I::Item; N] +where + I: Iterator + TrustedLen, +{ + let mut map = iter.map(|el| Ok::<_, Infallible>(el)); + + // SAFETY: Valid array elements are covered by the fact that all passed values + // to `collect_into_array` are `Ok`. + unsafe { collect_into_array_rslt_unchecked(&mut map).unwrap_unchecked() } +} + /// Pulls `N` items from `iter` and returns them as an array. If the iterator /// yields fewer than `N` items, `None` is returned and all already yielded /// items are dropped. @@ -473,13 +550,13 @@ pub fn as_mut_slice(&mut self) -> &mut [T] { /// /// If `iter.next()` panicks, all items already yielded by the iterator are /// dropped. -fn collect_into_array(iter: &mut I) -> Option<[I::Item; N]> +fn collect_into_array(iter: &mut I) -> Option> where - I: Iterator, + I: Iterator>, { if N == 0 { // SAFETY: An empty array is always inhabited and has no validity invariants. - return unsafe { Some(mem::zeroed()) }; + return unsafe { Some(Ok(mem::zeroed())) }; } struct Guard { @@ -504,7 +581,14 @@ fn drop(&mut self) { let mut guard: Guard<_, N> = Guard { ptr: MaybeUninit::slice_as_mut_ptr(&mut array), initialized: 0 }; - while let Some(item) = iter.next() { + while let Some(item_rslt) = iter.next() { + let item = match item_rslt { + Err(err) => { + return Some(Err(err)); + } + Ok(elem) => elem, + }; + // SAFETY: `guard.initialized` starts at 0, is increased by one in the // loop and the loop is aborted once it reaches N (which is // `array.len()`). @@ -520,7 +604,7 @@ fn drop(&mut self) { // SAFETY: the condition above asserts that all elements are // initialized. let out = unsafe { MaybeUninit::array_assume_init(array) }; - return Some(out); + return Some(Ok(out)); } } diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 0ae625bdb68..6889fe9a19e 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -1,5 +1,6 @@ use core::array; use core::convert::TryFrom; +use core::sync::atomic::{AtomicUsize, Ordering}; #[test] fn array_from_ref() { @@ -303,8 +304,6 @@ fn array_map() { #[test] #[should_panic(expected = "test succeeded")] fn array_map_drop_safety() { - use core::sync::atomic::AtomicUsize; - use core::sync::atomic::Ordering; static DROPPED: AtomicUsize = AtomicUsize::new(0); struct DropCounter; impl Drop for DropCounter { @@ -356,3 +355,81 @@ fn new() -> B<'a> { b3.a[0].set(Some(&b1)); b3.a[1].set(Some(&b2)); } + +#[test] +fn array_from_fn() { + let array = core::array::from_fn(|idx| idx); + assert_eq!(array, [0, 1, 2, 3, 4]); +} + +#[test] +fn array_try_from_fn() { + #[derive(Debug, PartialEq)] + enum SomeError { + Foo, + } + + let array = core::array::try_from_fn(|i| Ok::<_, SomeError>(i)); + assert_eq!(array, Ok([0, 1, 2, 3, 4])); + + let another_array = core::array::try_from_fn::(|_| Err(SomeError::Foo)); + assert_eq!(another_array, Err(SomeError::Foo)); +} + +#[test] +fn array_try_from_fn_drops_inserted_elements_on_err() { + static DROP_COUNTER: AtomicUsize = AtomicUsize::new(0); + + struct CountDrop; + impl Drop for CountDrop { + fn drop(&mut self) { + DROP_COUNTER.fetch_add(1, Ordering::SeqCst); + } + } + + let _ = catch_unwind_silent(move || { + let _: Result<[CountDrop; 4], ()> = core::array::try_from_fn(|idx| { + if idx == 2 { + return Err(()); + } + Ok(CountDrop) + }); + }); + + assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 2); +} + +#[test] +fn array_try_from_fn_drops_inserted_elements_on_panic() { + static DROP_COUNTER: AtomicUsize = AtomicUsize::new(0); + + struct CountDrop; + impl Drop for CountDrop { + fn drop(&mut self) { + DROP_COUNTER.fetch_add(1, Ordering::SeqCst); + } + } + + let _ = catch_unwind_silent(move || { + let _: Result<[CountDrop; 4], ()> = core::array::try_from_fn(|idx| { + if idx == 2 { + panic!("peek a boo"); + } + Ok(CountDrop) + }); + }); + + assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 2); +} + +// https://stackoverflow.com/a/59211505 +fn catch_unwind_silent(f: F) -> std::thread::Result +where + F: FnOnce() -> R + core::panic::UnwindSafe, +{ + let prev_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(|_| {})); + let result = std::panic::catch_unwind(f); + std::panic::set_hook(prev_hook); + result +} diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index cd3aed4cd28..12088e91840 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -26,6 +26,7 @@ #![feature(extern_types)] #![feature(flt2dec)] #![feature(fmt_internals)] +#![feature(array_from_fn)] #![feature(hashmap_internals)] #![feature(try_find)] #![feature(is_sorted)] From fdccc7dad9f9d4227ce07189585cc5214868bc28 Mon Sep 17 00:00:00 2001 From: Caio Date: Thu, 30 Sep 2021 08:40:05 -0300 Subject: [PATCH 2/6] Use reference instead of raw pointer --- library/core/src/array/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index b823c33ee23..3fa63cf968f 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -559,16 +559,17 @@ unsafe fn collect_into_array_rslt_unchecked( return unsafe { Some(Ok(mem::zeroed())) }; } - struct Guard { - ptr: *mut T, + struct Guard<'a, T, const N: usize> { + array_mut: &'a mut [MaybeUninit; N], initialized: usize, } - impl Drop for Guard { + impl Drop for Guard<'_, T, N> { fn drop(&mut self) { debug_assert!(self.initialized <= N); - let initialized_part = crate::ptr::slice_from_raw_parts_mut(self.ptr, self.initialized); + let ptr = MaybeUninit::slice_as_mut_ptr(self.array_mut); + let initialized_part = crate::ptr::slice_from_raw_parts_mut(ptr, self.initialized); // SAFETY: this raw slice will contain only initialized objects. unsafe { @@ -578,8 +579,7 @@ fn drop(&mut self) { } let mut array = MaybeUninit::uninit_array::(); - let mut guard: Guard<_, N> = - Guard { ptr: MaybeUninit::slice_as_mut_ptr(&mut array), initialized: 0 }; + let mut guard: Guard<'_, _, N> = Guard { array_mut: &mut array, initialized: 0 }; while let Some(item_rslt) = iter.next() { let item = match item_rslt { @@ -593,7 +593,7 @@ fn drop(&mut self) { // loop and the loop is aborted once it reaches N (which is // `array.len()`). unsafe { - array.get_unchecked_mut(guard.initialized).write(item); + guard.array_mut.get_unchecked_mut(guard.initialized).write(item); } guard.initialized += 1; From 325025e74b98ee3bab229ed416fa9669b9eeca71 Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Thu, 30 Sep 2021 13:53:24 +0200 Subject: [PATCH 3/6] Improve previous commit --- library/core/src/array/mod.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 3fa63cf968f..6825e663fc6 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -568,18 +568,17 @@ impl Drop for Guard<'_, T, N> { fn drop(&mut self) { debug_assert!(self.initialized <= N); - let ptr = MaybeUninit::slice_as_mut_ptr(self.array_mut); - let initialized_part = crate::ptr::slice_from_raw_parts_mut(ptr, self.initialized); - - // SAFETY: this raw slice will contain only initialized objects. + // SAFETY: this slice will contain only initialized objects. unsafe { - crate::ptr::drop_in_place(initialized_part); + crate::ptr::drop_in_place(MaybeUninit::slice_assume_init_mut( + &mut self.array_mut.get_unchecked_mut(..self.initialized), + )); } } } let mut array = MaybeUninit::uninit_array::(); - let mut guard: Guard<'_, _, N> = Guard { array_mut: &mut array, initialized: 0 }; + let mut guard = Guard { array_mut: &mut array, initialized: 0 }; while let Some(item_rslt) = iter.next() { let item = match item_rslt { From 355c7e941557fcea12971a6bf2963af04695d385 Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Thu, 30 Sep 2021 14:19:56 +0200 Subject: [PATCH 4/6] Remove an unnecessary use of unwrap_unchecked also add a new SAFETY comment and simplify/remove a closure --- library/core/src/array/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 6825e663fc6..24a1eef382b 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -533,11 +533,14 @@ unsafe fn collect_into_array_rslt_unchecked( where I: Iterator + TrustedLen, { - let mut map = iter.map(|el| Ok::<_, Infallible>(el)); + let mut map = iter.map(Ok::<_, Infallible>); - // SAFETY: Valid array elements are covered by the fact that all passed values - // to `collect_into_array` are `Ok`. - unsafe { collect_into_array_rslt_unchecked(&mut map).unwrap_unchecked() } + // SAFETY: The same safety considerations w.r.t. the iterator length + // apply for `collect_into_array_rslt_unchecked` as for + // `collect_into_array_unchecked` + match unsafe { collect_into_array_rslt_unchecked(&mut map) } { + Ok(array) => array, + } } /// Pulls `N` items from `iter` and returns them as an array. If the iterator From 91ad91efb68507d388aa7332dd458372893c2929 Mon Sep 17 00:00:00 2001 From: Caio Date: Sun, 3 Oct 2021 12:25:23 -0300 Subject: [PATCH 5/6] Skip platforms without unwinding support --- library/core/tests/array.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 6889fe9a19e..183f055baf1 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -376,6 +376,7 @@ enum SomeError { assert_eq!(another_array, Err(SomeError::Foo)); } +#[cfg(not(panic = "abort"))] #[test] fn array_try_from_fn_drops_inserted_elements_on_err() { static DROP_COUNTER: AtomicUsize = AtomicUsize::new(0); @@ -399,6 +400,7 @@ fn drop(&mut self) { assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 2); } +#[cfg(not(panic = "abort"))] #[test] fn array_try_from_fn_drops_inserted_elements_on_panic() { static DROP_COUNTER: AtomicUsize = AtomicUsize::new(0); From 85c4a52807d6137505b0d1419cc800c210d79159 Mon Sep 17 00:00:00 2001 From: Caio Date: Fri, 8 Oct 2021 06:40:24 -0300 Subject: [PATCH 6/6] Also cfg flag auxiliar function --- library/core/tests/array.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 183f055baf1..b3af1328c90 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -424,6 +424,7 @@ fn drop(&mut self) { assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 2); } +#[cfg(not(panic = "abort"))] // https://stackoverflow.com/a/59211505 fn catch_unwind_silent(f: F) -> std::thread::Result where