Skip to content

Commit 982b413

Browse files
mariusaefacebook-github-bot
authored andcommitted
ndslice: make iterators owned (#734)
Summary: Pull Request resolved: #734 Pull Request resolved: #713 References make them needlessly difficult to compose in practice. For example, it is very hard to build an iterator that composes on slice iterators, without creating an explicit intermediate object first. Reviewed By: shayne-fletcher Differential Revision: D79400562 fbshipit-source-id: 2cf74bea92039d2d7535d02bdb800daa3899989a
1 parent 35d6478 commit 982b413

File tree

4 files changed

+28
-28
lines changed

4 files changed

+28
-28
lines changed

hyperactor_mesh/src/mesh.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub trait Mesh {
5555
/// An iterator over the nodes of a mesh.
5656
pub struct MeshIter<'a, M: Mesh + ?Sized> {
5757
mesh: &'a M,
58-
slice_iter: SliceIterator<'a>,
58+
slice_iter: SliceIterator,
5959
}
6060

6161
impl<M: Mesh> Iterator for MeshIter<'_, M> {

ndslice/src/shape.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ impl Shape {
232232
/// `[2, 2, 8]` shape.
233233
pub struct SelectIterator<'a> {
234234
shape: &'a Shape,
235-
iter: DimSliceIterator<'a>,
235+
iter: DimSliceIterator,
236236
}
237237

238238
impl<'a> Iterator for SelectIterator<'a> {

ndslice/src/slice.rs

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -385,15 +385,15 @@ impl Slice {
385385
/// Iterator over the slice's indices.
386386
pub fn iter(&self) -> SliceIterator {
387387
SliceIterator {
388-
slice: self,
389-
pos: CartesianIterator::new(&self.sizes),
388+
slice: self.clone(),
389+
pos: CartesianIterator::new(self.sizes.clone()),
390390
}
391391
}
392392

393393
/// Iterator over sub-dimensions of the slice.
394394
pub fn dim_iter(&self, dims: usize) -> DimSliceIterator {
395395
DimSliceIterator {
396-
pos: CartesianIterator::new(&self.sizes[0..dims]),
396+
pos: CartesianIterator::new(self.sizes[0..dims].to_vec()),
397397
}
398398
}
399399

@@ -607,7 +607,7 @@ impl Slice {
607607

608608
// Validate that every address in the new view maps to a valid
609609
// coordinate in base.
610-
for coord in CartesianIterator::new(new_sizes) {
610+
for coord in CartesianIterator::new(new_sizes.to_vec()) {
611611
#[allow(clippy::identity_op)]
612612
let offset_in_view = 0 + coord
613613
.iter()
@@ -662,20 +662,20 @@ impl std::fmt::Display for Slice {
662662
}
663663
}
664664

665-
impl<'a> IntoIterator for &'a Slice {
665+
impl IntoIterator for &Slice {
666666
type Item = usize;
667-
type IntoIter = SliceIterator<'a>;
667+
type IntoIter = SliceIterator;
668668
fn into_iter(self) -> Self::IntoIter {
669669
self.iter()
670670
}
671671
}
672672

673-
pub struct SliceIterator<'a> {
674-
pub(crate) slice: &'a Slice,
675-
pos: CartesianIterator<'a>,
673+
pub struct SliceIterator {
674+
pub(crate) slice: Slice,
675+
pos: CartesianIterator,
676676
}
677677

678-
impl<'a> Iterator for SliceIterator<'a> {
678+
impl Iterator for SliceIterator {
679679
type Item = usize;
680680

681681
fn next(&mut self) -> Option<Self::Item> {
@@ -694,11 +694,11 @@ impl<'a> Iterator for SliceIterator<'a> {
694694
///
695695
/// Coordinates are yielded in row-major order (last dimension varies
696696
/// fastest).
697-
pub struct DimSliceIterator<'a> {
698-
pos: CartesianIterator<'a>,
697+
pub struct DimSliceIterator {
698+
pos: CartesianIterator,
699699
}
700700

701-
impl<'a> Iterator for DimSliceIterator<'a> {
701+
impl Iterator for DimSliceIterator {
702702
type Item = Vec<usize>;
703703

704704
fn next(&mut self) -> Option<Self::Item> {
@@ -712,25 +712,25 @@ impl<'a> Iterator for DimSliceIterator<'a> {
712712
/// `dims`, where each coordinate lies in `[0..dims[i])`.
713713
/// # Example
714714
/// ```ignore
715-
/// let iter = CartesianIterator::new(&[2, 3]);
715+
/// let iter = CartesianIterator::new(vec![2, 3]);
716716
/// let coords: Vec<_> = iter.collect();
717717
/// assert_eq!(coords, vec![
718718
/// vec![0, 0], vec![0, 1], vec![0, 2],
719719
/// vec![1, 0], vec![1, 1], vec![1, 2],
720720
/// ]);
721721
/// ```
722-
pub(crate) struct CartesianIterator<'a> {
723-
dims: &'a [usize],
722+
pub(crate) struct CartesianIterator {
723+
dims: Vec<usize>,
724724
index: usize,
725725
}
726726

727-
impl<'a> CartesianIterator<'a> {
728-
pub(crate) fn new(dims: &'a [usize]) -> Self {
727+
impl CartesianIterator {
728+
pub(crate) fn new(dims: Vec<usize>) -> Self {
729729
CartesianIterator { dims, index: 0 }
730730
}
731731
}
732732

733-
impl<'a> Iterator for CartesianIterator<'a> {
733+
impl Iterator for CartesianIterator {
734734
type Item = Vec<usize>;
735735

736736
fn next(&mut self) -> Option<Self::Item> {
@@ -804,7 +804,7 @@ mod tests {
804804
#[test]
805805
fn test_cartesian_iterator() {
806806
let dims = vec![2, 2, 2];
807-
let iter = CartesianIterator::new(&dims);
807+
let iter = CartesianIterator::new(dims);
808808
let products: Vec<Vec<usize>> = iter.collect();
809809
assert_eq!(
810810
products,

ndslice/src/view.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ impl Extent {
176176
pub fn iter(&self) -> ExtentIterator {
177177
ExtentIterator {
178178
extent: self,
179-
pos: CartesianIterator::new(self.sizes()),
179+
pos: CartesianIterator::new(self.sizes().to_vec()),
180180
}
181181
}
182182
}
@@ -197,7 +197,7 @@ impl std::fmt::Display for Extent {
197197
/// An iterator for points in an extent.
198198
pub struct ExtentIterator<'a> {
199199
extent: &'a Extent,
200-
pos: CartesianIterator<'a>,
200+
pos: CartesianIterator,
201201
}
202202

203203
impl<'a> Iterator for ExtentIterator<'a> {
@@ -412,12 +412,12 @@ impl View {
412412
}
413413

414414
/// The iterator over views.
415-
pub struct ViewIterator<'a> {
416-
extent: Extent, // Note that `extent` and...
417-
pos: SliceIterator<'a>, // ... `pos` share the same `Slice`.
415+
pub struct ViewIterator {
416+
extent: Extent, // Note that `extent` and...
417+
pos: SliceIterator, // ... `pos` share the same `Slice`.
418418
}
419419

420-
impl<'a> Iterator for ViewIterator<'a> {
420+
impl Iterator for ViewIterator {
421421
type Item = (Point, usize);
422422

423423
fn next(&mut self) -> Option<Self::Item> {

0 commit comments

Comments
 (0)