Skip to content

clean up some old const trait impl syntax #146071

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

clean up some old const trait impl syntax #146071

wants to merge 1 commit into from
+59 −110

Conversation

npmccallum
Copy link
Contributor

Related to: #143874

@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
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-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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Sep 1, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2025

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

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.

Sign in to view
@zachs18
Copy link
Contributor

zachs18 commented Sep 1, 2025 •
edited
Loading

I think these two lines need to be flipped in rustfmt, so it formats the visibility of a trait before the constness.

rust/src/tools/rustfmt/src/items.rs

Lines 1178 to 1179 in 828e45a

format_constness(constness),
format_visibility(context, &item.vis),

Also, I think the tidy checks use bootstrap~~/beta~~ rustfmt, not in-tree rustfmt, so fixing rustfmt will probably need to happen first, then the stdlib changes will probably be blocked on the rustfmt fix reaching beta.

@bjorn3
Copy link
Member

bjorn3 commented Sep 1, 2025

We use a pinned nightly for rustfmt, so landing a rustfmt change and updating rustfmt the next day should work.

@npmccallum
Copy link
Contributor Author

It seems that this:

rust/src/tools/rustfmt/src/items.rs

Lines 1176 to 1182 in 84a1747

let header = format!(
"{}{}{}{}trait ",
format_constness(constness),
format_visibility(context, &item.vis),
format_safety(safety),
format_auto(is_auto),
);

Conflicts with:

https://github.com/rust-lang/rustfmt/blob/0332da01486508710f2a542111e40513bfb215aa/src/items.rs#L1169-L1174

@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2025

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@rustbot rustbot added the T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. label Sep 1, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@npmccallum npmccallum mentioned this pull request Sep 1, 2025
Merged
fmease added a commit to fmease/rust that referenced this pull request Sep 1, 2025
@fmease
Rollup merge of rust-lang#146089 - npmccallum:rustfmt, r=fmease
fe36b6d 
fix a constness ordering bug in rustfmt

Normally, changes to rustfmt go into the separate repo. But, in this case, the bug is introduced in a local change and therefore isn't present in the rustfmt repo.

Related to: rust-lang#146071
Fixes rust-lang/rustfmt#6619.
jhpratt added a commit to jhpratt/rust that referenced this pull request Sep 2, 2025
@jhpratt
Rollup merge of rust-lang#146089 - npmccallum:rustfmt, r=fmease
50e38b5 
fix a constness ordering bug in rustfmt

Normally, changes to rustfmt go into the separate repo. But, in this case, the bug is introduced in a local change and therefore isn't present in the rustfmt repo.

Related to: rust-lang#146071
Fixes rust-lang/rustfmt#6619.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 2, 2025
@GuillaumeGomez
Rollup merge of rust-lang#146089 - npmccallum:rustfmt, r=fmease
f77a31f 
fix a constness ordering bug in rustfmt

Normally, changes to rustfmt go into the separate repo. But, in this case, the bug is introduced in a local change and therefore isn't present in the rustfmt repo.

Related to: rust-lang#146071
Fixes rust-lang/rustfmt#6619.
rust-timer added a commit that referenced this pull request Sep 2, 2025
@rust-timer
Unrolled build for #146089
b66ddc8 
Rollup merge of #146089 - npmccallum:rustfmt, r=fmease

fix a constness ordering bug in rustfmt

Normally, changes to rustfmt go into the separate repo. But, in this case, the bug is introduced in a local change and therefore isn't present in the rustfmt repo.

Related to: #146071
Fixes rust-lang/rustfmt#6619.
@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Collaborator

bors commented Sep 3, 2025

☔ The latest upstream changes (presumably #146160) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fmease fmease added PG-const-traits Project group: Const traits and removed T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Sep 6, 2025
@fmease
Copy link
Member

fmease commented Sep 6, 2025

Rebased to pick up RUST-146250, force-pushed away the r-a changes because these should be filed separately in the r-a repo.

While this is mostly a libs change, I'll approve this with my PG-const-traits hat on.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 6, 2025

📌 Commit 7e98eb1 has been approved by fmease

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 Sep 6, 2025
@fmease
Copy link
Member

fmease commented Sep 6, 2025

@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 Sep 6, 2025
@fmease
Copy link
Member

fmease commented Sep 6, 2025 •
edited
Loading

Diff

Also migrated Borrow, BorrowMut and FunnelShift while we're already at it. I couldn't migrate BytewiseEq and SliceIndex due to #146122.

@fmease
Copy link
Member

fmease commented Sep 6, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 6, 2025

📌 Commit 09550d5 has been approved by fmease

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 Sep 6, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fmease
Copy link
Member

fmease commented Sep 6, 2025

@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 Sep 6, 2025
@fmease
Copy link
Member

fmease commented Sep 6, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 6, 2025

📌 Commit 00fcb14 has been approved by fmease

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 Sep 6, 2025
bors added a commit that referenced this pull request Sep 6, 2025
@bors
Auto merge of #146071 - npmccallum:cleanup, r=fmease
397f933 
clean up some old const trait impl syntax

Related to: #143874
@bors
Copy link
Collaborator

bors commented Sep 6, 2025

⌛ Testing commit 00fcb14 with merge 397f933...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmease fmease fmease approved these changes

Assignees

@fmease fmease

Labels
PG-const-traits Project group: Const traits 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@npmccallum @rustbot @rust-log-analyzer @zachs18 @bjorn3 @bors @fmease @ibraheemdev