Skip to content

Fix missing const for inherent pointer replace methods #136877

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
Apr 5, 2025

Conversation

Sky9x
Copy link
Contributor

@Sky9x Sky9x commented Feb 11, 2025

ptr::replace (the free fn) is already const stable. However, there are inherent convenience methods on *mut T and NonNull<T>, allowing you to write eg. unsafe { foo.replace(bar) } where foo is *mut T or NonNull<T>.

It seems const was never added to the inherent method (likely oversight), so this PR adds it.
I don't believe this needs another1 FCP as the inherent methods are already stable and ptr::replace is already const stable, so this adds no new API.

Original tracking issue: #83164
ptr::replace constified in #83091
ptr::replace const stabilized in #130954

Footnotes

  1. const_replace FCP completed: https://github.com/rust-lang/rust/issues/83164#issuecomment-2385670050

@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 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 Feb 11, 2025
@Sky9x
Copy link
Contributor Author

Sky9x commented Feb 11, 2025

@rustbot label +T-libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 11, 2025
@jhpratt
Copy link
Member

jhpratt commented Feb 11, 2025

I agree that this seems like an oversight and doesn't need another FCP, but I'll defer to @rust-lang/libs-api as it is ultimately their decision. r=me if they're fine with it.

@dtolnay dtolnay removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Feb 11, 2025
@dtolnay
Copy link
Member

dtolnay commented Feb 11, 2025

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 11, 2025

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 11, 2025
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 26, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Mar 26, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@riking
Copy link

riking commented Apr 4, 2025

Historical note: This was likely blocked on &mut in const support, which was resolved recently, then missed during the stabilization pass because it's *mut instead of &mut.

@RalfJung
Copy link
Member

RalfJung commented Apr 4, 2025

Cc @rust-lang/wg-const-eval (as noted before, please always ping us when proposing a const-stabilization FCP)

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 5, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 5, 2025

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@jhpratt
Copy link
Member

jhpratt commented Apr 5, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 5, 2025

📌 Commit 7bca1f2 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 Apr 5, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2025
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#136877 (Fix missing const for inherent pointer `replace` methods)
 - rust-lang#138797 (Fix `ProvenVia` for global where clauses)
 - rust-lang#139121 (Rename internal module from `statik` to `no_threads`)
 - rust-lang#139319 (StableMIR: Prepare for refactoring)
 - rust-lang#139404 (Small smir cleanup)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0b34287 into rust-lang:master Apr 5, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 5, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2025
Rollup merge of rust-lang#136877 - Sky9x:const-inherent-ptr-replace, r=jhpratt

Fix missing const for inherent pointer `replace` methods

`ptr::replace` (the free fn) is already const stable. However, there are inherent convenience methods on `*mut T` and `NonNull<T>`, allowing you to write eg. `unsafe { foo.replace(bar) }` where `foo` is `*mut T` or `NonNull<T>`.

It seems const was never added to the inherent method (likely oversight), so this PR adds it.
I don't believe this needs another[^1] FCP as the inherent methods are already stable and `ptr::replace` is already const stable, so this adds no new API.

Original tracking issue: rust-lang#83164
`ptr::replace` constified in rust-lang#83091
`ptr::replace` const stabilized in rust-lang#130954

[^1]: `const_replace` FCP completed: rust-lang#83164 (comment)
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 10, 2025
…r=jhpratt

Fix missing const for inherent pointer `replace` methods

`ptr::replace` (the free fn) is already const stable. However, there are inherent convenience methods on `*mut T` and `NonNull<T>`, allowing you to write eg. `unsafe { foo.replace(bar) }` where `foo` is `*mut T` or `NonNull<T>`.

It seems const was never added to the inherent method (likely oversight), so this PR adds it.
I don't believe this needs another[^1] FCP as the inherent methods are already stable and `ptr::replace` is already const stable, so this adds no new API.

Original tracking issue: rust-lang#83164
`ptr::replace` constified in rust-lang#83091
`ptr::replace` const stabilized in rust-lang#130954

[^1]: `const_replace` FCP completed: rust-lang#83164 (comment)
@Sky9x Sky9x deleted the const-inherent-ptr-replace branch April 13, 2025 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants