Skip to content
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

Tracking Issue for ptr::addr_eq #116324

Closed
3 of 4 tasks
scottmcm opened this issue Oct 2, 2023 · 7 comments · Fixed by #117968
Closed
3 of 4 tasks

Tracking Issue for ptr::addr_eq #116324

scottmcm opened this issue Oct 2, 2023 · 7 comments · Fixed by #117968
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Oct 2, 2023

Feature gate: #![feature(ptr_addr_eq)]

This is a tracking issue for ...

Public API

// core::ptr

pub fn addr_eq<T: ?Sized, U: ?Sized>(p: *const T, q: *const U) -> bool;

Steps / History

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@scottmcm scottmcm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Oct 2, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 2, 2023
@asquared31415
Copy link
Contributor

While ptr.addr() (or maybe expose_addr) is gated under strict_provenance, I think that it should be noted here because it would allow for writing this code to compare the addresses. However, since addr is a method on pointer types, it cannot be used with &(mut) without a cast and some parentheses first, which is less convenient.

I think that in the quest to replace as with better methods, a function with some semantics that involve getting the address will be stabilized and for the purposes of implementing this method it doesn't matter what other semantics w.r.t. provenance might exist.

@asquared31415
Copy link
Contributor

To clarify, today you can write

#![feature(strict_provenance)]
fn addr_eq<T: ?Sized, U: ?Sized>(p: *const T, q: *const U) -> bool {
    p.addr() == q.addr()
}

(or just inline that code directly, if you can cast to raw pointers first instead of relying on coercion)

@scottmcm
Copy link
Member Author

scottmcm commented Oct 2, 2023

Note that I implemented this via casting to *const (), not via .addr() (nor .expose_addr()). I wanted to avoid any ptr2int in the codegen, since even with .addr() in LLVM we still end up exposing the address.

You can copy exactly what the PR does if you want; it works on stable.

@joshtriplett
Copy link
Member

Lang would like to recommend addr_eq in a lint (#117758), so I'd like to propose stabilizing this function.

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Nov 15, 2023

Team member @joshtriplett 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. 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 Nov 15, 2023
@rfcbot
Copy link

rfcbot commented Nov 15, 2023

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

@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 Nov 25, 2023
@rfcbot
Copy link

rfcbot commented Nov 25, 2023

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.

@dtolnay dtolnay linked a pull request Nov 25, 2023 that will close this issue
compiler-errors added a commit to compiler-errors/rust that referenced this issue Nov 25, 2023
…lnay

Stabilize `ptr::addr_eq`

This PR stabilize the `ptr_addr_eq` library feature, representing:

```rust
// core::ptr

pub fn addr_eq<T: ?Sized, U: ?Sized>(p: *const T, q: *const U) -> bool;
```

FCP has already started [on the tracking issue](rust-lang#116324 (comment)) and is waiting on the final period comment.

Note: stabilizing this feature is somewhat of requirement for a new T-lang lint, cf. rust-lang#117758 (comment).
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 26, 2023
Rollup merge of rust-lang#117968 - Urgau:stabilize-ptr-addr-eq, r=dtolnay

Stabilize `ptr::addr_eq`

This PR stabilize the `ptr_addr_eq` library feature, representing:

```rust
// core::ptr

pub fn addr_eq<T: ?Sized, U: ?Sized>(p: *const T, q: *const U) -> bool;
```

FCP has already started [on the tracking issue](rust-lang#116324 (comment)) and is waiting on the final period comment.

Note: stabilizing this feature is somewhat of requirement for a new T-lang lint, cf. rust-lang#117758 (comment).
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Dec 14, 2023
@traviscross traviscross added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants