Skip to content

Make TrustedStep require Copy #112083

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

Merged
merged 2 commits into from
May 30, 2023
Merged
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
25 changes: 13 additions & 12 deletions library/core/src/iter/range.rs
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
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 {}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
The loop took around 13s
The loop took around 7s
108 changes: 78 additions & 30 deletions tests/mir-opt/pre-codegen/range_iter.forward_loop.PreCodegen.after.mir
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
}
}
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
}
}