Merge pull request #738 from jturner314/relax-alignment

Remove alignment restriction on RawArrayView/Mut

src/impl_raw_views.rs

@@ -33,8 +33,8 @@ where
     ///
     /// Unsafe because caller is responsible for ensuring all of the following:
     ///
-    /// * `ptr` must be non-null and aligned, and it must be safe to
-    ///   [`.offset()`] `ptr` by zero.
+    /// * `ptr` must be non-null, and it must be safe to [`.offset()`] `ptr` by
+    ///   zero.
     ///
     /// * It must be safe to [`.offset()`] the pointer repeatedly along all
     ///   axes and calculate the `count`s for the `.offset()` calls without
@@ -70,7 +70,6 @@ where
         let strides = shape.strides;
         if cfg!(debug_assertions) {
             assert!(!ptr.is_null(), "The pointer must be non-null.");
-            assert!(is_aligned(ptr), "The pointer must be aligned.");
             dimension::max_abs_offset_check_overflow::<A, _>(&dim, &strides).unwrap();
         }
         RawArrayView::new_(ptr, dim, strides)
@@ -80,9 +79,14 @@ where
     ///
     /// **Warning** from a safety standpoint, this is equivalent to
     /// dereferencing a raw pointer for every element in the array. You must
-    /// ensure that all of the data is valid and choose the correct lifetime.
+    /// ensure that all of the data is valid, ensure that the pointer is
+    /// aligned, and choose the correct lifetime.
     #[inline]
     pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> {
+        debug_assert!(
+            is_aligned(self.ptr.as_ptr()),
+            "The pointer must be aligned."
+        );
         ArrayView::new(self.ptr, self.dim, self.strides)
     }
 
@@ -130,12 +134,6 @@ where
             "size mismatch in raw view cast"
         );
         let ptr = self.ptr.cast::<B>();
-        debug_assert!(
-            is_aligned(ptr.as_ptr()),
-            "alignment mismatch in raw view cast"
-        );
-        /* Alignment checked with debug assertion: alignment could be dynamically correct,
-         * and we don't have a check that compiles out for that. */
         unsafe { RawArrayView::new(ptr, self.dim, self.strides) }
     }
 }
@@ -167,8 +165,8 @@ where
     ///
     /// Unsafe because caller is responsible for ensuring all of the following:
     ///
-    /// * `ptr` must be non-null and aligned, and it must be safe to
-    ///   [`.offset()`] `ptr` by zero.
+    /// * `ptr` must be non-null, and it must be safe to [`.offset()`] `ptr` by
+    ///   zero.
     ///
     /// * It must be safe to [`.offset()`] the pointer repeatedly along all
     ///   axes and calculate the `count`s for the `.offset()` calls without
@@ -204,7 +202,6 @@ where
         let strides = shape.strides;
         if cfg!(debug_assertions) {
             assert!(!ptr.is_null(), "The pointer must be non-null.");
-            assert!(is_aligned(ptr), "The pointer must be aligned.");
             dimension::max_abs_offset_check_overflow::<A, _>(&dim, &strides).unwrap();
         }
         RawArrayViewMut::new_(ptr, dim, strides)
@@ -220,19 +217,29 @@ where
     ///
     /// **Warning** from a safety standpoint, this is equivalent to
     /// dereferencing a raw pointer for every element in the array. You must
-    /// ensure that all of the data is valid and choose the correct lifetime.
+    /// ensure that all of the data is valid, ensure that the pointer is
+    /// aligned, and choose the correct lifetime.
     #[inline]
     pub unsafe fn deref_into_view<'a>(self) -> ArrayView<'a, A, D> {
+        debug_assert!(
+            is_aligned(self.ptr.as_ptr()),
+            "The pointer must be aligned."
+        );
         ArrayView::new(self.ptr, self.dim, self.strides)
     }
 
     /// Converts to a mutable view of the array.
     ///
     /// **Warning** from a safety standpoint, this is equivalent to
     /// dereferencing a raw pointer for every element in the array. You must
-    /// ensure that all of the data is valid and choose the correct lifetime.
+    /// ensure that all of the data is valid, ensure that the pointer is
+    /// aligned, and choose the correct lifetime.
     #[inline]
     pub unsafe fn deref_into_view_mut<'a>(self) -> ArrayViewMut<'a, A, D> {
+        debug_assert!(
+            is_aligned(self.ptr.as_ptr()),
+            "The pointer must be aligned."
+        );
         ArrayViewMut::new(self.ptr, self.dim, self.strides)
     }
 
@@ -267,12 +274,6 @@ where
             "size mismatch in raw view cast"
         );
         let ptr = self.ptr.cast::<B>();
-        debug_assert!(
-            is_aligned(ptr.as_ptr()),
-            "alignment mismatch in raw view cast"
-        );
-        /* Alignment checked with debug assertion: alignment could be dynamically correct,
-         * and we don't have a check that compiles out for that. */
         unsafe { RawArrayViewMut::new(ptr, self.dim, self.strides) }
     }
 }

src/lib.rs

@@ -1106,10 +1106,12 @@ pub type Ixs = isize;
 //      `dim`, and `strides` must be exclusively borrowed and not aliased by
 //      multiple indices.
 //
-// 2. `ptr` must be non-null and aligned, and it must be safe to [`.offset()`]
-//    `ptr` by zero.
+// 2. If the type of `data` implements `Data`, then `ptr` must be aligned.
 //
-// 3. It must be safe to [`.offset()`] the pointer repeatedly along all axes
+// 3. `ptr` must be non-null, and it must be safe to [`.offset()`] `ptr` by
+//    zero.
+//
+// 4. It must be safe to [`.offset()`] the pointer repeatedly along all axes
 //    and calculate the `count`s for the `.offset()` calls without overflow,
 //    even if the array is empty or the elements are zero-sized.
 //
@@ -1177,13 +1179,13 @@ pub type Ixs = isize;
 //    `.offset()` at all, even by zero bytes, but the implementation of
 //    `Vec<A>` does this, so we can too. See rust-lang/rust#54857 for details.)
 //
-// 4. The product of non-zero axis lengths must not exceed `isize::MAX`. (This
+// 5. The product of non-zero axis lengths must not exceed `isize::MAX`. (This
 //    also implies that the length of any individual axis must not exceed
 //    `isize::MAX`, and an array can contain at most `isize::MAX` elements.)
 //    This constraint makes various calculations easier because they don't have
 //    to worry about overflow and axis lengths can be freely cast to `isize`.
 //
-// Constraints 2–4 are carefully designed such that if they're upheld for the
+// Constraints 2–5 are carefully designed such that if they're upheld for the
 // array, they're also upheld for any subset of axes of the array as well as
 // slices/subviews/reshapes of the array. This is important for iterators that
 // produce subviews (and other similar cases) to be safe without extra (easy to
@@ -1209,8 +1211,8 @@ where
     /// Data buffer / ownership information. (If owned, contains the data
     /// buffer; if borrowed, contains the lifetime and mutability.)
     data: S,
-    /// A non-null and aligned pointer into the buffer held by `data`; may
-    /// point anywhere in its range.
+    /// A non-null pointer into the buffer held by `data`; may point anywhere
+    /// in its range. If `S: Data`, this pointer must be aligned.
     ptr: std::ptr::NonNull<S::Elem>,
     /// The lengths of the axes.
     dim: D,
@@ -1331,7 +1333,7 @@ pub type ArrayViewMut<'a, A, D> = ArrayBase<ViewRepr<&'a mut A>, D>;
 /// conversion into an [`ArrayView`]. The relationship between `RawArrayView`
 /// and [`ArrayView`] is somewhat analogous to the relationship between `*const
 /// T` and `&T`, but `RawArrayView` has additional requirements that `*const T`
-/// does not, such as alignment and non-nullness.
+/// does not, such as non-nullness.
 ///
 /// [`ArrayView`]: type.ArrayView.html
 ///
@@ -1356,8 +1358,7 @@ pub type RawArrayView<A, D> = ArrayBase<RawViewRepr<*const A>, D>;
 /// unsafe conversion into an [`ArrayViewMut`]. The relationship between
 /// `RawArrayViewMut` and [`ArrayViewMut`] is somewhat analogous to the
 /// relationship between `*mut T` and `&mut T`, but `RawArrayViewMut` has
-/// additional requirements that `*mut T` does not, such as alignment and
-/// non-nullness.
+/// additional requirements that `*mut T` does not, such as non-nullness.
 ///
 /// [`ArrayViewMut`]: type.ArrayViewMut.html
 ///

tests/raw_views.rs

@@ -2,8 +2,6 @@ use ndarray::prelude::*;
 use ndarray::Zip;
 
 use std::cell::Cell;
-#[cfg(debug_assertions)]
-use std::mem;
 
 #[test]
 fn raw_view_cast_cell() {
@@ -59,28 +57,27 @@ fn raw_view_mut_invalid_size_cast() {
 }
 
 #[test]
-#[cfg(debug_assertions)]
-#[should_panic = "alignment mismatch"]
-fn raw_view_invalid_align_cast() {
-    #[derive(Copy, Clone, Debug)]
-    #[repr(transparent)]
-    struct A([u8; 16]);
-    #[derive(Copy, Clone, Debug)]
-    #[repr(transparent)]
-    struct B([f64; 2]);
-
+fn raw_view_misaligned() {
+    let data: [u16; 2] = [0x0011, 0x2233];
+    let ptr: *const u16 = data.as_ptr();
     unsafe {
-        const LEN: usize = 16;
-        let mut buffer = [0u8; mem::size_of::<A>() * (LEN + 1)];
-        // Take out a slice of buffer as &[A] which is misaligned for B
-        let mut ptr = buffer.as_mut_ptr();
-        if ptr as usize % mem::align_of::<B>() == 0 {
-            ptr = ptr.add(1);
-        }
-
-        let view = RawArrayViewMut::from_shape_ptr(LEN, ptr as *mut A);
+        let misaligned_ptr = (ptr as *const u8).add(1) as *const u16;
+        RawArrayView::from_shape_ptr(1, misaligned_ptr);
+    }
+}
 
-        // misaligned cast - test debug assertion
-        view.cast::<B>();
+#[test]
+#[cfg(debug_assertions)]
+#[should_panic = "The pointer must be aligned."]
+fn raw_view_deref_into_view_misaligned() {
+    fn misaligned_deref(data: &[u16; 2]) -> ArrayView1<'_, u16> {
+        let ptr: *const u16 = data.as_ptr();
+        unsafe {
+            let misaligned_ptr = (ptr as *const u8).add(1) as *const u16;
+            let raw_view = RawArrayView::from_shape_ptr(1, misaligned_ptr);
+            raw_view.deref_into_view()
+        }
     }
+    let data: [u16; 2] = [0x0011, 0x2233];
+    misaligned_deref(&data);
 }