Skip to content

Commit e3f1bcd

Browse files
committed
FIX: Use NonNull internal constructors for Raw/ArrayView/Mut
This saves som unwrapping/rewrapping in NonNull, by simpliy copying `self.ptr` along. This requires *some* scrutiny, because when we remove calls to .as_ptr_mut(), we also remove the check for breaking sharing in copy on write arrays (Cow and Arc). However all instances of this, in this commit, should already be from uniquely owned arrays or are covered by `ensure_unique` already (view_mut). For the array views, the old new_ constructor also remains for use in all places we make array views directly from raw pointers.
1 parent 277e49b commit e3f1bcd

File tree

6 files changed

+60
-31
lines changed

6 files changed

+60
-31
lines changed

src/impl_methods.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ where
139139
S: Data,
140140
{
141141
debug_assert!(self.pointer_is_inbounds());
142-
unsafe { ArrayView::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) }
142+
unsafe { ArrayView::new(self.ptr, self.dim.clone(), self.strides.clone()) }
143143
}
144144

145145
/// Return a read-write view of the array
@@ -148,7 +148,7 @@ where
148148
S: DataMut,
149149
{
150150
self.ensure_unique();
151-
unsafe { ArrayViewMut::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) }
151+
unsafe { ArrayViewMut::new(self.ptr, self.dim.clone(), self.strides.clone()) }
152152
}
153153

154154
/// Return an uniquely owned copy of the array.
@@ -1313,7 +1313,7 @@ where
13131313
/// Return a raw view of the array.
13141314
#[inline]
13151315
pub fn raw_view(&self) -> RawArrayView<A, D> {
1316-
unsafe { RawArrayView::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) }
1316+
unsafe { RawArrayView::new(self.ptr, self.dim.clone(), self.strides.clone()) }
13171317
}
13181318

13191319
/// Return a raw mutable view of the array.
@@ -1323,7 +1323,7 @@ where
13231323
S: RawDataMut,
13241324
{
13251325
self.try_ensure_unique(); // for RcArray
1326-
unsafe { RawArrayViewMut::new_(self.ptr.as_ptr(), self.dim.clone(), self.strides.clone()) }
1326+
unsafe { RawArrayViewMut::new(self.ptr, self.dim.clone(), self.strides.clone()) }
13271327
}
13281328

13291329
/// Return the array’s data as a slice, if it is contiguous and in standard order.
@@ -1620,7 +1620,7 @@ where
16201620
Some(st) => st,
16211621
None => return None,
16221622
};
1623-
unsafe { Some(ArrayView::new_(self.ptr.as_ptr(), dim, broadcast_strides)) }
1623+
unsafe { Some(ArrayView::new(self.ptr, dim, broadcast_strides)) }
16241624
}
16251625

16261626
/// Swap axes `ax` and `bx`.

src/impl_raw_views.rs

+22-12
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::ptr::NonNull;
2+
13
use crate::dimension::{self, stride_offset};
24
use crate::extension::nonnull::nonnull_debug_checked_from_ptr;
35
use crate::imp_prelude::*;
@@ -11,16 +13,20 @@ where
1113
///
1214
/// Unsafe because caller is responsible for ensuring that the array will
1315
/// meet all of the invariants of the `ArrayBase` type.
14-
#[inline(always)]
15-
pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self {
16+
#[inline]
17+
pub(crate) unsafe fn new(ptr: NonNull<A>, dim: D, strides: D) -> Self {
1618
RawArrayView {
1719
data: RawViewRepr::new(),
18-
ptr: nonnull_debug_checked_from_ptr(ptr as *mut _),
20+
ptr,
1921
dim,
2022
strides,
2123
}
2224
}
2325

26+
unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self {
27+
Self::new(nonnull_debug_checked_from_ptr(ptr as *mut A), dim, strides)
28+
}
29+
2430
/// Create an `RawArrayView<A, D>` from shape information and a raw pointer
2531
/// to the elements.
2632
///
@@ -76,7 +82,7 @@ where
7682
/// ensure that all of the data is valid and choose the correct lifetime.
7783
#[inline]
7884
pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> {
79-
ArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides)
85+
ArrayView::new(self.ptr, self.dim, self.strides)
8086
}
8187

8288
/// Split the array view along `axis` and return one array pointer strictly
@@ -115,16 +121,20 @@ where
115121
///
116122
/// Unsafe because caller is responsible for ensuring that the array will
117123
/// meet all of the invariants of the `ArrayBase` type.
118-
#[inline(always)]
119-
pub(crate) unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self {
124+
#[inline]
125+
pub(crate) unsafe fn new(ptr: NonNull<A>, dim: D, strides: D) -> Self {
120126
RawArrayViewMut {
121127
data: RawViewRepr::new(),
122-
ptr: nonnull_debug_checked_from_ptr(ptr),
128+
ptr,
123129
dim,
124130
strides,
125131
}
126132
}
127133

134+
unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self {
135+
Self::new(nonnull_debug_checked_from_ptr(ptr), dim, strides)
136+
}
137+
128138
/// Create an `RawArrayViewMut<A, D>` from shape information and a raw
129139
/// pointer to the elements.
130140
///
@@ -176,7 +186,7 @@ where
176186
/// Converts to a non-mutable `RawArrayView`.
177187
#[inline]
178188
pub(crate) fn into_raw_view(self) -> RawArrayView<A, D> {
179-
unsafe { RawArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) }
189+
unsafe { RawArrayView::new(self.ptr, self.dim, self.strides) }
180190
}
181191

182192
/// Converts to a read-only view of the array.
@@ -186,7 +196,7 @@ where
186196
/// ensure that all of the data is valid and choose the correct lifetime.
187197
#[inline]
188198
pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> {
189-
ArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides)
199+
ArrayView::new(self.ptr, self.dim, self.strides)
190200
}
191201

192202
/// Converts to a mutable view of the array.
@@ -196,7 +206,7 @@ where
196206
/// ensure that all of the data is valid and choose the correct lifetime.
197207
#[inline]
198208
pub unsafe fn deref_into_view_mut<'a>(self) -> ArrayViewMut<'a, A, D> {
199-
ArrayViewMut::new_(self.ptr.as_ptr(), self.dim, self.strides)
209+
ArrayViewMut::new(self.ptr, self.dim, self.strides)
200210
}
201211

202212
/// Split the array view along `axis` and return one array pointer strictly
@@ -207,8 +217,8 @@ where
207217
let (left, right) = self.into_raw_view().split_at(axis, index);
208218
unsafe {
209219
(
210-
Self::new_(left.ptr.as_ptr(), left.dim, left.strides),
211-
Self::new_(right.ptr.as_ptr(), right.dim, right.strides),
220+
Self::new(left.ptr, left.dim, left.strides),
221+
Self::new(right.ptr, right.dim, right.strides),
212222
)
213223
}
214224
}

src/impl_views/constructors.rs

+27-8
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
// option. This file may not be copied, modified, or distributed
77
// except according to those terms.
88

9+
use std::ptr::NonNull;
10+
911
use crate::dimension;
1012
use crate::error::ShapeError;
1113
use crate::extension::nonnull::nonnull_debug_checked_from_ptr;
@@ -200,11 +202,11 @@ where
200202

201203
/// Convert the view into an `ArrayViewMut<'b, A, D>` where `'b` is a lifetime
202204
/// outlived by `'a'`.
203-
pub fn reborrow<'b>(mut self) -> ArrayViewMut<'b, A, D>
205+
pub fn reborrow<'b>(self) -> ArrayViewMut<'b, A, D>
204206
where
205207
'a: 'b,
206208
{
207-
unsafe { ArrayViewMut::new_(self.as_mut_ptr(), self.dim, self.strides) }
209+
unsafe { ArrayViewMut::new(self.ptr, self.dim, self.strides) }
208210
}
209211
}
210212

@@ -217,14 +219,24 @@ where
217219
///
218220
/// Unsafe because: `ptr` must be valid for the given dimension and strides.
219221
#[inline(always)]
220-
pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self {
222+
pub(crate) unsafe fn new(ptr: NonNull<A>, dim: D, strides: D) -> Self {
223+
if cfg!(debug_assertions) {
224+
assert!(is_aligned(ptr.as_ptr()), "The pointer must be aligned.");
225+
dimension::max_abs_offset_check_overflow::<A, _>(&dim, &strides).unwrap();
226+
}
221227
ArrayView {
222228
data: ViewRepr::new(),
223-
ptr: nonnull_debug_checked_from_ptr(ptr as *mut A),
229+
ptr,
224230
dim,
225231
strides,
226232
}
227233
}
234+
235+
/// Unsafe because: `ptr` must be valid for the given dimension and strides.
236+
#[inline]
237+
pub(crate) unsafe fn new_(ptr: *const A, dim: D, strides: D) -> Self {
238+
Self::new(nonnull_debug_checked_from_ptr(ptr as *mut A), dim, strides)
239+
}
228240
}
229241

230242
impl<'a, A, D> ArrayViewMut<'a, A, D>
@@ -235,17 +247,24 @@ where
235247
///
236248
/// Unsafe because: `ptr` must be valid for the given dimension and strides.
237249
#[inline(always)]
238-
pub(crate) unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self {
250+
pub(crate) unsafe fn new(ptr: NonNull<A>, dim: D, strides: D) -> Self {
239251
if cfg!(debug_assertions) {
240-
assert!(!ptr.is_null(), "The pointer must be non-null.");
241-
assert!(is_aligned(ptr), "The pointer must be aligned.");
252+
assert!(is_aligned(ptr.as_ptr()), "The pointer must be aligned.");
242253
dimension::max_abs_offset_check_overflow::<A, _>(&dim, &strides).unwrap();
243254
}
244255
ArrayViewMut {
245256
data: ViewRepr::new(),
246-
ptr: nonnull_debug_checked_from_ptr(ptr),
257+
ptr,
247258
dim,
248259
strides,
249260
}
250261
}
262+
263+
/// Create a new `ArrayView`
264+
///
265+
/// Unsafe because: `ptr` must be valid for the given dimension and strides.
266+
#[inline(always)]
267+
pub(crate) unsafe fn new_(ptr: *mut A, dim: D, strides: D) -> Self {
268+
Self::new(nonnull_debug_checked_from_ptr(ptr), dim, strides)
269+
}
251270
}

src/impl_views/conversions.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ where
2626
where
2727
'a: 'b,
2828
{
29-
unsafe { ArrayView::new_(self.as_ptr(), self.dim, self.strides) }
29+
unsafe { ArrayView::new(self.ptr, self.dim, self.strides) }
3030
}
3131

3232
/// Return the array’s data as a slice, if it is contiguous and in standard order.
@@ -53,7 +53,7 @@ where
5353

5454
/// Converts to a raw array view.
5555
pub(crate) fn into_raw_view(self) -> RawArrayView<A, D> {
56-
unsafe { RawArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) }
56+
unsafe { RawArrayView::new(self.ptr, self.dim, self.strides) }
5757
}
5858
}
5959

@@ -161,12 +161,12 @@ where
161161
{
162162
// Convert into a read-only view
163163
pub(crate) fn into_view(self) -> ArrayView<'a, A, D> {
164-
unsafe { ArrayView::new_(self.ptr.as_ptr(), self.dim, self.strides) }
164+
unsafe { ArrayView::new(self.ptr, self.dim, self.strides) }
165165
}
166166

167167
/// Converts to a mutable raw array view.
168168
pub(crate) fn into_raw_view_mut(self) -> RawArrayViewMut<A, D> {
169-
unsafe { RawArrayViewMut::new_(self.ptr.as_ptr(), self.dim, self.strides) }
169+
unsafe { RawArrayViewMut::new(self.ptr, self.dim, self.strides) }
170170
}
171171

172172
#[inline]

src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1517,7 +1517,7 @@ where
15171517
let ptr = self.ptr;
15181518
let mut strides = dim.clone();
15191519
strides.slice_mut().copy_from_slice(self.strides.slice());
1520-
unsafe { ArrayView::new_(ptr.as_ptr(), dim, strides) }
1520+
unsafe { ArrayView::new(ptr, dim, strides) }
15211521
}
15221522

15231523
fn raw_strides(&self) -> D {

src/zip/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ where
7373
type Output = ArrayView<'a, A, E::Dim>;
7474
fn broadcast_unwrap(self, shape: E) -> Self::Output {
7575
let res: ArrayView<'_, A, E::Dim> = (&self).broadcast_unwrap(shape.into_dimension());
76-
unsafe { ArrayView::new_(res.ptr.as_ptr(), res.dim, res.strides) }
76+
unsafe { ArrayView::new(res.ptr, res.dim, res.strides) }
7777
}
7878
private_impl! {}
7979
}

0 commit comments

Comments
 (0)