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 feature(ptr_to_from_bits) #91126

Open
1 of 3 tasks
scottmcm opened this issue Nov 22, 2021 · 7 comments
Open
1 of 3 tasks

Tracking Issue for feature(ptr_to_from_bits) #91126

scottmcm opened this issue Nov 22, 2021 · 7 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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 Nov 22, 2021

Feature gate: #![feature(ptr_to_from_bits)]

This is a tracking issue for <*const T>::from_bits, <*mut T>::from_bits, <*const T>::to_bits, and <*mut T>::to_bits.

These, like the stable <*const T>::cast and <*mut T>::cast, is proposed as a method for one of the many specific things that as can do to help increase readability of which of those things is happening, as well as to be able to give better compilation errors (or lints) for suspicious code patterns that aren't necessarily wrong. (For example, p as u32 is legal but suspicious, as it's possible it was meant to be *p as u32. By having the method, p.to_bits() as u32 is unambiguous.)

See the conversation in https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Adding.20methods.20as.20more.20specific.20versions.20of.20.60as.60/near/238391074

Public API

impl<T> *const T {
    pub fn to_bits(self) -> usize where T: Sized;
    pub fn from_bits(bits: usize) -> Self where T: Sized;
}
impl<T> *mut T {
    pub fn to_bits(self) -> usize where T: Sized;
    pub fn from_bits(bits: usize) -> Self where T: Sized;
}

Steps / History

Unresolved Questions

  • None yet.
@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 Nov 22, 2021
@leonardo-m
Copy link

(For example, p as u32 is legal but suspicious, as it's possible it was meant to be *p as u32. By having the method, p.to_bits() as u32 is unambiguous.)

I'm not a fan of "as" casts in general :-]

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 11, 2021
Add `<*{const|mut} T>::{to|from}_bits`

Named based on the floating-point methods of the same name, as those are also about returning the *representation* of the value.

Tracking issue: rust-lang#91126

Based on the conversation in https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Adding.20methods.20as.20more.20specific.20versions.20of.20.60as.60/near/238391074

r? `@joshtriplett`
@workingjubilee
Copy link
Contributor

workingjubilee commented Mar 3, 2022

I do not believe these methods are the correct ones. In particular, I believe that stabilizing these methods would force Rust further down an incorrect evolutionary path regarding its story of pointers in general.

@jendrikw
Copy link
Contributor

Could we consider adding this to fn pointers?

@fbstj
Copy link
Contributor

fbstj commented Mar 26, 2022

is this related to #95241 ?

@Gankra
Copy link
Contributor

Gankra commented Mar 26, 2022

ptr_to_from_bits is inherently contradictory to the goals of the strict_provenance (#95228) experiment, yes. In my initial WIP PR I actually marked them as deprecated just to see what was using them.

The problems are that they:

  • Imply ptr-int-ptr roundtrips
  • Specify that ptr==usize (intptr_t)

Strict provenance is experimenting with whether we can specify

  • ptr-int-ptr doesn't actually work
  • ptr>=usize (size_t)

Both of these are very desirable for more properly targetting CHERI. The APIs don't need to be deprecated but stabilization should maybe be held off on while we're experimenting in this space.

@scottmcm
Copy link
Member Author

scottmcm commented Apr 2, 2022

@fbstj No, this came from a much less interesting place of wanting to be able to better lint typos like isize::max as usize, which very much doesn't do what you want 🙃

With https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.addr now available, I think this feature should just be deprecated. To be nice, though, we shouldn't remove this for another 5 weeks or so, to give unstable users an easier transition.

EDIT: I opened #95583 to do that.

Manishearth added a commit to Manishearth/rust that referenced this issue Nov 22, 2022
…, r=dtolnay

Deprecate the unstable `ptr_to_from_bits` feature

I propose that we deprecate the (unstable!) `to_bits` and `from_bits` methods on raw pointers.  (With the intent to ~~remove them once `addr` has been around long enough to make the transition easy on people -- maybe another 6 weeks~~ remove them fairly soon after, as the strict and expose versions have been around for a while already.)

The APIs that came from the strict provenance explorations (rust-lang#95228) are a more holistic version of these, and things like `.expose_addr()` work for the "that cast looks sketchy" case even if the full strict provenance stuff never happens.  (As a bonus, `addr` is even shorter than `to_bits`, though it is only applicable if people can use full strict provenance! `addr` is *not* a direct replacement for `to_bits`.)  So I think it's fine to move away from the `{to|from}_bits` methods, and encourage the others instead.

That also resolves the worry that was brought up (I forget where) that `q.to_bits()` and `(*q).to_bits()` both work if `q` is a pointer-to-floating-point, as they also have a `to_bits` method.

Tracking issue rust-lang#91126
Code search: https://github.com/search?l=Rust&p=1&q=ptr_to_from_bits&type=Code

For potential pushback, some users in case they want to chime in
- `@RSSchermer` https://github.com/RSSchermer/ARWA/blob/365bb68541447453fc44f6fbcc5d394bb94c14e9/arwa/src/html/custom_element.rs#L105
- `@strax` https://github.com/strax/pbr/blob/99616d1dbf42f93ec8dd668d05b3180649558180/openexr/src/core/alloc.rs#L36
- `@MiSawa` https://github.com/MiSawa/pomelo/blob/577c6223588d539295a71ff125d8f249e59f4146/crates/kernel/src/timer.rs#L50
@jmillikin
Copy link
Contributor

The 1 year anniversary of deprecating the ptr_to_from_bits functions is approaching -- would it be OK to tidy up and delete them now?

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. 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

7 participants