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::fn_addr_eq #129322

Open
2 of 4 tasks
Urgau opened this issue Aug 20, 2024 · 7 comments
Open
2 of 4 tasks

Tracking Issue for ptr::fn_addr_eq #129322

Urgau opened this issue Aug 20, 2024 · 7 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC 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. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy. 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

@Urgau
Copy link
Member

Urgau commented Aug 20, 2024

Feature gate: #![feature(ptr_fn_addr_eq)]

This is a tracking issue for std::ptr::fn_addr_eq

Compares the addresses of the two function pointers for equality.

Public API

// core::ptr

/// Compares the *addresses* of the two function pointers for equality.
///
/// Function pointers comparisons can have surprising results since
/// they are never guaranteed to be unique and could vary between different
/// code generation units. Furthermore, different functions could have the
/// same address after being merged together.
///
/// This is the same as `f == g` but using this function makes clear
/// that you are aware of these potentially surprising semantics.
pub fn fn_addr_eq<T: FnPtr, U: FnPtr>(f: T, g: U) -> bool {
    f.addr() == g.addr()
}

Steps / History

Unresolved Questions

  • None yet.

Footnotes

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

@Urgau Urgau added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Aug 20, 2024
@Urgau
Copy link
Member Author

Urgau commented Oct 17, 2024

T-lang as started an FCP for adding a lint against function pointer comparisons and would like to use this method ptr::fn_addr_eq as recommendation to suppress the lint and hang documentation about caveats.

Stabilization Report

Implementation History

API Summary

// core::ptr

/// Compares the *addresses* of the two function pointers for equality.
///
/// Function pointers comparisons can have surprising results since
/// they are never guaranteed to be unique and could vary between different
/// code generation units. Furthermore, different functions could have the
/// same address after being merged together.
///
/// This is the same as `f == g` but using this function makes clear
/// that you are aware of these potentially surprising semantics.
pub fn fn_addr_eq<T: FnPtr, U: FnPtr>(f: T, g: U) -> bool {
    f.addr() == g.addr()
}

The std::marker::FnPtr bound

The FnPtr marker trait is not stable and would be not stabilized by this FCP, but this commits us to having such trait (even in this unstable form).


cc @rust-lang/lang (regarding the FnPtr bound)
cc @rust-lang/libs-api

@dtolnay dtolnay added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Oct 17, 2024
@dtolnay
Copy link
Member

dtolnay commented Oct 17, 2024

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Oct 17, 2024

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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
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 Oct 17, 2024
@traviscross
Copy link
Contributor

@rfcbot reviewed

@traviscross traviscross added I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy. labels Oct 17, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 25, 2024
Expand `ptr::fn_addr_eq()` documentation.

* Describe more clearly what is (not) guaranteed, and de-emphasize the description of rustc implementation details.
* Explain what you *can* reliably use it for.

Tracking issue for `ptr_fn_addr_eq`: rust-lang#129322

The motivation for this PR is that I just learned that `ptr::fn_addr_eq()` exists, read the documentation, and thought: “*I* know what this means, but someone not already familiar with how `rustc` works could be left wondering whether this is even good for anything.” Fixing that seems especially important if we’re going to recommend people use it instead of `==` (as per rust-lang#118833).
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 25, 2024
Rollup merge of rust-lang#131457 - kpreid:fnaddr, r=dtolnay

Expand `ptr::fn_addr_eq()` documentation.

* Describe more clearly what is (not) guaranteed, and de-emphasize the description of rustc implementation details.
* Explain what you *can* reliably use it for.

Tracking issue for `ptr_fn_addr_eq`: rust-lang#129322

The motivation for this PR is that I just learned that `ptr::fn_addr_eq()` exists, read the documentation, and thought: “*I* know what this means, but someone not already familiar with how `rustc` works could be left wondering whether this is even good for anything.” Fixing that seems especially important if we’re going to recommend people use it instead of `==` (as per rust-lang#118833).
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@traviscross
Copy link
Contributor

@rustbot labels -I-lang-nominated

We discussed this in triage today. We've now unanimously checked off on the lang side. We appreciate @rust-lang/libs-api for moving this forward in support of the pending lang work.

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Oct 30, 2024
@Urgau Urgau added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Nov 12, 2024
@traviscross traviscross removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Nov 19, 2024
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Nov 19, 2024
@rfcbot
Copy link

rfcbot commented Nov 19, 2024

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

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC 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. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy. 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

No branches or pull requests

6 participants