From 7e1b2f81b30ad35f02eaeeb7f0b6c5c13b86e97d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Violeta=20Hern=C3=A1ndez?= Date: Sat, 17 Jul 2021 13:53:01 -0500 Subject: [PATCH] Fixed some more blatant issues --- src/base/edition.rs | 3 ++- src/base/matrix.rs | 46 +++++++++++++++++++---------------------- src/base/statistics.rs | 1 - src/linalg/pow.rs | 17 ++++++++------- src/sparse/cs_matrix.rs | 4 +++- 5 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/base/edition.rs b/src/base/edition.rs index 4e11bb26..9919cda3 100644 --- a/src/base/edition.rs +++ b/src/base/edition.rs @@ -942,7 +942,8 @@ impl OMatrix { where DefaultAllocator: Reallocator, { - // BEEEP!!!! BEEEEEEEP!!! + // IMPORTANT TODO: this method is still UB, and we should decide how to + // update the API to take it into account. let placeholder = unsafe { Matrix::new_uninitialized_generic(Dynamic::new(0), Dynamic::new(0)).assume_init() diff --git a/src/base/matrix.rs b/src/base/matrix.rs index 62f0e771..6ef2c162 100644 --- a/src/base/matrix.rs +++ b/src/base/matrix.rs @@ -5,7 +5,7 @@ use std::io::{Result as IOResult, Write}; use approx::{AbsDiffEq, RelativeEq, UlpsEq}; use std::any::TypeId; use std::cmp::Ordering; -use std::fmt; +use std::fmt;use std::ptr; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; use std::mem::{self, ManuallyDrop, MaybeUninit}; @@ -341,6 +341,7 @@ impl Matrix { } } +/// # Memory manipulation methods. impl OMatrix where DefaultAllocator: Allocator, @@ -365,6 +366,7 @@ where } } +/// # More memory manipulation methods. impl OMatrix, R, C> where DefaultAllocator: Allocator, @@ -377,6 +379,18 @@ where >::assume_init(self.data), ) } + + /// Assumes a matrix's entries to be initialized, and drops them. This allows the + /// buffer to be safely reused. + pub fn reinitialize(&mut self) { + for i in 0..self.nrows() { + for j in 0..self.ncols() { + unsafe { + ptr::drop_in_place(self.get_unchecked_mut((i, j))); + } + } + } + } } impl Matrix, R, C, S> { @@ -447,21 +461,6 @@ impl> Matrix { unsafe { Self::from_data_statically_unchecked(data) } } - /// Creates a new uninitialized matrix with the given uninitialized data - pub unsafe fn from_uninitialized_data(data: MaybeUninit) -> MaybeUninit { - // BEEP BEEP this doesn't seem good - let res: Matrix> = Matrix { - data, - _phantoms: PhantomData, - }; - let res: MaybeUninit>> = MaybeUninit::new(res); - // safety: since we wrap the inner MaybeUninit in an outer MaybeUninit above, the fact that the `data` field is partially-uninitialized is still opaque. - // with s/transmute_copy/transmute/, rustc claims that `MaybeUninit>>` may be of a different size from `MaybeUninit>` - // but MaybeUninit's documentation says "MaybeUninit is guaranteed to have the same size, alignment, and ABI as T", which implies those types should be the same size - let res: MaybeUninit> = mem::transmute_copy(&res); - res - } - /// The shape of this matrix returned as the tuple (number of rows, number of columns). /// /// # Examples: @@ -941,24 +940,22 @@ impl> Matrix { /// Folds a function `f` on each entry of `self`. #[inline] #[must_use] - pub fn fold(&self, init: Acc, mut f: impl FnMut(Acc, T) -> Acc) -> Acc + pub fn fold(&self, mut init: Acc, mut f: impl FnMut(Acc, T) -> Acc) -> Acc where T: Clone, { let (nrows, ncols) = self.data.shape(); - let mut res = init; - for j in 0..ncols.value() { for i in 0..nrows.value() { unsafe { let a = self.data.get_unchecked(i, j).clone(); - res = f(res, a) + init = f(init, a) } } } - res + init } /// Folds a function `f` on each pairs of entries from `self` and `rhs`. @@ -967,7 +964,7 @@ impl> Matrix { pub fn zip_fold( &self, rhs: &Matrix, - init: Acc, + mut init: Acc, mut f: impl FnMut(Acc, T, T2) -> Acc, ) -> Acc where @@ -976,7 +973,6 @@ impl> Matrix { ShapeConstraint: SameNumberOfRows + SameNumberOfColumns, { let (nrows, ncols) = self.data.shape(); - let mut res = init; assert_eq!( (nrows.value(), ncols.value()), @@ -989,12 +985,12 @@ impl> Matrix { unsafe { let a = self.data.get_unchecked(i, j).clone(); let b = rhs.data.get_unchecked(i, j).clone(); - res = f(res, a, b) + init = f(init, a, b) } } } - res + init } /// Replaces each component of `self` by the result of a closure `f` applied on it. diff --git a/src/base/statistics.rs b/src/base/statistics.rs index 88f9236a..d0f96179 100644 --- a/src/base/statistics.rs +++ b/src/base/statistics.rs @@ -59,7 +59,6 @@ impl> Matrix { } /// Returns a column vector resulting from the folding of `f` on each column of this matrix. - // BEEEEP!!!! Pretty sure there's something fishy here. #[inline] #[must_use] pub fn compress_columns( diff --git a/src/linalg/pow.rs b/src/linalg/pow.rs index 68eb9682..cb2115ad 100644 --- a/src/linalg/pow.rs +++ b/src/linalg/pow.rs @@ -42,23 +42,24 @@ where // extra allocations. let (nrows, ncols) = self.data.shape(); let mut multiplier = self.clone_owned(); - - // TODO: ACTUALLY MAKE BUF USEFUL! BEEEEEEEEP!! + let mut buf = Matrix::new_uninitialized_generic(nrows, ncols); // Exponentiation by squares. loop { if e % two == one { - let mut buf = Matrix::new_uninitialized_generic(nrows, ncols); self.mul_to(&multiplier, &mut buf); - let buf = unsafe { buf.assume_init() }; - self.copy_from(&buf); + unsafe { + self.copy_from(&buf.assume_init_ref()); + } + buf.reinitialize(); } e /= two; - let mut buf = Matrix::new_uninitialized_generic(nrows, ncols); multiplier.mul_to(&multiplier, &mut buf); - let buf = unsafe { buf.assume_init() }; - multiplier.copy_from(&buf); + unsafe { + multiplier.copy_from(&buf.assume_init_ref()); + } + buf.reinitialize(); if e == zero { return true; diff --git a/src/sparse/cs_matrix.rs b/src/sparse/cs_matrix.rs index d59b2438..b33a3cdd 100644 --- a/src/sparse/cs_matrix.rs +++ b/src/sparse/cs_matrix.rs @@ -264,7 +264,9 @@ where 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!!! + // IMPORTANT TODO: this method is still UB, and we should decide how to + // update the API to take it into account. + unsafe { i.set_len(nvals); }