From 78b8fa7aeed974a5b3a7c19efecc4dd4244949d2 Mon Sep 17 00:00:00 2001 From: Eric Berquist Date: Sun, 11 Aug 2019 12:35:00 -0400 Subject: [PATCH 1/7] Switch ArrayBase.ptr to be NonNull --- src/arraytraits.rs | 2 +- src/data_traits.rs | 5 +++-- src/impl_clone.rs | 12 ++++++++--- src/impl_constructors.rs | 4 +++- src/impl_methods.rs | 43 ++++++++++++++++++++++------------------ src/impl_owned_array.rs | 2 +- src/impl_raw_views.rs | 24 ++++++++++++---------- src/impl_views.rs | 21 ++++++++++---------- src/iterators/mod.rs | 4 ++-- src/iterators/windows.rs | 2 +- src/lib.rs | 4 ++-- src/zip/mod.rs | 6 +++--- 12 files changed, 73 insertions(+), 56 deletions(-) diff --git a/src/arraytraits.rs b/src/arraytraits.rs index 9a1133a7b..143bb5faf 100644 --- a/src/arraytraits.rs +++ b/src/arraytraits.rs @@ -50,7 +50,7 @@ where fn index(&self, index: I) -> &S::Elem { debug_bounds_check!(self, index); unsafe { - &*self.ptr.offset( + &*self.ptr.as_ptr().offset( index .index_checked(&self.dim, &self.strides) .unwrap_or_else(|| array_out_of_bounds()), diff --git a/src/data_traits.rs b/src/data_traits.rs index d7135e67c..9eafd4467 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -9,6 +9,7 @@ //! The data (inner representation) traits for ndarray use std::mem::{self, size_of}; +use std::ptr::NonNull; use std::sync::Arc; use crate::{ @@ -217,13 +218,13 @@ where let rcvec = &mut self_.data.0; let a_size = mem::size_of::() as isize; let our_off = if a_size != 0 { - (self_.ptr as isize - rcvec.as_ptr() as isize) / a_size + (self_.ptr.as_ptr() as isize - rcvec.as_ptr() as isize) / a_size } else { 0 }; let rvec = Arc::make_mut(rcvec); unsafe { - self_.ptr = rvec.as_mut_ptr().offset(our_off); + self_.ptr = NonNull::new(rvec.as_mut_ptr().offset(our_off)).unwrap(); } } diff --git a/src/impl_clone.rs b/src/impl_clone.rs index 013f070ca..7b1f534b2 100644 --- a/src/impl_clone.rs +++ b/src/impl_clone.rs @@ -8,13 +8,15 @@ use crate::imp_prelude::*; use crate::RawDataClone; +use std::ptr::NonNull; + impl Clone for ArrayBase { fn clone(&self) -> ArrayBase { unsafe { - let (data, ptr) = self.data.clone_with_ptr(self.ptr); + let (data, ptr) = self.data.clone_with_ptr(self.ptr.as_ptr()); ArrayBase { data, - ptr, + ptr: NonNull::new(ptr).unwrap(), dim: self.dim.clone(), strides: self.strides.clone(), } @@ -26,7 +28,11 @@ impl Clone for ArrayBase { /// potentially more efficient. fn clone_from(&mut self, other: &Self) { unsafe { - self.ptr = self.data.clone_from_with_ptr(&other.data, other.ptr); + self.ptr = NonNull::new( + self.data + .clone_from_with_ptr(&other.data, other.ptr.as_ptr()), + ) + .unwrap(); self.dim.clone_from(&other.dim); self.strides.clone_from(&other.strides); } diff --git a/src/impl_constructors.rs b/src/impl_constructors.rs index f8589f595..00201ff9c 100644 --- a/src/impl_constructors.rs +++ b/src/impl_constructors.rs @@ -12,6 +12,8 @@ #![allow(clippy::match_wild_err_arm)] +use std::ptr::NonNull; + use num_traits::{Float, One, Zero}; use crate::dimension; @@ -422,7 +424,7 @@ where // debug check for issues that indicates wrong use of this constructor debug_assert!(dimension::can_index_slice(&v, &dim, &strides).is_ok()); ArrayBase { - ptr: v.as_mut_ptr(), + ptr: NonNull::new(v.as_mut_ptr()).unwrap(), data: DataOwned::new(v), strides, dim, diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 08eed3038..eeec254dd 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -9,6 +9,7 @@ use std::cmp; use std::ptr as std_ptr; use std::slice; +use std_ptr::NonNull; use itertools::{izip, zip}; @@ -138,7 +139,7 @@ where S: Data, { debug_assert!(self.pointer_is_inbounds()); - unsafe { ArrayView::new_(self.ptr, self.dim.clone(), self.strides.clone()) } + unsafe { ArrayView::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) } } /// Return a read-write view of the array @@ -147,7 +148,7 @@ where S: DataMut, { self.ensure_unique(); - unsafe { ArrayViewMut::new_(self.ptr, self.dim.clone(), self.strides.clone()) } + unsafe { ArrayViewMut::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) } } /// Return an uniquely owned copy of the array. @@ -468,7 +469,7 @@ where indices, ); unsafe { - self.ptr = self.ptr.offset(offset); + self.ptr = NonNull::new(self.ptr.as_ptr().offset(offset)).unwrap(); } debug_assert!(self.pointer_is_inbounds()); } @@ -506,7 +507,7 @@ where let ptr = self.ptr; index .index_checked(&self.dim, &self.strides) - .map(move |offset| unsafe { ptr.offset(offset) as *const _ }) + .map(move |offset| unsafe { ptr.as_ptr().offset(offset) as *const _ }) } /// Return a mutable reference to the element at `index`, or return `None` @@ -545,7 +546,7 @@ where { arraytraits::debug_bounds_check(self, &index); let off = index.index_unchecked(&self.strides); - &*self.ptr.offset(off) + &*self.ptr.as_ptr().offset(off) } /// Perform *unchecked* array indexing. @@ -563,7 +564,7 @@ where debug_assert!(self.data.is_unique()); arraytraits::debug_bounds_check(self, &index); let off = index.index_unchecked(&self.strides); - &mut *self.ptr.offset(off) + &mut *self.ptr.as_ptr().offset(off) } /// Swap elements at indices `index1` and `index2`. @@ -599,7 +600,10 @@ where arraytraits::debug_bounds_check(self, &index2); let off1 = index1.index_unchecked(&self.strides); let off2 = index2.index_unchecked(&self.strides); - std_ptr::swap(self.ptr.offset(off1), self.ptr.offset(off2)); + std_ptr::swap( + self.ptr.as_ptr().offset(off1), + self.ptr.as_ptr().offset(off2), + ); } // `get` for zero-dimensional arrays @@ -698,7 +702,7 @@ where /// **Panics** if `axis` or `index` is out of bounds. pub fn collapse_axis(&mut self, axis: Axis, index: usize) { let offset = dimension::do_collapse_axis(&mut self.dim, &self.strides, axis.index(), index); - self.ptr = unsafe { self.ptr.offset(offset) }; + self.ptr = unsafe { NonNull::new(self.ptr.as_ptr().offset(offset)).unwrap() }; debug_assert!(self.pointer_is_inbounds()); } @@ -1271,7 +1275,7 @@ where /// where *d* is `self.ndim()`. #[inline(always)] pub fn as_ptr(&self) -> *const A { - self.ptr + self.ptr.as_ptr() as *const A } /// Return a mutable pointer to the first element in the array. @@ -1281,13 +1285,13 @@ where S: RawDataMut, { self.try_ensure_unique(); // for RcArray - self.ptr + self.ptr.as_ptr() } /// Return a raw view of the array. #[inline] pub fn raw_view(&self) -> RawArrayView { - unsafe { RawArrayView::new_(self.ptr, self.dim.clone(), self.strides.clone()) } + unsafe { RawArrayView::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) } } /// Return a raw mutable view of the array. @@ -1297,7 +1301,7 @@ where S: RawDataMut, { self.try_ensure_unique(); // for RcArray - unsafe { RawArrayViewMut::new_(self.ptr, self.dim.clone(), self.strides.clone()) } + unsafe { RawArrayViewMut::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) } } /// Return the array’s data as a slice, if it is contiguous and in standard order. @@ -1310,7 +1314,7 @@ where S: Data, { if self.is_standard_layout() { - unsafe { Some(slice::from_raw_parts(self.ptr, self.len())) } + unsafe { Some(slice::from_raw_parts(self.ptr.as_ptr(), self.len())) } } else { None } @@ -1324,7 +1328,7 @@ where { if self.is_standard_layout() { self.ensure_unique(); - unsafe { Some(slice::from_raw_parts_mut(self.ptr, self.len())) } + unsafe { Some(slice::from_raw_parts_mut(self.ptr.as_ptr(), self.len())) } } else { None } @@ -1342,7 +1346,7 @@ where S: Data, { if self.is_contiguous() { - unsafe { Some(slice::from_raw_parts(self.ptr, self.len())) } + unsafe { Some(slice::from_raw_parts(self.ptr.as_ptr(), self.len())) } } else { None } @@ -1356,7 +1360,7 @@ where { if self.is_contiguous() { self.ensure_unique(); - unsafe { Some(slice::from_raw_parts_mut(self.ptr, self.len())) } + unsafe { Some(slice::from_raw_parts_mut(self.ptr.as_ptr(), self.len())) } } else { None } @@ -1594,7 +1598,7 @@ where Some(st) => st, None => return None, }; - unsafe { Some(ArrayView::new_(self.ptr, dim, broadcast_strides)) } + unsafe { Some(ArrayView::new_(self.ptr.as_ptr(), dim, broadcast_strides)) } } /// Swap axes `ax` and `bx`. @@ -1719,7 +1723,8 @@ where let s = self.strides.axis(axis) as Ixs; let m = self.dim.axis(axis); if m != 0 { - self.ptr = self.ptr.offset(stride_offset(m - 1, s as Ix)); + self.ptr = + NonNull::new(self.ptr.as_ptr().offset(stride_offset(m - 1, s as Ix))).unwrap(); } self.strides.set_axis(axis, (-s) as Ix); } @@ -1821,7 +1826,7 @@ where Some(slc) => { let ptr = slc.as_ptr() as *mut A; let end = unsafe { ptr.add(slc.len()) }; - self.ptr >= ptr && self.ptr <= end + self.ptr.as_ptr() >= ptr && self.ptr.as_ptr() <= end } } } diff --git a/src/impl_owned_array.rs b/src/impl_owned_array.rs index 35086ffcf..96075593f 100644 --- a/src/impl_owned_array.rs +++ b/src/impl_owned_array.rs @@ -29,7 +29,7 @@ impl Array { // (This is necessary because the element in the array might not be // the first element in the `Vec`, such as if the array was created // by `array![1, 2, 3, 4].slice_move(s![2])`.) - let first = self.ptr as usize; + let first = self.ptr.as_ptr() as usize; let base = self.data.0.as_ptr() as usize; let index = (first - base) / size; debug_assert_eq!((first - base) % size, 0); diff --git a/src/impl_raw_views.rs b/src/impl_raw_views.rs index e5cf8438a..7ae8cc2be 100644 --- a/src/impl_raw_views.rs +++ b/src/impl_raw_views.rs @@ -1,3 +1,5 @@ +use std::ptr::NonNull; + use crate::dimension::{self, stride_offset}; use crate::imp_prelude::*; use crate::{is_aligned, StrideShape}; @@ -14,7 +16,7 @@ where pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self { RawArrayView { data: RawViewRepr::new(), - ptr: ptr as *mut A, + ptr: NonNull::new(ptr as *mut A).unwrap(), dim, strides, } @@ -75,7 +77,7 @@ where /// ensure that all of the data is valid and choose the correct lifetime. #[inline] pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> { - ArrayView::new_(self.ptr, self.dim, self.strides) + ArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) } /// Split the array view along `axis` and return one array pointer strictly @@ -84,13 +86,13 @@ where /// **Panics** if `axis` or `index` is out of bounds. pub fn split_at(self, axis: Axis, index: Ix) -> (Self, Self) { assert!(index <= self.len_of(axis)); - let left_ptr = self.ptr; + let left_ptr = self.ptr.as_ptr(); let right_ptr = if index == self.len_of(axis) { - self.ptr + self.ptr.as_ptr() } else { let offset = stride_offset(index, self.strides.axis(axis)); // The `.offset()` is safe due to the guarantees of `RawData`. - unsafe { self.ptr.offset(offset) } + unsafe { self.ptr.as_ptr().offset(offset) } }; let mut dim_left = self.dim.clone(); @@ -118,7 +120,7 @@ where pub(crate) unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self { RawArrayViewMut { data: RawViewRepr::new(), - ptr, + ptr: NonNull::new(ptr).unwrap(), dim, strides, } @@ -175,7 +177,7 @@ where /// Converts to a non-mutable `RawArrayView`. #[inline] pub(crate) fn into_raw_view(self) -> RawArrayView { - unsafe { RawArrayView::new_(self.ptr, self.dim, self.strides) } + unsafe { RawArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) } } /// Converts to a read-only view of the array. @@ -185,7 +187,7 @@ where /// ensure that all of the data is valid and choose the correct lifetime. #[inline] pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> { - ArrayView::new_(self.ptr, self.dim, self.strides) + ArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) } /// Converts to a mutable view of the array. @@ -195,7 +197,7 @@ where /// ensure that all of the data is valid and choose the correct lifetime. #[inline] pub unsafe fn deref_into_view_mut<'a>(self) -> ArrayViewMut<'a, A, D> { - ArrayViewMut::new_(self.ptr, self.dim, self.strides) + ArrayViewMut::new_(self.ptr.as_ptr(), self.dim, self.strides) } /// Split the array view along `axis` and return one array pointer strictly @@ -206,8 +208,8 @@ where let (left, right) = self.into_raw_view().split_at(axis, index); unsafe { ( - Self::new_(left.ptr, left.dim, left.strides), - Self::new_(right.ptr, right.dim, right.strides), + Self::new_(left.ptr.as_ptr(), left.dim, left.strides), + Self::new_(right.ptr.as_ptr(), right.dim, right.strides), ) } } diff --git a/src/impl_views.rs b/src/impl_views.rs index 63b4ea526..4578b0e2d 100644 --- a/src/impl_views.rs +++ b/src/impl_views.rs @@ -6,6 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::ptr::NonNull; use std::slice; use crate::arraytraits::array_out_of_bounds; @@ -141,7 +142,7 @@ where #[allow(clippy::wrong_self_convention)] pub fn into_slice(&self) -> Option<&'a [A]> { if self.is_standard_layout() { - unsafe { Some(slice::from_raw_parts(self.ptr, self.len())) } + unsafe { Some(slice::from_raw_parts(self.ptr.as_ptr(), self.len())) } } else { None } @@ -151,7 +152,7 @@ where /// Return `None` otherwise. pub fn to_slice(&self) -> Option<&'a [A]> { if self.is_standard_layout() { - unsafe { Some(slice::from_raw_parts(self.ptr, self.len())) } + unsafe { Some(slice::from_raw_parts(self.ptr.as_ptr(), self.len())) } } else { None } @@ -159,7 +160,7 @@ where /// Converts to a raw array view. pub(crate) fn into_raw_view(self) -> RawArrayView { - unsafe { RawArrayView::new_(self.ptr, self.dim, self.strides) } + unsafe { RawArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) } } } @@ -487,7 +488,7 @@ where pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self { ArrayView { data: ViewRepr::new(), - ptr: ptr as *mut A, + ptr: NonNull::new(ptr as *mut A).unwrap(), dim, strides, } @@ -495,7 +496,7 @@ where #[inline] pub(crate) fn into_base_iter(self) -> Baseiter { - unsafe { Baseiter::new(self.ptr, self.dim, self.strides) } + unsafe { Baseiter::new(self.ptr.as_ptr(), self.dim, self.strides) } } #[inline] @@ -534,7 +535,7 @@ where } ArrayViewMut { data: ViewRepr::new(), - ptr, + ptr: NonNull::new(ptr).unwrap(), dim, strides, } @@ -542,17 +543,17 @@ where // Convert into a read-only view pub(crate) fn into_view(self) -> ArrayView<'a, A, D> { - unsafe { ArrayView::new_(self.ptr, self.dim, self.strides) } + unsafe { ArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) } } /// Converts to a mutable raw array view. pub(crate) fn into_raw_view_mut(self) -> RawArrayViewMut { - unsafe { RawArrayViewMut::new_(self.ptr, self.dim, self.strides) } + unsafe { RawArrayViewMut::new_(self.ptr.as_ptr(), self.dim, self.strides) } } #[inline] pub(crate) fn into_base_iter(self) -> Baseiter { - unsafe { Baseiter::new(self.ptr, self.dim, self.strides) } + unsafe { Baseiter::new(self.ptr.as_ptr(), self.dim, self.strides) } } #[inline] @@ -562,7 +563,7 @@ where pub(crate) fn into_slice_(self) -> Result<&'a mut [A], Self> { if self.is_standard_layout() { - unsafe { Ok(slice::from_raw_parts_mut(self.ptr, self.len())) } + unsafe { Ok(slice::from_raw_parts_mut(self.ptr.as_ptr(), self.len())) } } else { Err(self) } diff --git a/src/iterators/mod.rs b/src/iterators/mod.rs index b98d944f8..dcc425c4f 100644 --- a/src/iterators/mod.rs +++ b/src/iterators/mod.rs @@ -775,7 +775,7 @@ impl AxisIterCore { stride, inner_dim: v.dim.remove_axis(axis), inner_strides: v.strides.remove_axis(axis), - ptr: v.ptr, + ptr: v.ptr.as_ptr(), } } @@ -1197,7 +1197,7 @@ fn chunk_iter_parts( stride, inner_dim, inner_strides: v.strides, - ptr: v.ptr, + ptr: v.ptr.as_ptr(), }; (iter, n_whole_chunks, last_dim) diff --git a/src/iterators/windows.rs b/src/iterators/windows.rs index f05183ddd..e0abbc537 100644 --- a/src/iterators/windows.rs +++ b/src/iterators/windows.rs @@ -41,7 +41,7 @@ impl<'a, A, D: Dimension> Windows<'a, A, D> { unsafe { Windows { - base: ArrayView::from_shape_ptr(size.clone().strides(a.strides), a.ptr), + base: ArrayView::from_shape_ptr(size.clone().strides(a.strides), a.ptr.as_ptr()), window, strides: window_strides, } diff --git a/src/lib.rs b/src/lib.rs index e5af0863c..109d36304 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1217,7 +1217,7 @@ where data: S, /// A non-null and aligned pointer into the buffer held by `data`; may /// point anywhere in its range. - ptr: *mut S::Elem, + ptr: std::ptr::NonNull, /// The lengths of the axes. dim: D, /// The element count stride per axis. To be parsed as `isize`. @@ -1523,7 +1523,7 @@ where let ptr = self.ptr; let mut strides = dim.clone(); strides.slice_mut().copy_from_slice(self.strides.slice()); - unsafe { ArrayView::new_(ptr, dim, strides) } + unsafe { ArrayView::new_(ptr.as_ptr(), dim, strides) } } fn raw_strides(&self) -> D { diff --git a/src/zip/mod.rs b/src/zip/mod.rs index a2567f732..784fda7d0 100644 --- a/src/zip/mod.rs +++ b/src/zip/mod.rs @@ -73,7 +73,7 @@ where type Output = ArrayView<'a, A, E::Dim>; fn broadcast_unwrap(self, shape: E) -> Self::Output { let res: ArrayView<'_, A, E::Dim> = (&self).broadcast_unwrap(shape.into_dimension()); - unsafe { ArrayView::new_(res.ptr, res.dim, res.strides) } + unsafe { ArrayView::new_(res.ptr.as_ptr(), res.dim, res.strides) } } private_impl! {} } @@ -317,7 +317,7 @@ impl<'a, A, D: Dimension> NdProducer for ArrayView<'a, A, D> { #[doc(hidden)] unsafe fn uget_ptr(&self, i: &Self::Dim) -> *mut A { - self.ptr.offset(i.index_unchecked(&self.strides)) + self.ptr.as_ptr().offset(i.index_unchecked(&self.strides)) } #[doc(hidden)] @@ -370,7 +370,7 @@ impl<'a, A, D: Dimension> NdProducer for ArrayViewMut<'a, A, D> { #[doc(hidden)] unsafe fn uget_ptr(&self, i: &Self::Dim) -> *mut A { - self.ptr.offset(i.index_unchecked(&self.strides)) + self.ptr.as_ptr().offset(i.index_unchecked(&self.strides)) } #[doc(hidden)] From dec13badccea17e100c921331d4c5af80982a1d4 Mon Sep 17 00:00:00 2001 From: Eric Berquist Date: Sun, 11 Aug 2019 13:02:11 -0400 Subject: [PATCH 2/7] Handle NonNull ArrayBase.ptr in linalg --- src/linalg/impl_linalg.rs | 42 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/linalg/impl_linalg.rs b/src/linalg/impl_linalg.rs index 5ccfbdf84..4d644824e 100644 --- a/src/linalg/impl_linalg.rs +++ b/src/linalg/impl_linalg.rs @@ -108,9 +108,9 @@ where if blas_compat_1d::<$ty, _>(self) && blas_compat_1d::<$ty, _>(rhs) { unsafe { let (lhs_ptr, n, incx) = - blas_1d_params(self.ptr, self.len(), self.strides()[0]); + blas_1d_params(self.ptr.as_ptr(), self.len(), self.strides()[0]); let (rhs_ptr, _, incy) = - blas_1d_params(rhs.ptr, rhs.len(), rhs.strides()[0]); + blas_1d_params(rhs.ptr.as_ptr(), rhs.len(), rhs.strides()[0]); let ret = blas_sys::$func( n, lhs_ptr as *const $ty, @@ -432,17 +432,17 @@ fn mat_mul_impl( CblasRowMajor, lhs_trans, rhs_trans, - m as blas_index, // m, rows of Op(a) - n as blas_index, // n, cols of Op(b) - k as blas_index, // k, cols of Op(a) - cast_as(&alpha), // alpha - lhs_.ptr as *const _, // a - lhs_stride, // lda - rhs_.ptr as *const _, // b - rhs_stride, // ldb - cast_as(&beta), // beta - c_.ptr as *mut _, // c - c_stride, // ldc + m as blas_index, // m, rows of Op(a) + n as blas_index, // n, cols of Op(b) + k as blas_index, // k, cols of Op(a) + cast_as(&alpha), // alpha + lhs_.ptr.as_ptr() as *const _, // a + lhs_stride, // lda + rhs_.ptr.as_ptr() as *const _, // b + rhs_stride, // ldb + cast_as(&beta), // beta + c_.ptr.as_ptr() as *mut _, // c + c_stride, // ldc ); } return; @@ -622,15 +622,15 @@ pub fn general_mat_vec_mul( blas_sys::$gemv( layout, a_trans, - m as blas_index, // m, rows of Op(a) - k as blas_index, // n, cols of Op(a) - cast_as(&alpha), // alpha - a.ptr as *const _, // a - a_stride, // lda - x.ptr as *const _, // x + m as blas_index, // m, rows of Op(a) + k as blas_index, // n, cols of Op(a) + cast_as(&alpha), // alpha + a.ptr.as_ptr() as *const _, // a + a_stride, // lda + x.ptr.as_ptr() as *const _, // x x_stride, - cast_as(&beta), // beta - y.ptr as *mut _, // x + cast_as(&beta), // beta + y.ptr.as_ptr() as *mut _, // x y_stride, ); } From b9dbed35487bb8bf914c37af6d3aaf1a2cd25d38 Mon Sep 17 00:00:00 2001 From: Eric Berquist Date: Sun, 11 Aug 2019 13:23:07 -0400 Subject: [PATCH 3/7] Reorder impl_clone imports --- src/impl_clone.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/impl_clone.rs b/src/impl_clone.rs index 7b1f534b2..7669cbea8 100644 --- a/src/impl_clone.rs +++ b/src/impl_clone.rs @@ -5,11 +5,11 @@ // , at your // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::ptr::NonNull; + use crate::imp_prelude::*; use crate::RawDataClone; -use std::ptr::NonNull; - impl Clone for ArrayBase { fn clone(&self) -> ArrayBase { unsafe { From 712bcdd51e1210e755e2d0a7bebe333add37c451 Mon Sep 17 00:00:00 2001 From: Eric Berquist Date: Mon, 26 Aug 2019 00:26:19 -0400 Subject: [PATCH 4/7] Switch entirely to NonNull pointers in data_traits --- src/data_traits.rs | 35 ++++++++++++++++++----------------- src/impl_clone.rs | 11 +++-------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index 9eafd4467..f63ba6666 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -68,14 +68,14 @@ pub unsafe trait RawDataMut: RawData { pub unsafe trait RawDataClone: RawData { #[doc(hidden)] /// Unsafe because, `ptr` must point inside the current storage. - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem); + unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull); #[doc(hidden)] unsafe fn clone_from_with_ptr( &mut self, other: &Self, - ptr: *mut Self::Elem, - ) -> *mut Self::Elem { + ptr: NonNull, + ) -> NonNull { let (data, ptr) = other.clone_with_ptr(ptr); *self = data; ptr @@ -149,7 +149,7 @@ unsafe impl RawData for RawViewRepr<*const A> { } unsafe impl RawDataClone for RawViewRepr<*const A> { - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { + unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull) { (*self, ptr) } } @@ -178,7 +178,7 @@ unsafe impl RawDataMut for RawViewRepr<*mut A> { } unsafe impl RawDataClone for RawViewRepr<*mut A> { - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { + unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull) { (*self, ptr) } } @@ -253,7 +253,7 @@ unsafe impl Data for OwnedArcRepr { unsafe impl DataMut for OwnedArcRepr where A: Clone {} unsafe impl RawDataClone for OwnedArcRepr { - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { + unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull) { // pointer is preserved (self.clone(), ptr) } @@ -299,28 +299,29 @@ unsafe impl RawDataClone for OwnedRepr where A: Clone, { - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { + unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull) { let mut u = self.clone(); let mut new_ptr = u.0.as_mut_ptr(); if size_of::() != 0 { - let our_off = (ptr as isize - self.0.as_ptr() as isize) / mem::size_of::() as isize; + let our_off = + (ptr.as_ptr() as isize - self.0.as_ptr() as isize) / mem::size_of::() as isize; new_ptr = new_ptr.offset(our_off); } - (u, new_ptr) + (u, NonNull::new(new_ptr).unwrap()) } unsafe fn clone_from_with_ptr( &mut self, other: &Self, - ptr: *mut Self::Elem, - ) -> *mut Self::Elem { + ptr: NonNull, + ) -> NonNull { let our_off = if size_of::() != 0 { - (ptr as isize - other.0.as_ptr() as isize) / mem::size_of::() as isize + (ptr.as_ptr() as isize - other.0.as_ptr() as isize) / mem::size_of::() as isize } else { 0 }; self.0.clone_from(&other.0); - self.0.as_mut_ptr().offset(our_off) + NonNull::new(self.0.as_mut_ptr().offset(our_off)).unwrap() } } @@ -343,7 +344,7 @@ unsafe impl<'a, A> Data for ViewRepr<&'a A> { } unsafe impl<'a, A> RawDataClone for ViewRepr<&'a A> { - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { + unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull) { (*self, ptr) } } @@ -470,7 +471,7 @@ unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> where A: Clone, { - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { + unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull) { match self { CowRepr::View(view) => { let (new_view, ptr) = view.clone_with_ptr(ptr); @@ -487,8 +488,8 @@ where unsafe fn clone_from_with_ptr( &mut self, other: &Self, - ptr: *mut Self::Elem, - ) -> *mut Self::Elem { + ptr: NonNull, + ) -> NonNull { match (&mut *self, other) { (CowRepr::View(self_), CowRepr::View(other)) => self_.clone_from_with_ptr(other, ptr), (CowRepr::Owned(self_), CowRepr::Owned(other)) => self_.clone_from_with_ptr(other, ptr), diff --git a/src/impl_clone.rs b/src/impl_clone.rs index 7669cbea8..603708849 100644 --- a/src/impl_clone.rs +++ b/src/impl_clone.rs @@ -5,7 +5,6 @@ // , at your // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::ptr::NonNull; use crate::imp_prelude::*; use crate::RawDataClone; @@ -13,10 +12,10 @@ use crate::RawDataClone; impl Clone for ArrayBase { fn clone(&self) -> ArrayBase { unsafe { - let (data, ptr) = self.data.clone_with_ptr(self.ptr.as_ptr()); + let (data, ptr) = self.data.clone_with_ptr(self.ptr); ArrayBase { data, - ptr: NonNull::new(ptr).unwrap(), + ptr, dim: self.dim.clone(), strides: self.strides.clone(), } @@ -28,11 +27,7 @@ impl Clone for ArrayBase { /// potentially more efficient. fn clone_from(&mut self, other: &Self) { unsafe { - self.ptr = NonNull::new( - self.data - .clone_from_with_ptr(&other.data, other.ptr.as_ptr()), - ) - .unwrap(); + self.ptr = self.data.clone_from_with_ptr(&other.data, other.ptr); self.dim.clone_from(&other.dim); self.strides.clone_from(&other.strides); } From 8b733454c102694956ccfcf8b51d1091ce633f1c Mon Sep 17 00:00:00 2001 From: Eric Berquist Date: Mon, 26 Aug 2019 23:42:27 -0400 Subject: [PATCH 5/7] Use unchecked NonNull creation in RawArrayViews --- src/impl_raw_views.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/impl_raw_views.rs b/src/impl_raw_views.rs index 7ae8cc2be..20cf62c85 100644 --- a/src/impl_raw_views.rs +++ b/src/impl_raw_views.rs @@ -16,7 +16,10 @@ where pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self { RawArrayView { data: RawViewRepr::new(), - ptr: NonNull::new(ptr as *mut A).unwrap(), + ptr: { + debug_assert!(!ptr.is_null()); + NonNull::new_unchecked(ptr as *mut A) + }, dim, strides, } @@ -120,7 +123,10 @@ where pub(crate) unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self { RawArrayViewMut { data: RawViewRepr::new(), - ptr: NonNull::new(ptr).unwrap(), + ptr: { + debug_assert!(!ptr.is_null()); + NonNull::new_unchecked(ptr) + }, dim, strides, } From df87abd69cf482d03e0a580455c0427fb2896e92 Mon Sep 17 00:00:00 2001 From: bluss Date: Sun, 8 Sep 2019 17:20:41 +0200 Subject: [PATCH 6/7] MAINT: Add dependency on rawpointer rawpointer 0.2 implements offset methods for NonNull, which will be useful for us. This was already a dev-dependency, and also already a regular transitive dependency (from matrixmultiply). --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index e9ea56d8c..92f3d9841 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,11 +44,11 @@ blas-src = { version = "0.2.0", optional = true, default-features = false } matrixmultiply = { version = "0.2.0" } # Use via the `serde-1` crate feature! serde = { version = "1.0", optional = true } +rawpointer = { version = "0.2" } [dev-dependencies] defmac = "0.2" quickcheck = { version = "0.8", default-features = false } -rawpointer = "0.1" approx = "0.3.2" itertools = { version = "0.8.0", default-features = false, features = ["use_std"] } From ff5729f6805ece33fbdd00fafc82efcdb1c95eeb Mon Sep 17 00:00:00 2001 From: bluss Date: Sun, 8 Sep 2019 17:30:28 +0200 Subject: [PATCH 7/7] Use NonNull::offset method and factor out nonnull constructors Add two constructors that suffice for ndarray: - Get NonNull from Vec's buffer pointer - Get NonNull from raw pointer, and check if it's nonnull with a debug assertion In all other cases we can use .offset() on the NonNull (method from rawpointer 0.2, so we don't have to explicit roundtrip through raw pointers when we offset NonNull. For safety, see this documentation in the rawpointer crate, which says why using .offset() on NonNull is safe. NonNull supports the same offsetting methods under the same safety constraints as the other raw pointer implementations. There is no difference - both when offsetting *mut T and NonNull, the offset is only well defined if we remain inside the same object or one-past the end, and we can never land in a null pointer while obeying those rules. --- src/data_traits.rs | 10 ++++++---- src/extension.rs | 11 +++++++++++ src/extension/nonnull.rs | 18 ++++++++++++++++++ src/impl_constructors.rs | 5 ++--- src/impl_methods.rs | 9 ++++----- src/impl_raw_views.rs | 13 +++---------- src/impl_views.rs | 6 +++--- src/lib.rs | 1 + 8 files changed, 48 insertions(+), 25 deletions(-) create mode 100644 src/extension.rs create mode 100644 src/extension/nonnull.rs diff --git a/src/data_traits.rs b/src/data_traits.rs index f63ba6666..b3f07959e 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -8,6 +8,8 @@ //! The data (inner representation) traits for ndarray +use crate::extension::nonnull::nonnull_from_vec_data; +use rawpointer::PointerExt; use std::mem::{self, size_of}; use std::ptr::NonNull; use std::sync::Arc; @@ -224,7 +226,7 @@ where }; let rvec = Arc::make_mut(rcvec); unsafe { - self_.ptr = NonNull::new(rvec.as_mut_ptr().offset(our_off)).unwrap(); + self_.ptr = nonnull_from_vec_data(rvec).offset(our_off); } } @@ -301,13 +303,13 @@ where { unsafe fn clone_with_ptr(&self, ptr: NonNull) -> (Self, NonNull) { let mut u = self.clone(); - let mut new_ptr = u.0.as_mut_ptr(); + let mut new_ptr = nonnull_from_vec_data(&mut u.0); if size_of::() != 0 { let our_off = (ptr.as_ptr() as isize - self.0.as_ptr() as isize) / mem::size_of::() as isize; new_ptr = new_ptr.offset(our_off); } - (u, NonNull::new(new_ptr).unwrap()) + (u, new_ptr) } unsafe fn clone_from_with_ptr( @@ -321,7 +323,7 @@ where 0 }; self.0.clone_from(&other.0); - NonNull::new(self.0.as_mut_ptr().offset(our_off)).unwrap() + nonnull_from_vec_data(&mut self.0).offset(our_off) } } diff --git a/src/extension.rs b/src/extension.rs new file mode 100644 index 000000000..c74a09236 --- /dev/null +++ b/src/extension.rs @@ -0,0 +1,11 @@ +// Copyright 2019 bluss and ndarray developers. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Extension traits and utility functions for types from outside ndarray + +pub(crate) mod nonnull; diff --git a/src/extension/nonnull.rs b/src/extension/nonnull.rs new file mode 100644 index 000000000..32fbb07c4 --- /dev/null +++ b/src/extension/nonnull.rs @@ -0,0 +1,18 @@ +use std::ptr::NonNull; + +/// Return a NonNull pointer to the vector's data +pub(crate) fn nonnull_from_vec_data(v: &mut Vec) -> NonNull { + // this pointer is guaranteed to be non-null + unsafe { NonNull::new_unchecked(v.as_mut_ptr()) } +} + +/// Converts `ptr` to `NonNull` +/// +/// Safety: `ptr` *must* be non-null. +/// This is checked with a debug assertion, and will panic if this is not true, +/// but treat this as an unconditional conversion. +#[inline] +pub(crate) unsafe fn nonnull_debug_checked_from_ptr(ptr: *mut T) -> NonNull { + debug_assert!(!ptr.is_null()); + NonNull::new_unchecked(ptr) +} diff --git a/src/impl_constructors.rs b/src/impl_constructors.rs index 00201ff9c..4c35bbfb3 100644 --- a/src/impl_constructors.rs +++ b/src/impl_constructors.rs @@ -12,12 +12,11 @@ #![allow(clippy::match_wild_err_arm)] -use std::ptr::NonNull; - use num_traits::{Float, One, Zero}; use crate::dimension; use crate::error::{self, ShapeError}; +use crate::extension::nonnull::nonnull_from_vec_data; use crate::imp_prelude::*; use crate::indexes; use crate::indices; @@ -424,7 +423,7 @@ where // debug check for issues that indicates wrong use of this constructor debug_assert!(dimension::can_index_slice(&v, &dim, &strides).is_ok()); ArrayBase { - ptr: NonNull::new(v.as_mut_ptr()).unwrap(), + ptr: nonnull_from_vec_data(&mut v), data: DataOwned::new(v), strides, dim, diff --git a/src/impl_methods.rs b/src/impl_methods.rs index eeec254dd..d04bcefe7 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -9,9 +9,9 @@ use std::cmp; use std::ptr as std_ptr; use std::slice; -use std_ptr::NonNull; use itertools::{izip, zip}; +use rawpointer::PointerExt; use crate::imp_prelude::*; @@ -469,7 +469,7 @@ where indices, ); unsafe { - self.ptr = NonNull::new(self.ptr.as_ptr().offset(offset)).unwrap(); + self.ptr = self.ptr.offset(offset); } debug_assert!(self.pointer_is_inbounds()); } @@ -702,7 +702,7 @@ where /// **Panics** if `axis` or `index` is out of bounds. pub fn collapse_axis(&mut self, axis: Axis, index: usize) { let offset = dimension::do_collapse_axis(&mut self.dim, &self.strides, axis.index(), index); - self.ptr = unsafe { NonNull::new(self.ptr.as_ptr().offset(offset)).unwrap() }; + self.ptr = unsafe { self.ptr.offset(offset) }; debug_assert!(self.pointer_is_inbounds()); } @@ -1723,8 +1723,7 @@ where let s = self.strides.axis(axis) as Ixs; let m = self.dim.axis(axis); if m != 0 { - self.ptr = - NonNull::new(self.ptr.as_ptr().offset(stride_offset(m - 1, s as Ix))).unwrap(); + self.ptr = self.ptr.offset(stride_offset(m - 1, s as Ix)); } self.strides.set_axis(axis, (-s) as Ix); } diff --git a/src/impl_raw_views.rs b/src/impl_raw_views.rs index 20cf62c85..154f86688 100644 --- a/src/impl_raw_views.rs +++ b/src/impl_raw_views.rs @@ -1,6 +1,5 @@ -use std::ptr::NonNull; - use crate::dimension::{self, stride_offset}; +use crate::extension::nonnull::nonnull_debug_checked_from_ptr; use crate::imp_prelude::*; use crate::{is_aligned, StrideShape}; @@ -16,10 +15,7 @@ where pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self { RawArrayView { data: RawViewRepr::new(), - ptr: { - debug_assert!(!ptr.is_null()); - NonNull::new_unchecked(ptr as *mut A) - }, + ptr: nonnull_debug_checked_from_ptr(ptr as *mut _), dim, strides, } @@ -123,10 +119,7 @@ where pub(crate) unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self { RawArrayViewMut { data: RawViewRepr::new(), - ptr: { - debug_assert!(!ptr.is_null()); - NonNull::new_unchecked(ptr) - }, + ptr: nonnull_debug_checked_from_ptr(ptr), dim, strides, } diff --git a/src/impl_views.rs b/src/impl_views.rs index 4578b0e2d..05ae0a83c 100644 --- a/src/impl_views.rs +++ b/src/impl_views.rs @@ -6,12 +6,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::ptr::NonNull; use std::slice; use crate::arraytraits::array_out_of_bounds; use crate::dimension; use crate::error::ShapeError; +use crate::extension::nonnull::nonnull_debug_checked_from_ptr; use crate::imp_prelude::*; use crate::{is_aligned, NdIndex, StrideShape}; @@ -488,7 +488,7 @@ where pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self { ArrayView { data: ViewRepr::new(), - ptr: NonNull::new(ptr as *mut A).unwrap(), + ptr: nonnull_debug_checked_from_ptr(ptr as *mut _), dim, strides, } @@ -535,7 +535,7 @@ where } ArrayViewMut { data: ViewRepr::new(), - ptr: NonNull::new(ptr).unwrap(), + ptr: nonnull_debug_checked_from_ptr(ptr), dim, strides, } diff --git a/src/lib.rs b/src/lib.rs index 109d36304..e8243604e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -153,6 +153,7 @@ pub use crate::free_functions::*; pub use crate::iterators::iter; mod error; +mod extension; mod geomspace; mod indexes; mod iterators;