From d8718183b2f3e0dacc2731b5c84fe7b0eb197a6e Mon Sep 17 00:00:00 2001 From: kadmin Date: Thu, 6 Aug 2020 07:40:06 +0000 Subject: [PATCH 1/6] Create lang item array and add map fn This creates the language item for arrays, and adds the map fn which is like map in options or iterators. It currently allocates an extra array, unfortunately. Added fixme for transmuting Fix typo Add drop guard --- library/core/src/array/mod.rs | 53 ++++++++++++++++++++++++++++++++++ src/librustc_hir/lang_items.rs | 1 + 2 files changed, 54 insertions(+) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index c0bf3833b9c33..b549cd8959f80 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -364,3 +364,56 @@ macro_rules! array_impl_default { } array_impl_default! {32, T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T} + +#[cfg(not(bootstrap))] +#[lang = "array"] +impl [T; N] { + /// Returns an array of the same size as self, with `f` applied to each element. + /// + /// # Examples + /// ``` + /// let x = [1,2,3]; + /// let y = x.map(|v| v + 1); + /// assert_eq!(y, [2,3,4]); + /// ``` + #[unstable(feature = "array_map", issue = "77777")] + fn map(self, f: F) -> [S; N] + where + F: FnMut(T) -> S, + { + use crate::mem::MaybeUninit; + struct Guard { + dst: *mut T, + curr_init: usize, + } + + impl Guard { + fn new(dst: &mut [MaybeUninit; N]) -> Self { + Guard { dst: dst as *mut _ as *mut T, curr_init: 0 } + } + } + + impl Drop for Guard { + fn drop(&mut self) { + debug_assert!(self.curr_init <= N); + + let initialized_part = + crate::ptr::slice_from_raw_parts_mut(self.dst, self.curr_init); + // SAFETY: this raw slice will contain only initialized objects + // that's why, it is allowed to drop it. + unsafe { + crate::ptr::drop_in_place(initialized_part); + } + } + } + let dst = MaybeUninit::uninit_array::(); + let mut guard = Guard::new(&mut dst); + for (i, e) in self.into_iter().enumerate() { + dst[i] = MaybeUninit::new(f(e)); + guard.curr_init += 1; + } + // FIXME convert to crate::mem::transmute when works with generics + // unsafe { crate::mem::transmute::<[MaybeUninit; N], [S; N]>(dst) } + unsafe { (&mut dst as *mut _ as *mut [S; N]).read() } + } +} diff --git a/src/librustc_hir/lang_items.rs b/src/librustc_hir/lang_items.rs index 7f473a458481f..2f7edeb405ffe 100644 --- a/src/librustc_hir/lang_items.rs +++ b/src/librustc_hir/lang_items.rs @@ -165,6 +165,7 @@ language_item_table! { BoolImplItem, sym::bool, bool_impl, Target::Impl; CharImplItem, sym::char, char_impl, Target::Impl; StrImplItem, sym::str, str_impl, Target::Impl; + ArrayImplItem, sym::array, array_impl, Target::Impl; SliceImplItem, sym::slice, slice_impl, Target::Impl; SliceU8ImplItem, sym::slice_u8, slice_u8_impl, Target::Impl; StrAllocImplItem, sym::str_alloc, str_alloc_impl, Target::Impl; From f6411e4c666248d7a74413da03f31b761cf8f66f Mon Sep 17 00:00:00 2001 From: kadmin Date: Thu, 6 Aug 2020 22:08:56 +0000 Subject: [PATCH 2/6] Add Array Impl Lang Item in various places Add basic test And also run fmt which is where the other changes are from Fix mut issues These only appear when running tests, so resolved by adding mut Swap order of forget Add pub and rm guard impl Add explicit type to guard Add safety note Change guard type from T to S It should never have been T, as it guards over [MaybeUninit; N] Also add feature to test --- library/core/src/array/mod.rs | 17 +++++++---------- library/core/tests/array.rs | 7 +++++++ library/core/tests/lib.rs | 1 + src/librustc_typeck/check/method/probe.rs | 4 ++++ src/librustc_typeck/coherence/inherent_impls.rs | 10 ++++++++++ src/librustdoc/clean/utils.rs | 2 +- src/librustdoc/passes/collect_trait_impls.rs | 1 + 7 files changed, 31 insertions(+), 11 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index b549cd8959f80..f85f5efbb9a3c 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -377,7 +377,7 @@ impl [T; N] { /// assert_eq!(y, [2,3,4]); /// ``` #[unstable(feature = "array_map", issue = "77777")] - fn map(self, f: F) -> [S; N] + pub fn map(self, mut f: F) -> [S; N] where F: FnMut(T) -> S, { @@ -387,12 +387,6 @@ impl [T; N] { curr_init: usize, } - impl Guard { - fn new(dst: &mut [MaybeUninit; N]) -> Self { - Guard { dst: dst as *mut _ as *mut T, curr_init: 0 } - } - } - impl Drop for Guard { fn drop(&mut self) { debug_assert!(self.curr_init <= N); @@ -406,14 +400,17 @@ impl [T; N] { } } } - let dst = MaybeUninit::uninit_array::(); - let mut guard = Guard::new(&mut dst); - for (i, e) in self.into_iter().enumerate() { + let mut dst = MaybeUninit::uninit_array::(); + let mut guard: Guard = Guard { dst: &mut dst as *mut _ as *mut S, curr_init: 0 }; + for (i, e) in IntoIter::new(self).enumerate() { dst[i] = MaybeUninit::new(f(e)); guard.curr_init += 1; } // FIXME convert to crate::mem::transmute when works with generics // unsafe { crate::mem::transmute::<[MaybeUninit; N], [S; N]>(dst) } + crate::mem::forget(guard); + // SAFETY: At this point we've properly initialized the whole array + // and we just need to cast it to the correct type unsafe { (&mut dst as *mut _ as *mut [S; N]).read() } } } diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 4bc44e98fc802..5ae622c1182e9 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -290,3 +290,10 @@ fn empty_array_is_always_default() { let _arr = <[DoesNotImplDefault; 0]>::default(); } + +#[test] +fn array_map() { + let a = [1, 2, 3]; + let b = a.map(|v| v + 1); + assert_eq!(b, [2, 3, 4]); +} diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index b4c299d390586..904e3f7284049 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -1,5 +1,6 @@ #![feature(alloc_layout_extra)] #![feature(array_chunks)] +#![feature(array_map)] #![feature(bool_to_option)] #![feature(bound_cloned)] #![feature(box_syntax)] diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 106df847a05cf..9078dc40041aa 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -649,6 +649,10 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { self.assemble_inherent_impl_for_primitive(lang_def_id); } } + ty::Array(_, _) => { + let lang_def_id = lang_items.array_impl(); + self.assemble_inherent_impl_for_primitive(lang_def_id); + } ty::RawPtr(ty::TypeAndMut { ty: _, mutbl }) => { let (lang_def_id1, lang_def_id2) = match mutbl { hir::Mutability::Not => { diff --git a/src/librustc_typeck/coherence/inherent_impls.rs b/src/librustc_typeck/coherence/inherent_impls.rs index 93ee87f6c572e..cd7429f166f26 100644 --- a/src/librustc_typeck/coherence/inherent_impls.rs +++ b/src/librustc_typeck/coherence/inherent_impls.rs @@ -112,6 +112,16 @@ impl ItemLikeVisitor<'v> for InherentCollect<'tcx> { item.span, ); } + ty::Array(_, _) => { + self.check_primitive_impl( + def_id, + lang_items.array_impl(), + None, + "array", + "[T; N]", + item.span, + ); + } ty::RawPtr(ty::TypeAndMut { ty: inner, mutbl: hir::Mutability::Not }) if matches!(inner.kind, ty::Slice(_)) => { diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index c22538f21f69f..168967cdeb7cf 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -388,7 +388,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V Bool => tcx.lang_items().bool_impl(), Str => tcx.lang_items().str_impl(), Slice => tcx.lang_items().slice_impl(), - Array => tcx.lang_items().slice_impl(), + Array => tcx.lang_items().array_impl(), Tuple => None, Unit => None, RawPointer => tcx.lang_items().const_ptr_impl(), diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 3000afde0c25d..a40b45f9a7e2c 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -55,6 +55,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { lang_items.bool_impl(), lang_items.char_impl(), lang_items.str_impl(), + lang_items.array_impl(), lang_items.slice_impl(), lang_items.slice_u8_impl(), lang_items.str_alloc_impl(), From 56a651ca15ac2d3f74259b8df8ac07bd80dbeaf8 Mon Sep 17 00:00:00 2001 From: kadmin Date: Thu, 6 Aug 2020 23:36:50 +0000 Subject: [PATCH 3/6] Add recommend changes to array Switch from indexing to zip, and also use `write` on `MaybeUninit`. Add array_map feature to core/src/lib Attempt to fix issue of no such feature Update w/ pickfire's review This changes a couple of names around, adds another small test of variable size, and hides the rustdoc #![feature(..)]. Fmt doctest Add suggestions from lcnr --- library/core/src/array/mod.rs | 31 ++++++++++++++++--------------- library/core/src/lib.rs | 1 + library/core/tests/array.rs | 4 ++++ 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index f85f5efbb9a3c..16baef137cbe2 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -372,27 +372,28 @@ impl [T; N] { /// /// # Examples /// ``` - /// let x = [1,2,3]; + /// # #![feature(array_map)] + /// let x = [1, 2, 3]; /// let y = x.map(|v| v + 1); - /// assert_eq!(y, [2,3,4]); + /// assert_eq!(y, [2, 3, 4]); /// ``` #[unstable(feature = "array_map", issue = "77777")] - pub fn map(self, mut f: F) -> [S; N] + pub fn map(self, mut f: F) -> [U; N] where - F: FnMut(T) -> S, + F: FnMut(T) -> U, { use crate::mem::MaybeUninit; struct Guard { dst: *mut T, - curr_init: usize, + initialized: usize, } impl Drop for Guard { fn drop(&mut self) { - debug_assert!(self.curr_init <= N); + debug_assert!(self.initialized <= N); let initialized_part = - crate::ptr::slice_from_raw_parts_mut(self.dst, self.curr_init); + crate::ptr::slice_from_raw_parts_mut(self.dst, self.initialized); // SAFETY: this raw slice will contain only initialized objects // that's why, it is allowed to drop it. unsafe { @@ -401,16 +402,16 @@ impl [T; N] { } } let mut dst = MaybeUninit::uninit_array::(); - let mut guard: Guard = Guard { dst: &mut dst as *mut _ as *mut S, curr_init: 0 }; - for (i, e) in IntoIter::new(self).enumerate() { - dst[i] = MaybeUninit::new(f(e)); - guard.curr_init += 1; + let mut guard: Guard = Guard { dst: &mut dst as *mut _ as *mut U, initialized: 0 }; + for (src, dst) in IntoIter::new(self).zip(&mut dst) { + dst.write(f(src)); + guard.initialized += 1; } - // FIXME convert to crate::mem::transmute when works with generics - // unsafe { crate::mem::transmute::<[MaybeUninit; N], [S; N]>(dst) } + // FIXME: Convert to crate::mem::transmute once it works with generics. + // unsafe { crate::mem::transmute::<[MaybeUninit; N], [U; N]>(dst) } crate::mem::forget(guard); // SAFETY: At this point we've properly initialized the whole array - // and we just need to cast it to the correct type - unsafe { (&mut dst as *mut _ as *mut [S; N]).read() } + // and we just need to cast it to the correct type. + unsafe { (&mut dst as *mut _ as *mut [U; N]).read() } } } diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index fcf5454308b47..763457d485da4 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -145,6 +145,7 @@ #![feature(abi_unadjusted)] #![feature(adx_target_feature)] #![feature(maybe_uninit_slice)] +#![feature(maybe_uninit_extra)] #![feature(external_doc)] #![feature(associated_type_bounds)] #![feature(const_caller_location)] diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 5ae622c1182e9..d4a9b061d851e 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -296,4 +296,8 @@ fn array_map() { let a = [1, 2, 3]; let b = a.map(|v| v + 1); assert_eq!(b, [2, 3, 4]); + + let a = [1u8, 2, 3]; + let b = a.map(|v| v as u64); + assert_eq!(b, [1, 2, 3]); } From 54b821ebc0a2f3fa0587ab42df16c99ee4bf6787 Mon Sep 17 00:00:00 2001 From: kadmin Date: Fri, 7 Aug 2020 06:00:52 +0000 Subject: [PATCH 4/6] Add tracking issue #75243 Add note & example about iter order Add doc changes Update doc comments --- library/core/src/array/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 16baef137cbe2..01bf9d5ef1e80 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -368,16 +368,23 @@ array_impl_default! {32, T T T T T T T T T T T T T T T T T T T T T T T T T T T T #[cfg(not(bootstrap))] #[lang = "array"] impl [T; N] { - /// Returns an array of the same size as self, with `f` applied to each element. + /// Returns an array of the same size as `self`, with function `f` applied to each element. + /// The closure will be called on elements 0 up to but excluding N. /// /// # Examples + /// /// ``` /// # #![feature(array_map)] /// let x = [1, 2, 3]; /// let y = x.map(|v| v + 1); /// assert_eq!(y, [2, 3, 4]); + /// + /// let x = [1, 2, 3]; + /// let mut temp = 0; + /// let y = x.map(|v| { temp += 1; v * temp }); + /// assert_eq!(y, [1, 4, 9]); /// ``` - #[unstable(feature = "array_map", issue = "77777")] + #[unstable(feature = "array_map", issue = "75243")] pub fn map(self, mut f: F) -> [U; N] where F: FnMut(T) -> U, From 412417d807ec3c0999e281f21e63cf3537c73803 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 8 Aug 2020 23:20:28 +0000 Subject: [PATCH 5/6] Rm hiding feature gate & add 1 more example Update order docs for `map` --- library/core/src/array/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 01bf9d5ef1e80..cf3b9f8cce277 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -368,13 +368,13 @@ array_impl_default! {32, T T T T T T T T T T T T T T T T T T T T T T T T T T T T #[cfg(not(bootstrap))] #[lang = "array"] impl [T; N] { - /// Returns an array of the same size as `self`, with function `f` applied to each element. - /// The closure will be called on elements 0 up to but excluding N. + /// Returns an array of the same size as `self`, with function `f` applied to each element + /// in order. /// /// # Examples /// /// ``` - /// # #![feature(array_map)] + /// #![feature(array_map)] /// let x = [1, 2, 3]; /// let y = x.map(|v| v + 1); /// assert_eq!(y, [2, 3, 4]); @@ -383,6 +383,10 @@ impl [T; N] { /// let mut temp = 0; /// let y = x.map(|v| { temp += 1; v * temp }); /// assert_eq!(y, [1, 4, 9]); + /// + /// let x = ["Ferris", "Bueller's", "Day", "Off"]; + /// let y = x.map(|v| v.len()); + /// assert_eq!(y, [6, 9, 3, 3]); /// ``` #[unstable(feature = "array_map", issue = "75243")] pub fn map(self, mut f: F) -> [U; N] From af32db21c8e1b1970886eb64b9955f9590e4c445 Mon Sep 17 00:00:00 2001 From: kadmin Date: Tue, 11 Aug 2020 18:39:12 +0000 Subject: [PATCH 6/6] Add drop check test & MaybeUninit::first_ptr_mut Also in drop check test add hacky workaround for platforms that don't support panic=unwind --- library/core/src/array/mod.rs | 5 +++-- library/core/tests/array.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index cf3b9f8cce277..6b28ab7d75563 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -413,7 +413,8 @@ impl [T; N] { } } let mut dst = MaybeUninit::uninit_array::(); - let mut guard: Guard = Guard { dst: &mut dst as *mut _ as *mut U, initialized: 0 }; + let mut guard: Guard = + Guard { dst: MaybeUninit::first_ptr_mut(&mut dst), initialized: 0 }; for (src, dst) in IntoIter::new(self).zip(&mut dst) { dst.write(f(src)); guard.initialized += 1; @@ -423,6 +424,6 @@ impl [T; N] { crate::mem::forget(guard); // SAFETY: At this point we've properly initialized the whole array // and we just need to cast it to the correct type. - unsafe { (&mut dst as *mut _ as *mut [U; N]).read() } + unsafe { crate::mem::transmute_copy::<_, [U; N]>(&dst) } } } diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index d4a9b061d851e..5aba1a5d958d1 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -301,3 +301,32 @@ fn array_map() { let b = a.map(|v| v as u64); assert_eq!(b, [1, 2, 3]); } + +// See note on above test for why `should_panic` is used. +#[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 { + fn drop(&mut self) { + DROPPED.fetch_add(1, Ordering::SeqCst); + } + } + + let num_to_create = 5; + let success = std::panic::catch_unwind(|| { + let items = [0; 10]; + let mut nth = 0; + items.map(|_| { + assert!(nth < num_to_create); + nth += 1; + DropCounter + }); + }); + assert!(success.is_err()); + assert_eq!(DROPPED.load(Ordering::SeqCst), num_to_create); + panic!("test succeeded") +}