Skip to content

Make Sub, Mul, Div and Rem const_traits #143000

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 1 commit into from
Jun 26, 2025
Merged

Conversation

SciMind2460
Copy link
Contributor

@SciMind2460 SciMind2460 commented Jun 25, 2025

#90080

Generally useful for implementation, like Add.

@rustbot
Copy link
Collaborator

rustbot commented Jun 25, 2025

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jun 25, 2025
@rustbot

This comment has been minimized.

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 25, 2025
@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. has-merge-commits PR has merge commits, merge with caution. labels Jun 25, 2025
@jhpratt
Copy link
Member

jhpratt commented Jun 25, 2025

cc @rust-lang/project-const-traits — I am unsure of the current status of const traits and whether this is wanted at this point in time

@jieyouxu
Copy link
Member

Not sure either, but maybe #133999? cc @compiler-errors as you may know

@SciMind2460
Copy link
Contributor Author

As far as I know @oli-obk had made a similar PR a few days ago that made PartialEq const and it was merged, so I think this might be helpful in the same vein.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@fee1-dead
Copy link
Member

I'd say our general approach here is still "don't make things const unless necessary". The PartialEq constification was done because it unblocked other work.

We're in the moment of transitioning to a new syntax and I'd want us to avoid introducing extra work/churn when a transition to new syntax comes eventually

@SciMind2460
Copy link
Contributor Author

@fee1-dead should we proceed with unconstifying Add until we transition to the new syntax?

@fee1-dead
Copy link
Member

No, see #133237

@oli-obk
Copy link
Contributor

oli-obk commented Jun 25, 2025

I personally am fine with changing our policy and @compiler-errors and I talked about it a week or two ago. Since we don't have bootstrap issues anymore, we can just rip out const traits that are a problem during any changes of the implementation, so adding more isn't much of an issue

@jhpratt
Copy link
Member

jhpratt commented Jun 25, 2025

Works for me.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 25, 2025

📌 Commit 8c8a85a has been approved by jhpratt

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 Jun 25, 2025
SciMind2460 added a commit to SciMind2460/rust that referenced this pull request Jun 25, 2025
Make `Sub`, `Mul`, `Div` and `Rem`  `const_traits`

Generally useful for implementation, like Add.
@SciMind2460
Copy link
Contributor Author

@rustbot label +F-const_trait_impl

@rustbot rustbot added the F-const_trait_impl `#![feature(const_trait_impl)]` label Jun 25, 2025
@fmease
Copy link
Member

fmease commented Jun 25, 2025

could you please squash into one commit? thanks
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 25, 2025
@SciMind2460
Copy link
Contributor Author

I think it works now?
@bors r=jhpratt rollup

@bors
Copy link
Collaborator

bors commented Jun 25, 2025

@SciMind2460: 🔑 Insufficient privileges: Not in reviewers

@bors
Copy link
Collaborator

bors commented Jun 25, 2025

@SciMind2460: 🔑 Insufficient privileges: not in try users

@fmease
Copy link
Member

fmease commented Jun 25, 2025

@bors r=jhpratt

@bors
Copy link
Collaborator

bors commented Jun 25, 2025

📌 Commit 09295af has been approved by jhpratt

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 25, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 25, 2025
Make `Sub`, `Mul`, `Div` and `Rem`  `const_traits`

Generally useful for implementation, like Add.
@fee1-dead
Copy link
Member

Since we don't have bootstrap issues anymore, we can just rip out const traits that are a problem during any changes of the implementation, so adding more isn't much of an issue

Ah, then no concerns from me

@SciMind2460
Copy link
Contributor Author

I may work on constifying other ops traits in the future, like BitXor.

bors added a commit that referenced this pull request Jun 25, 2025
Rollup of 7 pull requests

Successful merges:

 - #142146 (Withdraw the claim `extern "C-cmse-nonsecure-*"` always matches `extern "C"`)
 - #142200 (`tests/ui`: A New Order [8/N])
 - #142724 (Add runtime check to avoid overwrite arg in `Diag`)
 - #142809 (Add PrintTAFn flag for targeted type analysis printing)
 - #142976 (Check CoerceUnsized impl validity before coercing)
 - #142992 (Convert some ABI tests to use `extern "rust-invalid"`)
 - #143000 (Make `Sub`, `Mul`, `Div` and `Rem`  `const_traits`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c001128 into rust-lang:master Jun 26, 2025
10 checks passed
rust-timer added a commit that referenced this pull request Jun 26, 2025
Rollup merge of #143000 - SciMind2460:master, r=jhpratt

Make `Sub`, `Mul`, `Div` and `Rem`  `const_traits`

Generally useful for implementation, like Add.
@rustbot rustbot added this to the 1.90.0 milestone Jun 26, 2025
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jun 26, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#142146 (Withdraw the claim `extern "C-cmse-nonsecure-*"` always matches `extern "C"`)
 - rust-lang/rust#142200 (`tests/ui`: A New Order [8/N])
 - rust-lang/rust#142724 (Add runtime check to avoid overwrite arg in `Diag`)
 - rust-lang/rust#142809 (Add PrintTAFn flag for targeted type analysis printing)
 - rust-lang/rust#142976 (Check CoerceUnsized impl validity before coercing)
 - rust-lang/rust#142992 (Convert some ABI tests to use `extern "rust-invalid"`)
 - rust-lang/rust#143000 (Make `Sub`, `Mul`, `Div` and `Rem`  `const_traits`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-const_trait_impl `#![feature(const_trait_impl)]` 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.

9 participants