Skip to content

constify Step trait and all of its implementations #153821

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.

Sign up for GitHub

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
rust-bors merged 1 commit into from
Mar 28, 2026
Merged

constify Step trait and all of its implementations #153821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions library/core/src/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ where

/// Implements comparison of arrays [lexicographically](Ord#lexicographical-comparison).
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: PartialOrd, const N: usize> PartialOrd for [T; N] {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl<T: [const] PartialOrd, const N: usize> const PartialOrd for [T; N] {
#[inline]
fn partial_cmp(&self, other: &[T; N]) -> Option<Ordering> {
PartialOrd::partial_cmp(&&self[..], &&other[..])
Expand All @@ -430,7 +431,8 @@ impl<T: PartialOrd, const N: usize> PartialOrd for [T; N] {

/// Implements comparison of arrays [lexicographically](Ord#lexicographical-comparison).
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: Ord, const N: usize> Ord for [T; N] {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl<T: [const] Ord, const N: usize> const Ord for [T; N] {
#[inline]
fn cmp(&self, other: &[T; N]) -> Ordering {
Ord::cmp(&&self[..], &&other[..])
Expand Down
27 changes: 18 additions & 9 deletions library/core/src/iter/range.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ unsafe_impl_trusted_step![AsciiChar char i8 i16 i32 i64 i128 isize u8 u16 u32 u6
unstable `Step` trait"
)]
#[unstable(feature = "step_trait", issue = "42168")]
pub trait Step: Clone + PartialOrd + Sized {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
pub const trait Step: [const] Clone + [const] PartialOrd + Sized {
/// Returns the bounds on the number of *successor* steps required to get from `start` to `end`
/// like [`Iterator::size_hint()`][Iterator::size_hint()].
///
Expand Down Expand Up @@ -262,7 +263,8 @@ macro_rules! step_integer_impls {
$(
#[allow(unreachable_patterns)]
#[unstable(feature = "step_trait", issue = "42168")]
impl Step for $u_narrower {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for $u_narrower {
step_identical_methods!();
step_unsigned_methods!();

Expand Down Expand Up @@ -296,7 +298,8 @@ macro_rules! step_integer_impls {

#[allow(unreachable_patterns)]
#[unstable(feature = "step_trait", issue = "42168")]
impl Step for $i_narrower {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for $i_narrower {
step_identical_methods!();
step_signed_methods!($u_narrower);

Expand Down Expand Up @@ -362,7 +365,8 @@ macro_rules! step_integer_impls {
$(
#[allow(unreachable_patterns)]
#[unstable(feature = "step_trait", issue = "42168")]
impl Step for $u_wider {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for $u_wider {
step_identical_methods!();
step_unsigned_methods!();

Expand Down Expand Up @@ -392,7 +396,8 @@ macro_rules! step_integer_impls {

#[allow(unreachable_patterns)]
#[unstable(feature = "step_trait", issue = "42168")]
impl Step for $i_wider {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for $i_wider {
step_identical_methods!();
step_signed_methods!($u_wider);

Expand Down Expand Up @@ -449,7 +454,8 @@ step_integer_impls! {
}

#[unstable(feature = "step_trait", issue = "42168")]
impl Step for char {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for char {
#[inline]
fn steps_between(&start: &char, &end: &char) -> (usize, Option<usize>) {
let start = start as u32;
Expand Down Expand Up @@ -536,7 +542,8 @@ impl Step for char {
}

#[unstable(feature = "step_trait", issue = "42168")]
impl Step for AsciiChar {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for AsciiChar {
#[inline]
fn steps_between(&start: &AsciiChar, &end: &AsciiChar) -> (usize, Option<usize>) {
Step::steps_between(&start.to_u8(), &end.to_u8())
Expand Down Expand Up @@ -578,7 +585,8 @@ impl Step for AsciiChar {
}

#[unstable(feature = "step_trait", issue = "42168")]
impl Step for Ipv4Addr {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for Ipv4Addr {
#[inline]
fn steps_between(&start: &Ipv4Addr, &end: &Ipv4Addr) -> (usize, Option<usize>) {
u32::steps_between(&start.to_bits(), &end.to_bits())
Expand Down Expand Up @@ -610,7 +618,8 @@ impl Step for Ipv4Addr {
}

#[unstable(feature = "step_trait", issue = "42168")]
impl Step for Ipv6Addr {
#[rustc_const_unstable(feature = "step_trait", issue = "42168")]
impl const Step for Ipv6Addr {
#[inline]
fn steps_between(&start: &Ipv6Addr, &end: &Ipv6Addr) -> (usize, Option<usize>) {
u128::steps_between(&start.to_bits(), &end.to_bits())
Expand Down
18 changes: 12 additions & 6 deletions library/core/src/net/ip_addr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ pub enum IpAddr {
/// assert!("0xcb.0x0.0x71.0x00".parse::<Ipv4Addr>().is_err()); // all octets are in hex
/// ```
#[rustc_diagnostic_item = "Ipv4Addr"]
#[derive(Copy, Clone, PartialEq, Eq)]
#[derive(Copy)]
#[derive_const(Clone, PartialEq, Eq)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Ipv4Addr {
octets: [u8; 4],
Expand Down Expand Up @@ -161,7 +162,8 @@ impl Hash for Ipv4Addr {
/// assert_eq!(localhost.is_loopback(), true);
/// ```
#[rustc_diagnostic_item = "Ipv6Addr"]
#[derive(Copy, Clone, PartialEq, Eq)]
#[derive(Copy)]
#[derive_const(Clone, PartialEq, Eq)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Ipv6Addr {
octets: [u8; 16],
Expand Down Expand Up @@ -1183,7 +1185,8 @@ impl PartialEq<IpAddr> for Ipv4Addr {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl PartialOrd for Ipv4Addr {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl const PartialOrd for Ipv4Addr {
#[inline]
fn partial_cmp(&self, other: &Ipv4Addr) -> Option<Ordering> {
Some(self.cmp(other))
Expand Down Expand Up @@ -1213,7 +1216,8 @@ impl PartialOrd<IpAddr> for Ipv4Addr {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Ord for Ipv4Addr {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl const Ord for Ipv4Addr {
#[inline]
fn cmp(&self, other: &Ipv4Addr) -> Ordering {
self.octets.cmp(&other.octets)
Expand Down Expand Up @@ -2177,7 +2181,8 @@ impl PartialEq<Ipv6Addr> for IpAddr {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl PartialOrd for Ipv6Addr {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl const PartialOrd for Ipv6Addr {
#[inline]
fn partial_cmp(&self, other: &Ipv6Addr) -> Option<Ordering> {
Some(self.cmp(other))
Expand Down Expand Up @@ -2207,7 +2212,8 @@ impl PartialOrd<IpAddr> for Ipv6Addr {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Ord for Ipv6Addr {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl const Ord for Ipv6Addr {
#[inline]
fn cmp(&self, other: &Ipv6Addr) -> Ordering {
self.segments().cmp(&other.segments())
Expand Down
69 changes: 50 additions & 19 deletions library/core/src/slice/cmp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
use super::{from_raw_parts, memchr};
use crate::ascii;
use crate::cmp::{self, BytewiseEq, Ordering};
use crate::convert::Infallible;
use crate::intrinsics::compare_bytes;
use crate::marker::Destruct;
use crate::mem::SizedTypeProperties;
use crate::num::NonZero;
use crate::ops::ControlFlow;
Expand Down Expand Up @@ -33,7 +35,8 @@ impl<T: [const] Eq> const Eq for [T] {}

/// Implements comparison of slices [lexicographically](Ord#lexicographical-comparison).
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: Ord> Ord for [T] {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl<T: [const] Ord> const Ord for [T] {
fn cmp(&self, other: &[T]) -> Ordering {
SliceOrd::compare(self, other)
}
Expand All @@ -52,7 +55,8 @@ const fn as_underlying(x: ControlFlow<bool>) -> u8 {

/// Implements comparison of slices [lexicographically](Ord#lexicographical-comparison).
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: PartialOrd> PartialOrd for [T] {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl<T: [const] PartialOrd> const PartialOrd for [T] {
#[inline]
fn partial_cmp(&self, other: &[T]) -> Option<Ordering> {
SlicePartialOrd::partial_compare(self, other)
Expand Down Expand Up @@ -175,19 +179,31 @@ const trait SliceChain: Sized {

type AlwaysBreak<B> = ControlFlow<B, crate::convert::Infallible>;

impl<A: PartialOrd> SlicePartialOrd for A {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl<A: [const] PartialOrd> const SlicePartialOrd for A {
default fn partial_compare(left: &[A], right: &[A]) -> Option<Ordering> {
let elem_chain = |a, b| match PartialOrd::partial_cmp(a, b) {
Some(Ordering::Equal) => ControlFlow::Continue(()),
non_eq => ControlFlow::Break(non_eq),
};
let len_chain = |a: &_, b: &_| ControlFlow::Break(usize::partial_cmp(a, b));
// FIXME(const-hack): revert this to a const closure once possible
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
const fn elem_chain<A: [const] PartialOrd>(a: &A, b: &A) -> ControlFlow<Option<Ordering>> {
match PartialOrd::partial_cmp(a, b) {
Some(Ordering::Equal) => ControlFlow::Continue(()),
non_eq => ControlFlow::Break(non_eq),
}
}

// FIXME(const-hack): revert this to a const closure once possible
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
const fn len_chain(a: &usize, b: &usize) -> ControlFlow<Option<Ordering>, Infallible> {
ControlFlow::Break(usize::partial_cmp(a, b))
}

let AlwaysBreak::Break(b) = chaining_impl(left, right, elem_chain, len_chain);
b
}
}

impl<A: PartialOrd> SliceChain for A {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl<A: [const] PartialOrd> const SliceChain for A {
default fn chaining_lt(left: &[Self], right: &[Self]) -> ControlFlow<bool> {
chaining_impl(left, right, PartialOrd::__chaining_lt, usize::__chaining_lt)
}
Expand All @@ -202,12 +218,13 @@ impl<A: PartialOrd> SliceChain for A {
}
}

#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
#[inline]
fn chaining_impl<'l, 'r, A: PartialOrd, B, C>(
const fn chaining_impl<'l, 'r, A: PartialOrd, B, C>(
left: &'l [A],
right: &'r [A],
elem_chain: impl Fn(&'l A, &'r A) -> ControlFlow<B>,
len_chain: impl for<'a> FnOnce(&'a usize, &'a usize) -> ControlFlow<B, C>,
elem_chain: impl [const] Fn(&'l A, &'r A) -> ControlFlow<B> + [const] Destruct,
len_chain: impl for<'a> [const] FnOnce(&'a usize, &'a usize) -> ControlFlow<B, C> + [const] Destruct,
) -> ControlFlow<B, C> {
let l = cmp::min(left.len(), right.len());

Expand All @@ -216,8 +233,11 @@ fn chaining_impl<'l, 'r, A: PartialOrd, B, C>(
let lhs = &left[..l];
let rhs = &right[..l];

for i in 0..l {
// FIXME(const-hack): revert this to `for i in 0..l` once `impl const Iterator for Range<T>`
Copy link
Copy Markdown
Member

@scottmcm scottmcm Mar 23, 2026

Choose a reason for hiding this comment

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

Definitely sad that this PR fixing Step that's the main thing this needs still can't just use in 0..l 😢

Copy link
Copy Markdown
Contributor Author

@Lars-Schumann Lars-Schumann Mar 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Kind of a chicken and egg problem here, this is pretty much the last thing needed for const Ranges.

let mut i: usize = 0;
while i < l {
elem_chain(&lhs[i], &rhs[i])?;
i += 1;
}

len_chain(&left.len(), &right.len())
Expand Down Expand Up @@ -270,13 +290,24 @@ const trait SliceOrd: Sized {
fn compare(left: &[Self], right: &[Self]) -> Ordering;
}

impl<A: Ord> SliceOrd for A {
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
impl<A: [const] Ord> const SliceOrd for A {
default fn compare(left: &[Self], right: &[Self]) -> Ordering {
let elem_chain = |a, b| match Ord::cmp(a, b) {
Ordering::Equal => ControlFlow::Continue(()),
non_eq => ControlFlow::Break(non_eq),
};
let len_chain = |a: &_, b: &_| ControlFlow::Break(usize::cmp(a, b));
// FIXME(const-hack): revert this to a const closure once possible
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
const fn elem_chain<A: [const] Ord>(a: &A, b: &A) -> ControlFlow<Ordering> {
match Ord::cmp(a, b) {
Ordering::Equal => ControlFlow::Continue(()),
non_eq => ControlFlow::Break(non_eq),
}
}

// FIXME(const-hack): revert this to a const closure once possible
#[rustc_const_unstable(feature = "const_cmp", issue = "143800")]
const fn len_chain(a: &usize, b: &usize) -> ControlFlow<Ordering, Infallible> {
ControlFlow::Break(usize::cmp(a, b))
}

let AlwaysBreak::Break(b) = chaining_impl(left, right, elem_chain, len_chain);
b
}
Expand Down
21 changes: 10 additions & 11 deletions tests/codegen-llvm/array-cmp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,6 @@ pub fn array_of_tuple_le(a: &[(i16, u16); 2], b: &[(i16, u16); 2]) -> bool {
// CHECK: %[[EQ00:.+]] = icmp eq i16 %[[A00]], %[[B00]]
// CHECK-NEXT: br i1 %[[EQ00]], label %[[L01:.+]], label %[[EXIT_S:.+]]

// CHECK: [[L01]]:
// CHECK: %[[PA01:.+]] = getelementptr{{.+}}i8, ptr %a, {{i32|i64}} 2
// CHECK: %[[PB01:.+]] = getelementptr{{.+}}i8, ptr %b, {{i32|i64}} 2
// CHECK: %[[A01:.+]] = load i16, ptr %[[PA01]]
// CHECK: %[[B01:.+]] = load i16, ptr %[[PB01]]
// CHECK-NOT: cmp
// CHECK: %[[EQ01:.+]] = icmp eq i16 %[[A01]], %[[B01]]
// CHECK-NEXT: br i1 %[[EQ01]], label %[[L10:.+]], label %[[EXIT_U:.+]]

// CHECK: [[L10]]:
// CHECK: %[[PA10:.+]] = getelementptr{{.+}}i8, ptr %a, {{i32|i64}} 4
// CHECK: %[[PB10:.+]] = getelementptr{{.+}}i8, ptr %b, {{i32|i64}} 4
// CHECK: %[[A10:.+]] = load i16, ptr %[[PA10]]
Expand All @@ -64,7 +54,16 @@ pub fn array_of_tuple_le(a: &[(i16, u16); 2], b: &[(i16, u16); 2]) -> bool {
// CHECK: %[[B11:.+]] = load i16, ptr %[[PB11]]
// CHECK-NOT: cmp
// CHECK: %[[EQ11:.+]] = icmp eq i16 %[[A11]], %[[B11]]
// CHECK-NEXT: br i1 %[[EQ11]], label %[[DONE:.+]], label %[[EXIT_U]]
// CHECK-NEXT: br i1 %[[EQ11]], label %[[DONE:.+]], label %[[EXIT_U:.+]]

// CHECK: [[L01]]:
// CHECK: %[[PA01:.+]] = getelementptr{{.+}}i8, ptr %a, {{i32|i64}} 2
// CHECK: %[[PB01:.+]] = getelementptr{{.+}}i8, ptr %b, {{i32|i64}} 2
// CHECK: %[[A01:.+]] = load i16, ptr %[[PA01]]
// CHECK: %[[B01:.+]] = load i16, ptr %[[PB01]]
// CHECK-NOT: cmp
// CHECK: %[[EQ01:.+]] = icmp eq i16 %[[A01]], %[[B01]]
// CHECK-NEXT: br i1 %[[EQ01]], label %{{.+}}, label %[[EXIT_U]]
Copy link
Copy Markdown
Member

@Mark-Simulacrum Mark-Simulacrum Mar 22, 2026

Choose a reason for hiding this comment

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

Do we know what happened to cause this? Maybe one of the const-hack closure -> function conversions above changed inlining? If I'm reading this right, the new IR seems worse -- 0/2/4/6 vs. 0/4/6/2 seems like it might be at least a tiny regression.

Copy link
Copy Markdown
Member

@Mark-Simulacrum Mark-Simulacrum Mar 22, 2026

Choose a reason for hiding this comment

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

cc @scottmcm in case you have thoughts on the new IR -- I think it meets the goal of when we added it still...

Copy link
Copy Markdown
Contributor Author

@Lars-Schumann Lars-Schumann Mar 22, 2026

Choose a reason for hiding this comment

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

my guess was this swapped basically by chance and since the rest is the same it should be fine.

Copy link
Copy Markdown
Member

@scottmcm scottmcm Mar 23, 2026

Choose a reason for hiding this comment

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

It's definitely weird that it swapped things like this in such a way that the only branch to the first block is from below -- or at least I'm assuming that that's why it ended up without a header? Should probably try to fix that so the block name substitutions are still meaningful?

But in general, I'm not worried about the block order of the things, since that's really LLVM's business not ours, and is highly divorced from what we can do in code anyway.

That said, I'm definitely sad about all these const hacks, and kinda wish they'd stop until the features existed -- if it needs const closures it should use const closures, for example. But that's not really my call :/

Copy link
Copy Markdown
Contributor Author

@Lars-Schumann Lars-Schumann Mar 23, 2026

Choose a reason for hiding this comment

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

Const closures were reimplemented very recently and to my knowledge aren't supposed to be used in the std lib yet, these are rather tame const hacks.


// CHECK: [[DONE]]:
// CHECK: %[[RET:.+]] = phi i1 [ %{{.+}}, %[[EXIT_S]] ], [ %{{.+}}, %[[EXIT_U]] ], [ true, %[[L11]] ]
Expand Down
Loading