Skip to content

Make slice comparisons const #143925

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jul 14, 2025

This needed a fix for derive_const, too, as it wasn't usable in libcore anymore as trait impls need const stability attributes. I think we can't use the same system as normal trait impls while const_trait_impl is still unstable.

r? @fee1-dead

cc #143800

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

rustbot commented Jul 14, 2025

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

@rust-log-analyzer

This comment has been minimized.

@oli-obk oli-obk force-pushed the slice-const-partialeq branch from 494d350 to c3b13bb Compare July 14, 2025 12:45

let mut attrs = thin_vec![cx.attr_word(sym::automatically_derived, self.span),];

if self.is_const && self.is_staged_api_crate {
Copy link
Member

Choose a reason for hiding this comment

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

There's clearly something interesting going on here, please add a comment explaining what.

Copy link
Member

@fee1-dead fee1-dead Jul 15, 2025

Choose a reason for hiding this comment

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

I'm fine with this being a temporary solution. To me, we should change #[derive_const(PartialEq)] to something like #[derive(PartialEq(const))] (syntax to-be-bikeshedded), then we can have #[derive(PartialEq(const, unstable))] in all standard library crates.

Copy link
Member

@RalfJung RalfJung Jul 15, 2025

Choose a reason for hiding this comment

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

How does it know which feature gate to use...?
We're fairly close to actually enforcing feature stability on impls (#140399) so this question matters soon, IIUC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't. All derived const trait impls are under the derive_const feature gate

@@ -115,11 +119,13 @@ where
// Implemented as explicit indexing rather
// than zipped iterators for performance reasons.
// See PR https://github.com/rust-lang/rust/pull/116846
for idx in 0..self.len() {
let mut idx = 0;
while idx < self.len() {
Copy link
Member

Choose a reason for hiding this comment

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

This should get a FIXME(const-hack).

@oli-obk oli-obk force-pushed the slice-const-partialeq branch from c3b13bb to 19c2f1b Compare July 14, 2025 15:28

let mut attrs = thin_vec![cx.attr_word(sym::automatically_derived, self.span),];

if self.is_const && self.is_staged_api_crate {
Copy link
Member

@fee1-dead fee1-dead Jul 15, 2025

Choose a reason for hiding this comment

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

I'm fine with this being a temporary solution. To me, we should change #[derive_const(PartialEq)] to something like #[derive(PartialEq(const))] (syntax to-be-bikeshedded), then we can have #[derive(PartialEq(const, unstable))] in all standard library crates.

@fee1-dead
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 15, 2025

📌 Commit 19c2f1b has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 15, 2025
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.

6 participants