From 6ff33cd8c255f360723f3ca427f4f899b9a91846 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Thu, 21 Oct 2021 12:54:37 +0200 Subject: [PATCH] Replaced the decode implementation of [T; N] with the implementation from core --- src/de/impl_core.rs | 186 ++++++++++++++++++++++++++++++++++++++++++++ src/de/impls.rs | 59 ++------------ src/de/mod.rs | 1 + 3 files changed, 192 insertions(+), 54 deletions(-) create mode 100644 src/de/impl_core.rs diff --git a/src/de/impl_core.rs b/src/de/impl_core.rs new file mode 100644 index 0000000..a5dc4c4 --- /dev/null +++ b/src/de/impl_core.rs @@ -0,0 +1,186 @@ +#![allow(unused_unsafe)] + +//! Contains implementations for rust core that have not been stabilized +//! +//! Functions in this are expected to be properly peer reviewed by the community +//! +//! Any modifications done are purely to make the code compatible with bincode + +use core::mem::{self, MaybeUninit}; + +/// 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. +/// +/// Since the iterator is passed as a mutable reference and this function calls +/// `next` at most `N` times, the iterator can still be used afterwards to +/// retrieve the remaining items. +/// +/// If `iter.next()` panicks, all items already yielded by the iterator are +/// dropped. +#[allow(clippy::while_let_on_iterator)] +pub fn collect_into_array(iter: &mut I) -> Option> +where + I: Iterator>, +{ + if N == 0 { + // SAFETY: An empty array is always inhabited and has no validity invariants. + return unsafe { Some(Ok(mem::zeroed())) }; + } + + struct Guard<'a, T, const N: usize> { + array_mut: &'a mut [MaybeUninit; N], + initialized: usize, + } + + impl Drop for Guard<'_, T, N> { + fn drop(&mut self) { + debug_assert!(self.initialized <= N); + + // SAFETY: this slice will contain only initialized objects. + unsafe { + core::ptr::drop_in_place(slice_assume_init_mut( + &mut self.array_mut.get_unchecked_mut(..self.initialized), + )); + } + } + } + + let mut array = uninit_array::(); + let mut guard = Guard { + array_mut: &mut array, + initialized: 0, + }; + + 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()`). + unsafe { + guard + .array_mut + .get_unchecked_mut(guard.initialized) + .write(item); + } + guard.initialized += 1; + + // Check if the whole array was initialized. + if guard.initialized == N { + mem::forget(guard); + + // SAFETY: the condition above asserts that all elements are + // initialized. + let out = unsafe { array_assume_init(array) }; + return Some(Ok(out)); + } + } + + // This is only reached if the iterator is exhausted before + // `guard.initialized` reaches `N`. Also note that `guard` is dropped here, + // dropping all already initialized elements. + None +} + +/// Assuming all the elements are initialized, get a mutable slice to them. +/// +/// # Safety +/// +/// It is up to the caller to guarantee that the `MaybeUninit` elements +/// really are in an initialized state. +/// Calling this when the content is not yet fully initialized causes undefined behavior. +/// +/// See [`assume_init_mut`] for more details and examples. +/// +/// [`assume_init_mut`]: MaybeUninit::assume_init_mut +// #[unstable(feature = "maybe_uninit_slice", issue = "63569")] +// #[rustc_const_unstable(feature = "const_maybe_uninit_assume_init", issue = "none")] +#[inline(always)] +pub unsafe fn slice_assume_init_mut(slice: &mut [MaybeUninit]) -> &mut [T] { + // SAFETY: similar to safety notes for `slice_get_ref`, but we have a + // mutable reference which is also guaranteed to be valid for writes. + unsafe { &mut *(slice as *mut [MaybeUninit] as *mut [T]) } +} + +/// Create a new array of `MaybeUninit` items, in an uninitialized state. +/// +/// Note: in a future Rust version this method may become unnecessary +/// when Rust allows +/// [inline const expressions](https://github.com/rust-lang/rust/issues/76001). +/// The example below could then use `let mut buf = [const { MaybeUninit::::uninit() }; 32];`. +/// +/// # Examples +/// +/// ```ignore +/// #![feature(maybe_uninit_uninit_array, maybe_uninit_extra, maybe_uninit_slice)] +/// +/// use std::mem::MaybeUninit; +/// +/// extern "C" { +/// fn read_into_buffer(ptr: *mut u8, max_len: usize) -> usize; +/// } +/// +/// /// Returns a (possibly smaller) slice of data that was actually read +/// fn read(buf: &mut [MaybeUninit]) -> &[u8] { +/// unsafe { +/// let len = read_into_buffer(buf.as_mut_ptr() as *mut u8, buf.len()); +/// MaybeUninit::slice_assume_init_ref(&buf[..len]) +/// } +/// } +/// +/// let mut buf: [MaybeUninit; 32] = MaybeUninit::uninit_array(); +/// let data = read(&mut buf); +/// ``` +// #[unstable(feature = "maybe_uninit_uninit_array", issue = "none")] +// #[rustc_const_unstable(feature = "maybe_uninit_uninit_array", issue = "none")] +#[inline(always)] +fn uninit_array() -> [MaybeUninit; LEN] { + // SAFETY: An uninitialized `[MaybeUninit<_>; LEN]` is valid. + unsafe { MaybeUninit::<[MaybeUninit; LEN]>::uninit().assume_init() } +} + +/// Extracts the values from an array of `MaybeUninit` containers. +/// +/// # Safety +/// +/// It is up to the caller to guarantee that all elements of the array are +/// in an initialized state. +/// +/// # Examples +/// +/// ```ignore +/// #![feature(maybe_uninit_uninit_array)] +/// #![feature(maybe_uninit_array_assume_init)] +/// use std::mem::MaybeUninit; +/// +/// let mut array: [MaybeUninit; 3] = MaybeUninit::uninit_array(); +/// array[0].write(0); +/// array[1].write(1); +/// array[2].write(2); +/// +/// // SAFETY: Now safe as we initialised all elements +/// let array = unsafe { +/// MaybeUninit::array_assume_init(array) +/// }; +/// +/// assert_eq!(array, [0, 1, 2]); +/// ``` +// #[unstable(feature = "maybe_uninit_array_assume_init", issue = "80908")] +#[inline(always)] +pub unsafe fn array_assume_init(array: [MaybeUninit; N]) -> [T; N] { + // SAFETY: + // * The caller guarantees that all elements of the array are initialized + // * `MaybeUninit` and T are guaranteed to have the same layout + // * `MaybeUninit` does not drop, so there are no double-frees + // And thus the conversion is safe + unsafe { + // intrinsics::assert_inhabited::<[T; N]>(); + (&array as *const _ as *const [T; N]).read() + } +} diff --git a/src/de/impls.rs b/src/de/impls.rs index 99d1c10..9a738ed 100644 --- a/src/de/impls.rs +++ b/src/de/impls.rs @@ -397,61 +397,12 @@ where } } - // Safety: this is taken from the example of https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html - // and is therefor assumed to be safe + let result = + super::impl_core::collect_into_array(&mut (0..N).map(|_| T::decode(&mut decoder))); - use core::mem::{ManuallyDrop, MaybeUninit}; - - // Create an uninitialized array of `MaybeUninit`. The `assume_init` is - // safe because the type we are claiming to have initialized here is a - // bunch of `MaybeUninit`s, which do not require initialization. - let mut data: [MaybeUninit; N] = unsafe { MaybeUninit::uninit().assume_init() }; - - for (idx, elem) in data.iter_mut().enumerate() { - match T::decode(&mut decoder) { - Ok(t) => { - elem.write(t); - } - Err(e) => { - // We encountered an error, clean up all previous entries - - unsafe { - // Safety: we know we've written up to `idx` items - for item in &mut data[..idx] { - // Safety: We know the `item` is written with a valid value of `T`. - // And we know that `&mut item` won't be used any more after this. - ManuallyDrop::drop(&mut *(item as *mut _ as *mut ManuallyDrop)); - } - // make sure `data` isn't dropped twice - core::mem::forget(data); - } - - return Err(e); - } - } - } - - // Everything is initialized. Transmute the array to the - // initialized type. - - // BlockedTODO: https://github.com/rust-lang/rust/issues/61956 - // This does not work at the moment: - // let result = unsafe { transmute::<_, [T; N]>(data) }; - - // Alternatively we can use this: - // BlockedTODO: https://github.com/rust-lang/rust/issues/80908 - - // Const generics don't work well with transmute at the moment - // The first issue above recommends doing the following: - let ptr = &mut data as *mut _ as *mut [T; N]; - - // Safety: we know this is an array with every value written, - // and that the array is valid. - // As well as the original `data` that will be forgotten so we won't get - // multiple (mutable) references to the same memory - let res = unsafe { ptr.read() }; - core::mem::forget(data); - Ok(res) + // result is only None if N does not match the values of `(0..N)`, which it always should + // So this unsafe should never occur + result.unwrap() } } diff --git a/src/de/mod.rs b/src/de/mod.rs index cce0d0f..14a4e32 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -3,6 +3,7 @@ use crate::{config::Config, error::DecodeError}; mod decoder; +mod impl_core; mod impl_tuples; mod impls;