Skip to content

NonZero start #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ sudo: required
dist: trusty
matrix:
include:
- rust: 1.32.0
- rust: 1.34.0
env:
- FEATURES='test docs'
- rust: stable
Expand Down
3 changes: 1 addition & 2 deletions src/dimension/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,7 @@ fn to_abs_slice(axis_len: usize, slice: Slice) -> (usize, usize, isize) {
end,
axis_len,
);
ndassert!(step != 0, "Slice stride must not be zero");
(start, end, step)
(start, end, step.get())
}

/// Modify dimension, stride and return data pointer offset
Expand Down
2 changes: 1 addition & 1 deletion src/impl_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ where
.enumerate()
.for_each(|(axis, &slice_or_index)| match slice_or_index {
SliceOrIndex::Slice { start, end, step } => {
self.slice_axis_inplace(Axis(axis), Slice { start, end, step })
self.slice_axis_inplace(Axis(axis), Slice::new(start, end, step))
}
SliceOrIndex::Index(index) => {
let i_usize = abs_index(self.len_of(Axis(axis)), index);
Expand Down
59 changes: 21 additions & 38 deletions src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::error::{ErrorKind, ShapeError};
use crate::{ArrayView, ArrayViewMut, Dimension, RawArrayViewMut};
use std::fmt;
use std::marker::PhantomData;
use std::num::NonZeroIsize;
use std::ops::{Deref, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive};

/// A slice (range with step size).
Expand All @@ -35,7 +36,7 @@ use std::ops::{Deref, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, Rang
pub struct Slice {
pub start: isize,
pub end: Option<isize>,
pub step: isize,
pub step: NonZeroIsize,
}

impl Slice {
Expand All @@ -47,8 +48,11 @@ impl Slice {
/// `step` must be nonzero.
/// (This method checks with a debug assertion that `step` is not zero.)
pub fn new(start: isize, end: Option<isize>, step: isize) -> Slice {
debug_assert_ne!(step, 0, "Slice::new: step must be nonzero");
Slice { start, end, step }
Slice {
start,
end,
step: NonZeroIsize::new(step).expect("Slice::new: step must be nonzero"),
}
}

/// Create a new `Slice` with the given step size (multiplied with the
Expand All @@ -58,9 +62,8 @@ impl Slice {
/// (This method checks with a debug assertion that `step` is not zero.)
#[inline]
pub fn step_by(self, step: isize) -> Self {
debug_assert_ne!(step, 0, "Slice::step_by: step must be nonzero");
Slice {
step: self.step * step,
step: NonZeroIsize::new(self.step.get() * step).expect("step must be nonzero"),
..self
}
}
Expand Down Expand Up @@ -130,7 +133,6 @@ impl SliceOrIndex {
/// (This method checks with a debug assertion that `step` is not zero.)
#[inline]
pub fn step_by(self, step: isize) -> Self {
debug_assert_ne!(step, 0, "SliceOrIndex::step_by: step must be nonzero");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I don't think this should be removed yet; I'll add back post-review

match self {
SliceOrIndex::Slice {
start,
Expand Down Expand Up @@ -172,57 +174,42 @@ macro_rules! impl_slice_variant_from_range {
impl From<Range<$index>> for $self {
#[inline]
fn from(r: Range<$index>) -> $self {
$constructor {
start: r.start as isize,
end: Some(r.end as isize),
step: 1,
}
Slice::new(r.start as isize, Some(r.end as isize), 1).into()
}
}

impl From<RangeInclusive<$index>> for $self {
#[inline]
fn from(r: RangeInclusive<$index>) -> $self {
let end = *r.end() as isize;
$constructor {
start: *r.start() as isize,
end: if end == -1 { None } else { Some(end + 1) },
step: 1,
}
Slice::new(
*r.start() as isize,
if end == -1 { None } else { Some(end + 1) },
1,
)
.into()
}
}

impl From<RangeFrom<$index>> for $self {
#[inline]
fn from(r: RangeFrom<$index>) -> $self {
$constructor {
start: r.start as isize,
end: None,
step: 1,
}
Slice::new(r.start as isize, None, 1).into()
}
}

impl From<RangeTo<$index>> for $self {
#[inline]
fn from(r: RangeTo<$index>) -> $self {
$constructor {
start: 0,
end: Some(r.end as isize),
step: 1,
}
Slice::new(0, Some(r.end as isize), 1).into()
}
}

impl From<RangeToInclusive<$index>> for $self {
#[inline]
fn from(r: RangeToInclusive<$index>) -> $self {
let end = r.end as isize;
$constructor {
start: 0,
end: if end == -1 { None } else { Some(end + 1) },
step: 1,
}
Slice::new(0, if end == -1 { None } else { Some(end + 1) }, 1).into()
}
}
};
Expand All @@ -237,11 +224,7 @@ impl_slice_variant_from_range!(SliceOrIndex, SliceOrIndex::Slice, i32);
impl From<RangeFull> for Slice {
#[inline]
fn from(_: RangeFull) -> Slice {
Slice {
start: 0,
end: None,
step: 1,
}
Slice::new(0, None, 1)
}
}

Expand All @@ -262,7 +245,7 @@ impl From<Slice> for SliceOrIndex {
SliceOrIndex::Slice {
start: s.start,
end: s.end,
step: s.step,
step: s.step.into(),
}
}
}
Expand Down Expand Up @@ -610,7 +593,7 @@ macro_rules! s(
};
// convert range/index and step into SliceOrIndex
(@convert $r:expr, $s:expr) => {
<$crate::SliceOrIndex as ::std::convert::From<_>>::from($r).step_by($s as isize)
<$crate::SliceOrIndex as ::std::convert::From<_>>::from($r).step_by($s)
};
($($t:tt)*) => {
// The extra `*&` is a workaround for this compiler bug:
Expand Down