Skip to content

iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>#159518

Open
Lfan-ke wants to merge 1 commit into
rust-lang:mainfrom
Lfan-ke:fix/step-by-range-iter-specialization
Open

iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>#159518
Lfan-ke wants to merge 1 commit into
rust-lang:mainfrom
Lfan-ke:fix/step-by-range-iter-specialization

Conversation

@Lfan-ke

@Lfan-ke Lfan-ke commented Jul 18, 2026

Copy link
Copy Markdown

StepBy<Range<{integer}>> already has a fast path that converts the range
into a yield-count before iteration, avoiding per-step overflow checks and
enabling better loop structure for the optimizer. StepBy<RangeIter<{integer}>>
did not benefit from the same optimization even though RangeIter<T> is a thin
newtype wrapper around legacy::Range<T>.

Changes

  • library/core/src/range/iter.rs: make RangeIter<A>.0 pub(crate) so the
    step_by adapter can access the inner legacy range's start/end fields.
  • library/core/src/iter/adapters/step_by.rs: add spec_int_ranges_new! and
    spec_int_ranges_new_r! macros that mirror the existing spec_int_ranges! /
    spec_int_ranges_r! implementations. Forward specialization covers u8 u16 u32 u64 usize (pointer-width-gated for u32/u64); backward specialization is
    limited to u8 u16 usize since RangeIter<u32> / RangeIter<u64> do not
    implement ExactSizeIterator.
  • library/coretests/tests/iter/adapters/step_by.rs: add
    test_step_by_new_range_iter covering forward, backward, and interleaved
    iteration via the new-range iterator.

Motivation

Fixes #157754. The inner loop for core::range::Range::from(0..n).into_iter().step_by(2) currently generates noticeably worse assembly than the equivalent (0..n).step_by(2) because specialization did not cover RangeIter.

r? libs

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 18, 2026
@rustbot

rustbot commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @Darksonn (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Why was this reviewer chosen?

The reviewer was selected based on:

  • libs expanded to 12 candidates
  • Random selection from 6 candidates

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Comment on lines +602 to +612
fn spec_next(&mut self) -> Option<$t> {
let step = <$t>::try_from(self.original_step().get()).unwrap_or(<$t>::MAX);
let remaining = self.iter.0.end;
if remaining > 0 {
let val = self.iter.0.start;
self.iter.0.start = val.wrapping_add(step);
self.iter.0.end = remaining - 1;
Some(val)
} else {
None
}

@Darksonn Darksonn Jul 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would this work?

Suggested change
fn spec_next(&mut self) -> Option<$t> {
let step = <$t>::try_from(self.original_step().get()).unwrap_or(<$t>::MAX);
let remaining = self.iter.0.end;
if remaining > 0 {
let val = self.iter.0.start;
self.iter.0.start = val.wrapping_add(step);
self.iter.0.end = remaining - 1;
Some(val)
} else {
None
}
fn spec_next(&mut self) -> Option<$t> {
self.legacy.spec_next()

View changes since the review

@Lfan-ke
Lfan-ke force-pushed the fix/step-by-range-iter-specialization branch from e69b0ff to c555bd0 Compare July 25, 2026 13:28
@Lfan-ke

Lfan-ke commented Jul 25, 2026

Copy link
Copy Markdown
Author

Good call on cutting the duplication. Direct delegation like self.legacy.spec_next() doesn't quite work here though: there's no inner StepBy<Range> to forward to. StepBy keeps the step state (step_minus_one, first_take) in the wrapper rather than in the iterator, so StepBy<RangeIter<T>> isn't a StepBy<Range<T>> plus a newtype, and since StepBy is repr(Rust) reinterpreting one as the other isn't sound.

Instead I unified the two macros. RangeIter<T> is a newtype over the same legacy Range<T> the existing specialization already drives, so I added a small AsLegacyRange accessor (identity for Range, &self.0 for RangeIter) and made spec_int_ranges! / spec_int_ranges_r! take the iterator type as a parameter. Both specializations now come from one macro body; the accessor is #[inline] and optimizes away, so the Range codegen is unchanged. That drops the duplicated spec_int_ranges_new! / _new_r! (~145 lines).

I also squashed the two commits and moved the issue reference out of the commit message into the PR description, per the earlier bot warning. Let me know if you'd prefer a different shape.

@Darksonn Darksonn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

// `Range<u64>`, nor for `RangeIter<u32>` / `RangeIter<u64>`, so those are
// excluded from the backward specialization.
#[cfg(target_pointer_width = "64")]
spec_int_ranges!(Range; u8 u16 u32 u64 usize);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it'd be easier to read if we grouped them.

#[cfg(target_pointer_width = "64")]
mod step_by_spec {
    spec_int_ranges!(Range; u8 u16 u32 u64 usize);
    spec_int_ranges!(RangeIter; u8 u16 u32 u64 usize);
    spec_int_ranges_r!(Range; u8 u16 u32 usize);
    spec_int_ranges_r!(RangeIter; u8 u16 usize);
}

#[cfg(target_pointer_width = "32")]
mod step_by_spec {
    ...
}

#[cfg(target_pointer_width = "16")]
mod step_by_spec {
    ...
}

#[cfg(target_pointer_width = "32")]
spec_int_ranges_r!(u8 u16 u32 usize);
spec_int_ranges_r!(RangeIter; u8 u16 usize);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is u32 missing here?

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2026
@rustbot

rustbot commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 25, 2026
…r}>>

Signed-off-by: 林晨 (Leo Cheng) <leo-cheng@vip.qq.com>
@Lfan-ke
Lfan-ke force-pushed the fix/step-by-range-iter-specialization branch from c555bd0 to dcc9c46 Compare July 25, 2026 16:57
@Lfan-ke

Lfan-ke commented Jul 25, 2026

Copy link
Copy Markdown
Author

Grouped the invocations into a per-width mod step_by_spec as suggested.

On the missing u32: the backward (_r) specialization requires ExactSizeIterator, and RangeIter implements it more narrowly than the legacy Range. range_exact_iter_impl! in core::range::iter only covers usize u8 u16 isize i8 i16, so RangeIter<u32> is not ExactSizeIterator even on 64-bit, whereas Range<u32> is. That's why u32 appears in spec_int_ranges_r!(Range; ...) but not in spec_int_ranges_r!(RangeIter; ...). Forward specialization has no such requirement, so both cover u32/u64. I moved this explanation into the comment above the block.

Built and ran library/coretests step_by tests locally; all pass.

@Darksonn Darksonn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rust-bors

rust-bors Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

📌 Commit dcc9c46 has been approved by Darksonn

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

Reason for tree closure: spurious failures

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 25, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 25, 2026
…cialization, r=Darksonn

iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>

`StepBy<Range<{integer}>>` already has a fast path that converts the range
into a yield-count before iteration, avoiding per-step overflow checks and
enabling better loop structure for the optimizer. `StepBy<RangeIter<{integer}>>`
did not benefit from the same optimization even though `RangeIter<T>` is a thin
newtype wrapper around `legacy::Range<T>`.

## Changes

- `library/core/src/range/iter.rs`: make `RangeIter<A>.0` `pub(crate)` so the
  step_by adapter can access the inner legacy range's start/end fields.
- `library/core/src/iter/adapters/step_by.rs`: add `spec_int_ranges_new!` and
  `spec_int_ranges_new_r!` macros that mirror the existing `spec_int_ranges!` /
  `spec_int_ranges_r!` implementations. Forward specialization covers `u8 u16
  u32 u64 usize` (pointer-width-gated for u32/u64); backward specialization is
  limited to `u8 u16 usize` since `RangeIter<u32>` / `RangeIter<u64>` do not
  implement `ExactSizeIterator`.
- `library/coretests/tests/iter/adapters/step_by.rs`: add
  `test_step_by_new_range_iter` covering forward, backward, and interleaved
  iteration via the new-range iterator.

## Motivation

Fixes rust-lang#157754. The inner loop for `core::range::Range::from(0..n).into_iter().step_by(2)` currently generates noticeably worse assembly than the equivalent `(0..n).step_by(2)` because specialization did not cover `RangeIter`.

r? libs
rust-bors Bot pushed a commit that referenced this pull request Jul 25, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #138618 (Support using const pointers in asm `const` operand)
 - #157962 (Lower paths to functions in const args as ConstKind::Error)
 - #158404 (trait_solver: normalize next-gen region constraints)
 - #158709 (rustdoc: warn on improperly interleaved HTML/MD)
 - #159174 (Fix implicit_provenance_casts warnings on Xous)
 - #159179 (enable `unreachable_cfg_select_predicates` lint as part of `unused` lint group)
 - #159518 (iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>)
 - #159673 (bootstrap: forward -fdebug-prefix-map when using cc)
 - #159700 (Split non-local `semicolon_in_expressions_from_macros` into a separate lint)
 - #159720 (document #[global_allocator] constraints)
 - #159732 (optimization: don't look for diagnostic/canonical items without rustc_attrs enabled)
 - #159738 (implement `CovariantUnsafeCell`)
 - #159740 (reuse regular exported_non_generic_symbols logic in Miri)
 - #159780 (check `extern "custom"` function pointers)
 - #159785 (Share _Unwind_Exception definition between native and wasm)
 - #159786 (rustdoc-js: ignore editor temp files in test folder discovery)
 - #159819 (std::sync::poison: disable auto_cfg on PoisonError::new)
 - #155388 (stepping into where-clauses during normalization may be productive)
 - #155914 (when bailing on ambiguity, don't force other results to ambig)
 - #159204 (Add support to caller_location to rustc_public)
 - #159439 (Fix(lib/fs/win): Fall back on Win32 delete for `Dir::remove_file`)
 - #159676 (Update wasm-component-ld to 0.5.27)
 - #159695 (proc_macro: Fix cfg_attr inner attrs in file modules)
 - #159730 (allow accessing the contents of UnsafeCell without going through get)
 - #159809 (Avoid `#[target_features]`)
rust-bors Bot pushed a commit that referenced this pull request Jul 25, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #138618 (Support using const pointers in asm `const` operand)
 - #157962 (Lower paths to functions in const args as ConstKind::Error)
 - #158404 (trait_solver: normalize next-gen region constraints)
 - #158709 (rustdoc: warn on improperly interleaved HTML/MD)
 - #159174 (Fix implicit_provenance_casts warnings on Xous)
 - #159179 (enable `unreachable_cfg_select_predicates` lint as part of `unused` lint group)
 - #159518 (iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>)
 - #159673 (bootstrap: forward -fdebug-prefix-map when using cc)
 - #159700 (Split non-local `semicolon_in_expressions_from_macros` into a separate lint)
 - #159720 (document #[global_allocator] constraints)
 - #159732 (optimization: don't look for diagnostic/canonical items without rustc_attrs enabled)
 - #159738 (implement `CovariantUnsafeCell`)
 - #159740 (reuse regular exported_non_generic_symbols logic in Miri)
 - #159780 (check `extern "custom"` function pointers)
 - #159785 (Share _Unwind_Exception definition between native and wasm)
 - #159786 (rustdoc-js: ignore editor temp files in test folder discovery)
 - #159819 (std::sync::poison: disable auto_cfg on PoisonError::new)
 - #155388 (stepping into where-clauses during normalization may be productive)
 - #155914 (when bailing on ambiguity, don't force other results to ambig)
 - #159204 (Add support to caller_location to rustc_public)
 - #159439 (Fix(lib/fs/win): Fall back on Win32 delete for `Dir::remove_file`)
 - #159676 (Update wasm-component-ld to 0.5.27)
 - #159695 (proc_macro: Fix cfg_attr inner attrs in file modules)
 - #159730 (allow accessing the contents of UnsafeCell without going through get)
 - #159809 (Avoid `#[target_features]`)
rust-bors Bot pushed a commit that referenced this pull request Jul 25, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #138618 (Support using const pointers in asm `const` operand)
 - #157962 (Lower paths to functions in const args as ConstKind::Error)
 - #158404 (trait_solver: normalize next-gen region constraints)
 - #158709 (rustdoc: warn on improperly interleaved HTML/MD)
 - #159174 (Fix implicit_provenance_casts warnings on Xous)
 - #159179 (enable `unreachable_cfg_select_predicates` lint as part of `unused` lint group)
 - #159518 (iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>)
 - #159673 (bootstrap: forward -fdebug-prefix-map when using cc)
 - #159700 (Split non-local `semicolon_in_expressions_from_macros` into a separate lint)
 - #159720 (document #[global_allocator] constraints)
 - #159732 (optimization: don't look for diagnostic/canonical items without rustc_attrs enabled)
 - #159738 (implement `CovariantUnsafeCell`)
 - #159740 (reuse regular exported_non_generic_symbols logic in Miri)
 - #159780 (check `extern "custom"` function pointers)
 - #159785 (Share _Unwind_Exception definition between native and wasm)
 - #159786 (rustdoc-js: ignore editor temp files in test folder discovery)
 - #159819 (std::sync::poison: disable auto_cfg on PoisonError::new)
 - #155388 (stepping into where-clauses during normalization may be productive)
 - #155914 (when bailing on ambiguity, don't force other results to ambig)
 - #159204 (Add support to caller_location to rustc_public)
 - #159439 (Fix(lib/fs/win): Fall back on Win32 delete for `Dir::remove_file`)
 - #159676 (Update wasm-component-ld to 0.5.27)
 - #159695 (proc_macro: Fix cfg_attr inner attrs in file modules)
 - #159730 (allow accessing the contents of UnsafeCell without going through get)
 - #159809 (Avoid `#[target_features]`)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 25, 2026
…cialization, r=Darksonn

iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>

`StepBy<Range<{integer}>>` already has a fast path that converts the range
into a yield-count before iteration, avoiding per-step overflow checks and
enabling better loop structure for the optimizer. `StepBy<RangeIter<{integer}>>`
did not benefit from the same optimization even though `RangeIter<T>` is a thin
newtype wrapper around `legacy::Range<T>`.

## Changes

- `library/core/src/range/iter.rs`: make `RangeIter<A>.0` `pub(crate)` so the
  step_by adapter can access the inner legacy range's start/end fields.
- `library/core/src/iter/adapters/step_by.rs`: add `spec_int_ranges_new!` and
  `spec_int_ranges_new_r!` macros that mirror the existing `spec_int_ranges!` /
  `spec_int_ranges_r!` implementations. Forward specialization covers `u8 u16
  u32 u64 usize` (pointer-width-gated for u32/u64); backward specialization is
  limited to `u8 u16 usize` since `RangeIter<u32>` / `RangeIter<u64>` do not
  implement `ExactSizeIterator`.
- `library/coretests/tests/iter/adapters/step_by.rs`: add
  `test_step_by_new_range_iter` covering forward, backward, and interleaved
  iteration via the new-range iterator.

## Motivation

Fixes rust-lang#157754. The inner loop for `core::range::Range::from(0..n).into_iter().step_by(2)` currently generates noticeably worse assembly than the equivalent `(0..n).step_by(2)` because specialization did not cover `RangeIter`.

r? libs
rust-bors Bot pushed a commit that referenced this pull request Jul 25, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #159825 (codegen: handle OperandValue::Uninit in codegen_return_terminator)
 - #138618 (Support using const pointers in asm `const` operand)
 - #157962 (Lower paths to functions in const args as ConstKind::Error)
 - #158404 (trait_solver: normalize next-gen region constraints)
 - #158709 (rustdoc: warn on improperly interleaved HTML/MD)
 - #159174 (Fix implicit_provenance_casts warnings on Xous)
 - #159179 (enable `unreachable_cfg_select_predicates` lint as part of `unused` lint group)
 - #159518 (iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>)
 - #159673 (bootstrap: forward -fdebug-prefix-map when using cc)
 - #159700 (Split non-local `semicolon_in_expressions_from_macros` into a separate lint)
 - #159720 (document #[global_allocator] constraints)
 - #159732 (optimization: don't look for diagnostic/canonical items without rustc_attrs enabled)
 - #159738 (implement `CovariantUnsafeCell`)
 - #159740 (reuse regular exported_non_generic_symbols logic in Miri)
 - #159780 (check `extern "custom"` function pointers)
 - #159786 (rustdoc-js: ignore editor temp files in test folder discovery)
 - #159819 (std::sync::poison: disable auto_cfg on PoisonError::new)
 - #155388 (stepping into where-clauses during normalization may be productive)
 - #155914 (when bailing on ambiguity, don't force other results to ambig)
 - #159204 (Add support to caller_location to rustc_public)
 - #159439 (Fix(lib/fs/win): Fall back on Win32 delete for `Dir::remove_file`)
 - #159676 (Update wasm-component-ld to 0.5.27)
 - #159695 (proc_macro: Fix cfg_attr inner attrs in file modules)
 - #159730 (allow accessing the contents of UnsafeCell without going through get)
 - #159809 (Avoid `#[target_features]`)
rust-bors Bot pushed a commit that referenced this pull request Jul 25, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #159825 (codegen: handle OperandValue::Uninit in codegen_return_terminator)
 - #138618 (Support using const pointers in asm `const` operand)
 - #157962 (Lower paths to functions in const args as ConstKind::Error)
 - #158404 (trait_solver: normalize next-gen region constraints)
 - #158709 (rustdoc: warn on improperly interleaved HTML/MD)
 - #159174 (Fix implicit_provenance_casts warnings on Xous)
 - #159179 (enable `unreachable_cfg_select_predicates` lint as part of `unused` lint group)
 - #159518 (iter: extend step_by specialization to cover StepBy<RangeIter<{integer}>>)
 - #159673 (bootstrap: forward -fdebug-prefix-map when using cc)
 - #159700 (Split non-local `semicolon_in_expressions_from_macros` into a separate lint)
 - #159720 (document #[global_allocator] constraints)
 - #159732 (optimization: don't look for diagnostic/canonical items without rustc_attrs enabled)
 - #159738 (implement `CovariantUnsafeCell`)
 - #159740 (reuse regular exported_non_generic_symbols logic in Miri)
 - #159780 (check `extern "custom"` function pointers)
 - #159786 (rustdoc-js: ignore editor temp files in test folder discovery)
 - #159819 (std::sync::poison: disable auto_cfg on PoisonError::new)
 - #155388 (stepping into where-clauses during normalization may be productive)
 - #155914 (when bailing on ambiguity, don't force other results to ambig)
 - #159204 (Add support to caller_location to rustc_public)
 - #159439 (Fix(lib/fs/win): Fall back on Win32 delete for `Dir::remove_file`)
 - #159676 (Update wasm-component-ld to 0.5.27)
 - #159695 (proc_macro: Fix cfg_attr inner attrs in file modules)
 - #159730 (allow accessing the contents of UnsafeCell without going through get)
 - #159809 (Avoid `#[target_features]`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

step_by() on new ranges is less optimized than the old ranges.

4 participants