From 8d10e69e33c6e794758006fb48c097305de3c09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Violeta=20Hern=C3=A1ndez?= Date: Wed, 14 Jul 2021 13:24:27 -0500 Subject: [PATCH] Finally figured out some trait nitty-gritty --- nalgebra-lapack/src/schur.rs | 10 ++-- src/base/alias.rs | 1 + src/base/allocator.rs | 3 +- src/base/construction.rs | 55 +++++++++--------- src/base/conversion.rs | 19 ++++--- src/base/default_allocator.rs | 2 +- src/base/matrix.rs | 104 +++++++++++++++++----------------- src/linalg/schur.rs | 4 +- src/sparse/cs_matrix.rs | 2 + 9 files changed, 105 insertions(+), 95 deletions(-) diff --git a/nalgebra-lapack/src/schur.rs b/nalgebra-lapack/src/schur.rs index 3bee2635..35da8bec 100644 --- a/nalgebra-lapack/src/schur.rs +++ b/nalgebra-lapack/src/schur.rs @@ -153,15 +153,15 @@ where where DefaultAllocator: Allocator, D>, { - let mut out = unsafe { - OVector::new_uninitialized_generic(self.t.data.shape().0, Const::<1>).assume_init() - }; + let mut out = + unsafe { OVector::new_uninitialized_generic(self.t.data.shape().0, Const::<1>) }; for i in 0..out.len() { - out[i] = Complex::new(self.re[i], self.im[i]) + out[i] = MaybeUninit::new(Complex::new(self.re[i], self.im[i])); } - out + // Safety: all entries have been initialized. + unsafe { out.assume_init() } } } diff --git a/src/base/alias.rs b/src/base/alias.rs index 6bc04813..a1e82ac0 100644 --- a/src/base/alias.rs +++ b/src/base/alias.rs @@ -1,3 +1,4 @@ + #[cfg(any(feature = "alloc", feature = "std"))] use crate::base::dimension::Dynamic; use crate::base::dimension::{U1, U2, U3, U4, U5, U6}; diff --git a/src/base/allocator.rs b/src/base/allocator.rs index 98f34a0a..fcaae7cc 100644 --- a/src/base/allocator.rs +++ b/src/base/allocator.rs @@ -1,6 +1,5 @@ //! Abstract definition of a matrix data storage allocator. -use std::any::Any; use std::mem::MaybeUninit; use crate::base::constraint::{SameNumberOfColumns, SameNumberOfRows, ShapeConstraint}; @@ -17,7 +16,7 @@ use crate::base::DefaultAllocator; /// /// Every allocator must be both static and dynamic. Though not all implementations may share the /// same `Buffer` type. -pub trait Allocator: Any + Sized { +pub trait Allocator: 'static + Sized { /// The type of buffer this allocator can instanciate. type Buffer: ContiguousStorageMut; diff --git a/src/base/construction.rs b/src/base/construction.rs index 03bfb291..d5f29a19 100644 --- a/src/base/construction.rs +++ b/src/base/construction.rs @@ -30,16 +30,8 @@ use crate::base::{ #[macro_export] macro_rules! unimplemented_or_uninitialized_generic { ($nrows:expr, $ncols:expr) => {{ - #[cfg(feature="no_unsound_assume_init")] { - // Some of the call sites need the number of rows and columns from this to infer a type, so - // uninitialized memory is used to infer the type, as `T: Zero` isn't available at all callsites. - // This may technically still be UB even though the assume_init is dead code, but all callsites should be fixed before #556 is closed. - let typeinference_helper = crate::base::Matrix::new_uninitialized_generic($nrows, $ncols); - unimplemented!(); - typeinference_helper.assume_init() - } - #[cfg(not(feature="no_unsound_assume_init"))] { crate::base::Matrix::new_uninitialized_generic($nrows, $ncols).assume_init() } - }} + crate::base::Matrix::new_uninitialized_generic($nrows, $ncols) + }}; } /// # Generic constructors @@ -78,7 +70,7 @@ where #[inline] pub fn zeros_generic(nrows: R, ncols: C) -> Self where - T: Zero, + T: Zero + Clone, { Self::from_element_generic(nrows, ncols, T::zero()) } @@ -98,22 +90,28 @@ where /// The order of elements in the slice must follow the usual mathematic writing, i.e., /// row-by-row. #[inline] - pub fn from_row_slice_generic(nrows: R, ncols: C, slice: &[T]) -> Self { + pub fn from_row_slice_generic(nrows: R, ncols: C, slice: &[T]) -> Self + where + T: Clone, + { assert!( slice.len() == nrows.value() * ncols.value(), "Matrix init. error: the slice did not contain the right number of elements." ); - let mut res = unsafe { crate::unimplemented_or_uninitialized_generic!(nrows, ncols) }; + let mut res = Matrix::new_uninitialized_generic(nrows, ncols); let mut iter = slice.iter(); for i in 0..nrows.value() { for j in 0..ncols.value() { - unsafe { *res.get_unchecked_mut((i, j)) = iter.next().unwrap().inlined_clone() } + unsafe { + *res.get_unchecked_mut((i, j)) = MaybeUninit::new(iter.next().unwrap().clone()); + } } } - res + // Safety: all entries have been initialized. + unsafe { res.assume_init() } } /// Creates a matrix with its elements filled with the components provided by a slice. The @@ -130,15 +128,18 @@ where where F: FnMut(usize, usize) -> T, { - let mut res: Self = unsafe { crate::unimplemented_or_uninitialized_generic!(nrows, ncols) }; + let mut res = Matrix::new_uninitialized_generic(nrows, ncols); for j in 0..ncols.value() { for i in 0..nrows.value() { - unsafe { *res.get_unchecked_mut((i, j)) = f(i, j) } + unsafe { + *res.get_unchecked_mut((i, j)) = MaybeUninit::new(f(i, j)); + } } } - res + // Safety: all entries have been initialized. + unsafe { Matrix::assume_init(res) } } /// Creates a new identity matrix. @@ -160,7 +161,7 @@ where #[inline] pub fn from_diagonal_element_generic(nrows: R, ncols: C, elt: T) -> Self where - T: Zero + One, + T: Zero + One+Clone, { let mut res = Self::zeros_generic(nrows, ncols); @@ -178,7 +179,7 @@ where #[inline] pub fn from_partial_diagonal_generic(nrows: R, ncols: C, elts: &[T]) -> Self where - T: Zero, + T: Zero+Clone, { let mut res = Self::zeros_generic(nrows, ncols); assert!( @@ -187,7 +188,7 @@ where ); for (i, elt) in elts.iter().enumerate() { - unsafe { *res.get_unchecked_mut((i, i)) = elt.inlined_clone() } + unsafe { *res.get_unchecked_mut((i, i)) = elt.clone() } } res @@ -211,7 +212,7 @@ where /// ``` #[inline] pub fn from_rows(rows: &[Matrix, C, SB>]) -> Self - where + where T:Clone, SB: Storage, C>, { assert!(!rows.is_empty(), "At least one row must be given."); @@ -231,7 +232,7 @@ where // TODO: optimize that. Self::from_fn_generic(R::from_usize(nrows), C::from_usize(ncols), |i, j| { - rows[i][(0, j)].inlined_clone() + rows[i][(0, j)].clone() }) } @@ -253,7 +254,7 @@ where /// ``` #[inline] pub fn from_columns(columns: &[Vector]) -> Self - where + where T:Clone, SB: Storage, { assert!(!columns.is_empty(), "At least one column must be given."); @@ -273,7 +274,7 @@ where // TODO: optimize that. Self::from_fn_generic(R::from_usize(nrows), C::from_usize(ncols), |i, j| { - columns[j][i].inlined_clone() + columns[j][i].clone() }) } @@ -457,8 +458,8 @@ macro_rules! impl_constructors( /// ``` #[inline] pub fn zeros($($args: usize),*) -> Self - where - T: Zero + where + T: Zero + Clone { Self::zeros_generic($($gargs),*) } diff --git a/src/base/conversion.rs b/src/base/conversion.rs index 8ede11ca..97194a13 100644 --- a/src/base/conversion.rs +++ b/src/base/conversion.rs @@ -3,6 +3,7 @@ use alloc::vec::Vec; use simba::scalar::{SubsetOf, SupersetOf}; use std::borrow::{Borrow, BorrowMut}; use std::convert::{AsMut, AsRef, From, Into}; +use std::mem::MaybeUninit; use simba::simd::{PrimitiveSimdValue, SimdValue}; @@ -44,17 +45,19 @@ where let nrows2 = R2::from_usize(nrows); let ncols2 = C2::from_usize(ncols); - let mut res: OMatrix = - unsafe { crate::unimplemented_or_uninitialized_generic!(nrows2, ncols2) }; + let mut res = OMatrix::::new_uninitialized_generic(nrows2, ncols2); + for i in 0..nrows { for j in 0..ncols { unsafe { - *res.get_unchecked_mut((i, j)) = T2::from_subset(self.get_unchecked((i, j))) + *res.get_unchecked_mut((i, j)) = + MaybeUninit::new(T2::from_subset(self.get_unchecked((i, j)))); } } } - res + // Safety: all entries have been initialized. + unsafe { Matrix::assume_init(res) } } #[inline] @@ -68,16 +71,18 @@ where let nrows = R1::from_usize(nrows2); let ncols = C1::from_usize(ncols2); - let mut res: Self = unsafe { crate::unimplemented_or_uninitialized_generic!(nrows, ncols) }; + let mut res = OMatrix::new_uninitialized_generic(nrows, ncols); for i in 0..nrows2 { for j in 0..ncols2 { unsafe { - *res.get_unchecked_mut((i, j)) = m.get_unchecked((i, j)).to_subset_unchecked() + *res.get_unchecked_mut((i, j)) = + MaybeUninit::new(m.get_unchecked((i, j)).to_subset_unchecked()); } } } - res + // Safety: all entries have been initialized. + unsafe { res.assume_init() } } } diff --git a/src/base/default_allocator.rs b/src/base/default_allocator.rs index 798bdb46..041d590d 100644 --- a/src/base/default_allocator.rs +++ b/src/base/default_allocator.rs @@ -68,7 +68,7 @@ impl Allocator, Const> for Def ); // Safety: we have initialized all entries. - unsafe { Self::assume_init(res) } + unsafe { , Const>>::assume_init(res) } } } diff --git a/src/base/matrix.rs b/src/base/matrix.rs index ce4d1f6a..90f030fc 100644 --- a/src/base/matrix.rs +++ b/src/base/matrix.rs @@ -34,6 +34,10 @@ use crate::{ArrayStorage, SMatrix, SimdComplexField}; #[cfg(any(feature = "std", feature = "alloc"))] use crate::{DMatrix, DVector, Dynamic, VecStorage}; +/// An uninitialized matrix. +pub type UninitMatrix = + Matrix, R, C, >::UninitBuffer>; + /// A square matrix. pub type SquareMatrix = Matrix; @@ -347,39 +351,34 @@ impl Matrix { } } -impl Matrix +impl + Matrix, R, C, >::UninitBuffer> where - S: Storage, - DefaultAllocator: Allocator, + DefaultAllocator: Allocator, { /// Allocates a matrix with the given number of rows and columns without initializing its content. - pub fn new_uninitialized_generic( - nrows: R, - ncols: C, - ) -> Matrix, R, C, >::UninitBuffer> { - Matrix { + /// + /// Note: calling `Self::new_uninitialized_generic` is often **not** what you want to do. Consider + /// calling `Matrix::new_uninitialized_generic` instead. + pub fn new_uninitialized_generic(nrows: R, ncols: C) -> Self { + Self { data: >::allocate_uninitialized(nrows, ncols), _phantoms: PhantomData, } } } -impl Matrix, R, C, S> +impl + Matrix, R, C, >::UninitBuffer> where - S: Storage, - DefaultAllocator: Allocator, + DefaultAllocator: Allocator, { /// Assumes a matrix's entries to be initialized. This operation should be near zero-cost. pub unsafe fn assume_init( - uninit: Matrix< - MaybeUninit, - R, - C, - >::UninitBuffer, - >, - ) -> Matrix { + self, + ) -> Matrix>::Buffer> { Matrix { - data: >::assume_init(uninit.data), + data: >::assume_init(self.data), _phantoms: PhantomData, } } @@ -654,24 +653,25 @@ impl> Matrix { let nrows: SameShapeR = Dim::from_usize(nrows); let ncols: SameShapeC = Dim::from_usize(ncols); - let mut res: MatrixSum = - unsafe { crate::unimplemented_or_uninitialized_generic!(nrows, ncols) }; + let mut res = Matrix::new_uninitialized_generic(nrows, ncols); // TODO: use copy_from for j in 0..res.ncols() { for i in 0..res.nrows() { unsafe { - *res.get_unchecked_mut((i, j)) = self.get_unchecked((i, j)).clone(); + *res.get_unchecked_mut((i, j)) = + MaybeUninit::new(self.get_unchecked((i, j)).clone()); } } } - res + unsafe { Matrix::assume_init(res) } } - /// Transposes `self` and store the result into `out`. + /// Transposes `self` and store the result into `out`, which will become + /// fully initialized. #[inline] - pub fn transpose_to(&self, out: &mut Matrix) + pub fn transpose_to(&self, out: &mut Matrix, R2, C2, SB>) where T: Clone, SB: StorageMut, @@ -687,7 +687,8 @@ impl> Matrix { for i in 0..nrows { for j in 0..ncols { unsafe { - *out.get_unchecked_mut((j, i)) = self.get_unchecked((i, j)).clone(); + *out.get_unchecked_mut((j, i)) = + MaybeUninit::new(self.get_unchecked((i, j)).clone()); } } } @@ -702,17 +703,18 @@ impl> Matrix { DefaultAllocator: Allocator, { let (nrows, ncols) = self.data.shape(); + let mut res = OMatrix::new_uninitialized_generic(ncols, nrows); + self.transpose_to(&mut res); unsafe { - let mut res = crate::unimplemented_or_uninitialized_generic!(ncols, nrows); - self.transpose_to(&mut res); - - res + // Safety: res is now fully initialized due to the guarantees of transpose_to. + res.assume_init() } } } /// # Elementwise mapping and folding +// Todo: maybe make ref versions of these methods that can be used when T is expensive to clone? impl> Matrix { /// Returns a matrix containing the result of `f` applied to each of its entries. #[inline] @@ -724,19 +726,19 @@ impl> Matrix { { let (nrows, ncols) = self.data.shape(); - let mut res: OMatrix = - unsafe { crate::unimplemented_or_uninitialized_generic!(nrows, ncols) }; + let mut res = OMatrix::new_uninitialized_generic(nrows, ncols); for j in 0..ncols.value() { for i in 0..nrows.value() { unsafe { let a = self.data.get_unchecked(i, j).clone(); - *res.data.get_unchecked_mut(i, j) = f(a) + *res.data.get_unchecked_mut(i, j) = MaybeUninit::new(f(a)); } } } - res + // Safety: all entries have been initialized. + unsafe { res.assume_init() } } /// Cast the components of `self` to another type. @@ -821,8 +823,7 @@ impl> Matrix { { let (nrows, ncols) = self.data.shape(); - let mut res: OMatrix = - unsafe { crate::unimplemented_or_uninitialized_generic!(nrows, ncols) }; + let mut res = OMatrix::::new_uninitialized_generic(nrows, ncols); assert_eq!( (nrows.value(), ncols.value()), @@ -835,12 +836,13 @@ impl> Matrix { unsafe { let a = self.data.get_unchecked(i, j).clone(); let b = rhs.data.get_unchecked(i, j).clone(); - *res.data.get_unchecked_mut(i, j) = f(a, b) + *res.data.get_unchecked_mut(i, j) = MaybeUninit::new(f(a, b)); } } } - res + // Safety: all entries have been initialized. + unsafe { res.assume_init() } } /// Returns a matrix containing the result of `f` applied to each entries of `self` and @@ -862,8 +864,7 @@ impl> Matrix { { let (nrows, ncols) = self.data.shape(); - let mut res: OMatrix = - unsafe { crate::unimplemented_or_uninitialized_generic!(nrows, ncols) }; + let mut res = OMatrix::new_uninitialized_generic(nrows, ncols); assert_eq!( (nrows.value(), ncols.value()), @@ -882,12 +883,13 @@ impl> Matrix { let a = self.data.get_unchecked(i, j).clone(); let b = b.data.get_unchecked(i, j).clone(); let c = c.data.get_unchecked(i, j).clone(); - *res.data.get_unchecked_mut(i, j) = f(a, b, c) + *res.data.get_unchecked_mut(i, j) = MaybeUninit::new(f(a, b, c)); } } } - res + // Safety: all entries have been initialized. + unsafe { res.assume_init() } } /// Folds a function `f` on each entry of `self`. @@ -1322,7 +1324,7 @@ impl> Matrix { impl> Matrix { /// Takes the adjoint (aka. conjugate-transpose) of `self` and store the result into `out`. #[inline] - pub fn adjoint_to(&self, out: &mut Matrix) + pub fn adjoint_to(&self, out: &mut Matrix, R2, C2, SB>) where R2: Dim, C2: Dim, @@ -1339,7 +1341,8 @@ impl> Matrix> Matrix = - crate::unimplemented_or_uninitialized_generic!(ncols, nrows); + let mut res = OMatrix::new_uninitialized_generic(ncols, nrows); self.adjoint_to(&mut res); res @@ -1480,7 +1482,7 @@ impl> SquareMatrix { pub fn diagonal(&self) -> OVector where T: Clone, - DefaultAllocator: Allocator + Allocator, D>, + DefaultAllocator: Allocator, { self.map_diagonal(|e| e) } @@ -1493,7 +1495,7 @@ impl> SquareMatrix { pub fn map_diagonal(&self, mut f: impl FnMut(T) -> T2) -> OVector where T: Clone, - DefaultAllocator: Allocator + Allocator, D>, + DefaultAllocator: Allocator, { assert!( self.is_square(), @@ -1648,7 +1650,7 @@ impl, S: Storage> Vector { impl AbsDiffEq for Matrix where - T: AbsDiffEq, + T: AbsDiffEq, S: Storage, T::Epsilon: Copy, { @@ -1669,7 +1671,7 @@ where impl RelativeEq for Matrix where - T: RelativeEq, + T: RelativeEq, S: Storage, T::Epsilon: Copy, { @@ -1691,7 +1693,7 @@ where impl UlpsEq for Matrix where - T: UlpsEq, + T: UlpsEq, S: Storage, T::Epsilon: Copy, { diff --git a/src/linalg/schur.rs b/src/linalg/schur.rs index 1fcfcfa5..f359900d 100644 --- a/src/linalg/schur.rs +++ b/src/linalg/schur.rs @@ -297,10 +297,10 @@ where /// Computes the complex eigenvalues of the decomposed matrix. fn do_complex_eigenvalues( t: &OMatrix, - out: &mut OVector>, D>, + out: &mut OVector, D>, ) where T: RealField, - DefaultAllocator: Allocator>, D>, + DefaultAllocator: Allocator, D>, { let dim = t.nrows(); let mut m = 0; diff --git a/src/sparse/cs_matrix.rs b/src/sparse/cs_matrix.rs index cdacd044..bf2edf4e 100644 --- a/src/sparse/cs_matrix.rs +++ b/src/sparse/cs_matrix.rs @@ -263,6 +263,8 @@ where /// `nvals` possible non-zero values. pub fn new_uninitialized_generic(nrows: R, ncols: C, nvals: usize) -> Self { let mut i = Vec::with_capacity(nvals); + + //BEEP BEEP!!!! UNDEFINED BEHAVIOR ALERT!!! BEEP BEEEP!!! unsafe { i.set_len(nvals); }