Skip to content

Commit

Permalink
Make TrustedStep require Copy
Browse files Browse the repository at this point in the history 
All the implementations of the trait already are `Copy`, and this seems to be enough to simplify the implementations enough to make the MIR inliner willing to inline basics like `Range::next`.
  • Loading branch information
@scottmcm
scottmcm committed May 29, 2023
1 parent dc0943d commit 11fa176
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 45 deletions.
25 changes: 13 additions & 12 deletions library/core/src/iter/range.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -619,25 +619,26 @@ impl<T: TrustedStep> RangeIteratorImpl for ops::Range<T> {
#[inline]
fn spec_next(&mut self) -> Option<T> {
if self.start < self.end {
let old = self.start;
// SAFETY: just checked precondition
let n = unsafe { Step::forward_unchecked(self.start.clone(), 1) };
Some(mem::replace(&mut self.start, n))
self.start = unsafe { Step::forward_unchecked(old, 1) };
Some(old)
} else {
None
}
}

#[inline]
fn spec_nth(&mut self, n: usize) -> Option<T> {
if let Some(plus_n) = Step::forward_checked(self.start.clone(), n) {
if let Some(plus_n) = Step::forward_checked(self.start, n) {
if plus_n < self.end {
// SAFETY: just checked precondition
self.start = unsafe { Step::forward_unchecked(plus_n.clone(), 1) };
self.start = unsafe { Step::forward_unchecked(plus_n, 1) };
return Some(plus_n);
}
}

self.start = self.end.clone();
self.start = self.end;
None
}

Expand All @@ -655,7 +656,7 @@ impl<T: TrustedStep> RangeIteratorImpl for ops::Range<T> {
// then steps_between either returns a bound to which we clamp or returns None which
// together with the initial inequality implies more than usize::MAX steps.
// Otherwise 0 is returned which always safe to use.
self.start = unsafe { Step::forward_unchecked(self.start.clone(), taken) };
self.start = unsafe { Step::forward_unchecked(self.start, taken) };

NonZeroUsize::new(n - taken).map_or(Ok(()), Err)
}
Expand All @@ -664,24 +665,24 @@ impl<T: TrustedStep> RangeIteratorImpl for ops::Range<T> {
fn spec_next_back(&mut self) -> Option<T> {
if self.start < self.end {
// SAFETY: just checked precondition
self.end = unsafe { Step::backward_unchecked(self.end.clone(), 1) };
Some(self.end.clone())
self.end = unsafe { Step::backward_unchecked(self.end, 1) };
Some(self.end)
} else {
None
}
}

#[inline]
fn spec_nth_back(&mut self, n: usize) -> Option<T> {
if let Some(minus_n) = Step::backward_checked(self.end.clone(), n) {
if let Some(minus_n) = Step::backward_checked(self.end, n) {
if minus_n > self.start {
// SAFETY: just checked precondition
self.end = unsafe { Step::backward_unchecked(minus_n, 1) };
return Some(self.end.clone());
return Some(self.end);
}
}

self.end = self.start.clone();
self.end = self.start;
None
}

Expand All @@ -696,7 +697,7 @@ impl<T: TrustedStep> RangeIteratorImpl for ops::Range<T> {
let taken = available.min(n);

// SAFETY: same as the spec_advance_by() implementation
self.end = unsafe { Step::backward_unchecked(self.end.clone(), taken) };
self.end = unsafe { Step::backward_unchecked(self.end, taken) };

NonZeroUsize::new(n - taken).map_or(Ok(()), Err)
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/traits/marker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,4 @@ pub unsafe trait InPlaceIterable: Iterator {}
/// for details. Consumers are free to rely on the invariants in unsafe code.
#[unstable(feature = "trusted_step", issue = "85731")]
#[rustc_specialization_trait]
pub unsafe trait TrustedStep: Step {}
pub unsafe trait TrustedStep: Step + Copy {}
108 changes: 78 additions & 30 deletions tests/mir-opt/pre-codegen/range_iter.forward_loop.PreCodegen.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,32 @@ fn forward_loop(_1: u32, _2: u32, _3: impl Fn(u32)) -> () {
let mut _4: std::ops::Range<u32>; // in scope 0 at $DIR/range_iter.rs:+1:14: +1:24
let mut _5: std::ops::Range<u32>; // in scope 0 at $DIR/range_iter.rs:+1:14: +1:24
let mut _6: &mut std::ops::Range<u32>; // in scope 0 at $DIR/range_iter.rs:+1:14: +1:24
let mut _7: std::option::Option<u32>; // in scope 0 at $DIR/range_iter.rs:+1:14: +1:24
let mut _8: isize; // in scope 0 at $DIR/range_iter.rs:+1:5: +3:6
let mut _10: &impl Fn(u32); // in scope 0 at $DIR/range_iter.rs:+2:9: +2:10
let mut _11: (u32,); // in scope 0 at $DIR/range_iter.rs:+2:9: +2:13
let _12: (); // in scope 0 at $DIR/range_iter.rs:+1:14: +1:24
let mut _10: std::option::Option<u32>; // in scope 0 at $DIR/range_iter.rs:+1:14: +1:24
let mut _13: isize; // in scope 0 at $DIR/range_iter.rs:+1:5: +3:6
let mut _15: &impl Fn(u32); // in scope 0 at $DIR/range_iter.rs:+2:9: +2:10
let mut _16: (u32,); // in scope 0 at $DIR/range_iter.rs:+2:9: +2:13
let _17: (); // in scope 0 at $DIR/range_iter.rs:+1:14: +1:24
scope 1 {
debug iter => _5; // in scope 1 at $DIR/range_iter.rs:+1:14: +1:24
let _9: u32; // in scope 1 at $DIR/range_iter.rs:+1:9: +1:10
let _14: u32; // in scope 1 at $DIR/range_iter.rs:+1:9: +1:10
scope 2 {
debug x => _9; // in scope 2 at $DIR/range_iter.rs:+1:9: +1:10
debug x => _14; // in scope 2 at $DIR/range_iter.rs:+1:9: +1:10
}
scope 4 (inlined iter::range::<impl Iterator for std::ops::Range<u32>>::next) { // at $DIR/range_iter.rs:21:14: 21:24
debug self => _6; // in scope 4 at $SRC_DIR/core/src/iter/range.rs:LL:COL
scope 5 (inlined <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next) { // at $SRC_DIR/core/src/iter/range.rs:LL:COL
debug self => _6; // in scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _7: &u32; // in scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _8: &u32; // in scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _9: bool; // in scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let _11: u32; // in scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _12: u32; // in scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
scope 6 {
debug old => _11; // in scope 6 at $SRC_DIR/core/src/iter/range.rs:LL:COL
scope 7 {
}
}
}
}
}
scope 3 (inlined <std::ops::Range<u32> as IntoIterator>::into_iter) { // at $DIR/range_iter.rs:21:14: 21:24
Expand All @@ -35,57 +48,92 @@ fn forward_loop(_1: u32, _2: u32, _3: impl Fn(u32)) -> () {
}

bb1: {
StorageLive(_7); // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
StorageLive(_10); // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
_6 = &mut _5; // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
_7 = <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next(_6) -> [return: bb2, unwind: bb8]; // scope 4 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_11); // scope 4 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_9); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_7); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_7 = &((*_6).0: u32); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_8); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_8 = &((*_6).1: u32); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_9 = <u32 as PartialOrd>::lt(move _7, move _8) -> [return: bb2, unwind: bb12]; // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/iter/range.rs:LL:COL
// + literal: Const { ty: for<'a> fn(&'a mut std::ops::Range<u32>) -> Option<<std::ops::Range<u32> as iter::range::RangeIteratorImpl>::Item> {<std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next}, val: Value(<ZST>) }
// + literal: Const { ty: for<'a, 'b> fn(&'a u32, &'b u32) -> bool {<u32 as PartialOrd>::lt}, val: Value(<ZST>) }
}

bb2: {
_8 = discriminant(_7); // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
switchInt(move _8) -> [0: bb3, 1: bb5, otherwise: bb7]; // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
StorageDead(_8); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageDead(_7); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
switchInt(move _9) -> [0: bb3, otherwise: bb4]; // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
}

bb3: {
StorageDead(_7); // scope 1 at $DIR/range_iter.rs:+3:5: +3:6
StorageDead(_5); // scope 0 at $DIR/range_iter.rs:+3:5: +3:6
drop(_3) -> bb4; // scope 0 at $DIR/range_iter.rs:+4:1: +4:2
_10 = Option::<u32>::None; // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
goto -> bb6; // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
}

bb4: {
return; // scope 0 at $DIR/range_iter.rs:+4:2: +4:2
_11 = ((*_6).0: u32); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_12); // scope 6 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_12 = <u32 as Step>::forward_unchecked(_11, const 1_usize) -> [return: bb5, unwind: bb12]; // scope 7 at $SRC_DIR/core/src/iter/range.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/iter/range.rs:LL:COL
// + literal: Const { ty: unsafe fn(u32, usize) -> u32 {<u32 as Step>::forward_unchecked}, val: Value(<ZST>) }
}

bb5: {
_9 = ((_7 as Some).0: u32); // scope 1 at $DIR/range_iter.rs:+1:9: +1:10
StorageLive(_10); // scope 2 at $DIR/range_iter.rs:+2:9: +2:10
_10 = &_3; // scope 2 at $DIR/range_iter.rs:+2:9: +2:10
StorageLive(_11); // scope 2 at $DIR/range_iter.rs:+2:9: +2:13
_11 = (_9,); // scope 2 at $DIR/range_iter.rs:+2:9: +2:13
_12 = <impl Fn(u32) as Fn<(u32,)>>::call(move _10, move _11) -> [return: bb6, unwind: bb8]; // scope 2 at $DIR/range_iter.rs:+2:9: +2:13
((*_6).0: u32) = move _12; // scope 6 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageDead(_12); // scope 6 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_10 = Option::<u32>::Some(_11); // scope 6 at $SRC_DIR/core/src/iter/range.rs:LL:COL
goto -> bb6; // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
}

bb6: {
StorageDead(_9); // scope 5 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageDead(_11); // scope 4 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_13 = discriminant(_10); // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
switchInt(move _13) -> [0: bb7, 1: bb9, otherwise: bb11]; // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
}

bb7: {
StorageDead(_10); // scope 1 at $DIR/range_iter.rs:+3:5: +3:6
StorageDead(_5); // scope 0 at $DIR/range_iter.rs:+3:5: +3:6
drop(_3) -> bb8; // scope 0 at $DIR/range_iter.rs:+4:1: +4:2
}

bb8: {
return; // scope 0 at $DIR/range_iter.rs:+4:2: +4:2
}

bb9: {
_14 = ((_10 as Some).0: u32); // scope 1 at $DIR/range_iter.rs:+1:9: +1:10
StorageLive(_15); // scope 2 at $DIR/range_iter.rs:+2:9: +2:10
_15 = &_3; // scope 2 at $DIR/range_iter.rs:+2:9: +2:10
StorageLive(_16); // scope 2 at $DIR/range_iter.rs:+2:9: +2:13
_16 = (_14,); // scope 2 at $DIR/range_iter.rs:+2:9: +2:13
_17 = <impl Fn(u32) as Fn<(u32,)>>::call(move _15, move _16) -> [return: bb10, unwind: bb12]; // scope 2 at $DIR/range_iter.rs:+2:9: +2:13
// mir::Constant
// + span: $DIR/range_iter.rs:22:9: 22:10
// + literal: Const { ty: for<'a> extern "rust-call" fn(&'a impl Fn(u32), (u32,)) -> <impl Fn(u32) as FnOnce<(u32,)>>::Output {<impl Fn(u32) as Fn<(u32,)>>::call}, val: Value(<ZST>) }
}

bb6: {
StorageDead(_11); // scope 2 at $DIR/range_iter.rs:+2:12: +2:13
StorageDead(_10); // scope 2 at $DIR/range_iter.rs:+2:12: +2:13
StorageDead(_7); // scope 1 at $DIR/range_iter.rs:+3:5: +3:6
bb10: {
StorageDead(_16); // scope 2 at $DIR/range_iter.rs:+2:12: +2:13
StorageDead(_15); // scope 2 at $DIR/range_iter.rs:+2:12: +2:13
StorageDead(_10); // scope 1 at $DIR/range_iter.rs:+3:5: +3:6
goto -> bb1; // scope 1 at $DIR/range_iter.rs:+1:5: +3:6
}

bb7: {
bb11: {
unreachable; // scope 1 at $DIR/range_iter.rs:+1:14: +1:24
}

bb8 (cleanup): {
drop(_3) -> [return: bb9, unwind terminate]; // scope 0 at $DIR/range_iter.rs:+4:1: +4:2
bb12 (cleanup): {
drop(_3) -> [return: bb13, unwind terminate]; // scope 0 at $DIR/range_iter.rs:+4:1: +4:2
}

bb9 (cleanup): {
bb13 (cleanup): {
resume; // scope 0 at $DIR/range_iter.rs:+0:1: +4:2
}
}
52 changes: 50 additions & 2 deletions tests/mir-opt/pre-codegen/range_iter.range_iter_next.PreCodegen.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,64 @@ fn range_iter_next(_1: &mut std::ops::Range<u32>) -> Option<u32> {
let mut _0: std::option::Option<u32>; // return place in scope 0 at $DIR/range_iter.rs:+0:48: +0:59
scope 1 (inlined iter::range::<impl Iterator for std::ops::Range<u32>>::next) { // at $DIR/range_iter.rs:11:8: 11:14
debug self => _1; // in scope 1 at $SRC_DIR/core/src/iter/range.rs:LL:COL
scope 2 (inlined <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next) { // at $SRC_DIR/core/src/iter/range.rs:LL:COL
debug self => _1; // in scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _2: &u32; // in scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _3: &u32; // in scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _4: bool; // in scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let _5: u32; // in scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
let mut _6: u32; // in scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
scope 3 {
debug old => _5; // in scope 3 at $SRC_DIR/core/src/iter/range.rs:LL:COL
scope 4 {
}
}
}
}

bb0: {
_0 = <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next(_1) -> bb1; // scope 1 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_5); // scope 1 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_4); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_2); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_2 = &((*_1).0: u32); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_3); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_3 = &((*_1).1: u32); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_4 = <u32 as PartialOrd>::lt(move _2, move _3) -> bb1; // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/iter/range.rs:LL:COL
// + literal: Const { ty: for<'a> fn(&'a mut std::ops::Range<u32>) -> Option<<std::ops::Range<u32> as iter::range::RangeIteratorImpl>::Item> {<std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next}, val: Value(<ZST>) }
// + literal: Const { ty: for<'a, 'b> fn(&'a u32, &'b u32) -> bool {<u32 as PartialOrd>::lt}, val: Value(<ZST>) }
}

bb1: {
StorageDead(_3); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageDead(_2); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
switchInt(move _4) -> [0: bb2, otherwise: bb3]; // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
}

bb2: {
_0 = Option::<u32>::None; // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
goto -> bb5; // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
}

bb3: {
_5 = ((*_1).0: u32); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageLive(_6); // scope 3 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_6 = <u32 as Step>::forward_unchecked(_5, const 1_usize) -> bb4; // scope 4 at $SRC_DIR/core/src/iter/range.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/iter/range.rs:LL:COL
// + literal: Const { ty: unsafe fn(u32, usize) -> u32 {<u32 as Step>::forward_unchecked}, val: Value(<ZST>) }
}

bb4: {
((*_1).0: u32) = move _6; // scope 3 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageDead(_6); // scope 3 at $SRC_DIR/core/src/iter/range.rs:LL:COL
_0 = Option::<u32>::Some(_5); // scope 3 at $SRC_DIR/core/src/iter/range.rs:LL:COL
goto -> bb5; // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
}

bb5: {
StorageDead(_4); // scope 2 at $SRC_DIR/core/src/iter/range.rs:LL:COL
StorageDead(_5); // scope 1 at $SRC_DIR/core/src/iter/range.rs:LL:COL
return; // scope 0 at $DIR/range_iter.rs:+2:2: +2:2
}
}

0 comments on commit 11fa176

Please sign in to comment.