Clarify ArrayBase invariants and fix constructors

Author: jturner314

Cargo.toml

@@ -39,6 +39,7 @@ serde = { version = "1.0", optional = true }
 
 [dev-dependencies]
 defmac = "0.1"
+quickcheck = { version = "0.7.2", default-features = false }
 rawpointer = "0.1"
 
 [features]

src/array_serialize.rs

@@ -74,7 +74,10 @@ impl<A, S, D> Decodable for ArrayBase<S, D>
             let elements = try!(
                 d.read_struct_field("data", 2, |d| {
                     d.read_seq(|d, len| {
-                        if len != dim.size() {
+                        let dim_size = dim
+                            .size_checked()
+                            .ok_or_else(|| from_kind(ErrorKind::Overflow))?;
+                        if len != dim_size {
                             Err(d.error("data and dimension must match in size"))
                         } else {
                             let mut elements = Vec::with_capacity(len);

src/arraytraits.rs

@@ -216,11 +216,17 @@ pub const ARRAY_FORMAT_VERSION: u8 = 1u8;
 /// Implementation of `ArrayView::from(&S)` where `S` is a slice or slicable.
 ///
 /// Create a one-dimensional read-only array view of the data in `slice`.
+///
+/// **Panics** if the slice length is greater than `isize::MAX`.
 impl<'a, A, Slice: ?Sized> From<&'a Slice> for ArrayView<'a, A, Ix1>
     where Slice: AsRef<[A]>
 {
     fn from(slice: &'a Slice) -> Self {
         let xs = slice.as_ref();
+        assert!(
+            xs.len() <= ::std::isize::MAX as usize,
+            "Slice length must fit in `isize`.",
+        );
         unsafe {
             Self::from_shape_ptr(xs.len(), xs.as_ptr())
         }
@@ -242,11 +248,17 @@ impl<'a, A, S, D> From<&'a ArrayBase<S, D>> for ArrayView<'a, A, D>
 /// Implementation of `ArrayViewMut::from(&mut S)` where `S` is a slice or slicable.
 ///
 /// Create a one-dimensional read-write array view of the data in `slice`.
+///
+/// **Panics** if the slice length is greater than `isize::MAX`.
 impl<'a, A, Slice: ?Sized> From<&'a mut Slice> for ArrayViewMut<'a, A, Ix1>
     where Slice: AsMut<[A]>
 {
     fn from(slice: &'a mut Slice) -> Self {
         let xs = slice.as_mut();
+        assert!(
+            xs.len() <= ::std::isize::MAX as usize,
+            "Slice length must fit in `isize`.",
+        );
         unsafe {
             Self::from_shape_ptr(xs.len(), xs.as_mut_ptr())
         }

src/dimension/dimension_trait.rs

@@ -83,9 +83,27 @@ pub trait Dimension : Clone + Eq + Debug + Send + Sync + Default +
         self.slice().iter().fold(1, |s, &a| s * a as usize)
     }
 
-    /// Compute the size while checking for overflow.
+    /// Compute the size (number of elements) while checking for overflow.
+    ///
+    /// Returns `None` if the product of non-zero axis lengths overflows
+    /// `isize`.
+    ///
+    /// If `dim.size_checked()` returns `Some(size)`, a `Vec` is created of
+    /// length `size`, and `strides` are created with `self.default_strides()`
+    /// or `self.fortran_strides()`, then the invariants are met to construct
+    /// an owned `Array` from the `Vec`, `dim`, and `strides`. (See the docs of
+    /// `can_index_slice_not_custom` for a slightly more general case.)
     fn size_checked(&self) -> Option<usize> {
-        self.slice().iter().fold(Some(1), |s, &a| s.and_then(|s_| s_.checked_mul(a)))
+        let size_nonzero = self
+            .slice()
+            .iter()
+            .filter(|&&d| d != 0)
+            .try_fold(1usize, |acc, &d| acc.checked_mul(d))?;
+        if size_nonzero > ::std::isize::MAX as usize {
+            None
+        } else {
+            Some(self.size())
+        }
     }
 
     #[doc(hidden)]
@@ -109,12 +127,17 @@ pub trait Dimension : Clone + Eq + Debug + Send + Sync + Default +
         self.slice() == rhs.slice()
     }
 
+    /// Returns the strides for a standard layout array with the given shape.
+    ///
+    /// If the array is non-empty, the strides result in contiguous layout; if
+    /// the array is empty, the strides are all zeros.
     #[doc(hidden)]
     fn default_strides(&self) -> Self {
         // Compute default array strides
         // Shape (a, b, c) => Give strides (b * c, c, 1)
-        let mut strides = self.clone();
-        {
+        let mut strides = Self::zeros(self.ndim());
+        // For empty arrays, use all zero strides.
+        if self.slice().iter().all(|&d| d != 0) {
             let mut it = strides.slice_mut().iter_mut().rev();
             // Set first element to 1
             while let Some(rs) = it.next() {
@@ -130,12 +153,17 @@ pub trait Dimension : Clone + Eq + Debug + Send + Sync + Default +
         strides
     }
 
+    /// Returns the strides for a Fortran layout array with the given shape.
+    ///
+    /// If the array is non-empty, the strides result in contiguous layout; if
+    /// the array is empty, the strides are all zeros.
     #[doc(hidden)]
     fn fortran_strides(&self) -> Self {
         // Compute fortran array strides
         // Shape (a, b, c) => Give strides (1, a, a * b)
-        let mut strides = self.clone();
-        {
+        let mut strides = Self::zeros(self.ndim());
+        // For empty arrays, use all zero strides.
+        if self.slice().iter().all(|&d| d != 0) {
             let mut it = strides.slice_mut().iter_mut();
             // Set first element to 1
             while let Some(rs) = it.next() {
@@ -428,11 +456,22 @@ impl Dimension for Dim<[Ix; 1]> {
     #[inline]
     fn size(&self) -> usize { get!(self, 0) }
     #[inline]
-    fn size_checked(&self) -> Option<usize> { Some(get!(self, 0)) }
+    fn size_checked(&self) -> Option<usize> {
+        let size = get!(self, 0);
+        if size <= ::std::isize::MAX as usize {
+            Some(size)
+        } else {
+            None
+        }
+    }
 
     #[inline]
     fn default_strides(&self) -> Self {
-        Ix1(1)
+        if get!(self, 0) == 0 {
+            Ix1(0)
+        } else {
+            Ix1(1)
+        }
     }
 
     #[inline]
@@ -533,7 +572,15 @@ impl Dimension for Dim<[Ix; 2]> {
     fn size_checked(&self) -> Option<usize> {
         let m = get!(self, 0);
         let n = get!(self, 1);
-        (m as usize).checked_mul(n as usize)
+        let size = m.checked_mul(n)?;
+        if m <= ::std::isize::MAX as usize
+            && n <= ::std::isize::MAX as usize
+            && size <= ::std::isize::MAX as usize
+        {
+            Some(size)
+        } else {
+            None
+        }
     }
 
     #[inline]
@@ -548,13 +595,23 @@ impl Dimension for Dim<[Ix; 2]> {
 
     #[inline]
     fn default_strides(&self) -> Self {
-        // Compute default array strides
-        // Shape (a, b, c) => Give strides (b * c, c, 1)
-        Ix2(get!(self, 1), 1)
+        let m = get!(self, 0);
+        let n = get!(self, 1);
+        if m == 0 || n == 0 {
+            Ix2(0, 0)
+        } else {
+            Ix2(n, 1)
+        }
     }
     #[inline]
     fn fortran_strides(&self) -> Self {
-        Ix2(1, get!(self, 0))
+        let m = get!(self, 0);
+        let n = get!(self, 1);
+        if m == 0 || n == 0 {
+            Ix2(0, 0)
+        } else {
+            Ix2(1, m)
+        }
     }
 
     #[inline]