-
Notifications
You must be signed in to change notification settings - Fork 491
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
Document raw pointer <-> usize casts. #970
Conversation
I was advised that these casts are controversial (and to cc @RalfJung), but I think promising at minimum something like this is necessary to describe how people use raw pointers right now irl. |
I'm afraid "same value" is impossible since pointers carry provenance that is inadvertently lost on an integer roundtrip. See this blog post for the kind of issues that arise when one assumes that a ptr-int-ptr roundtrip returns the same value. So it might be a different value, but it will point to the same location ("numeric address", I guess). |
C++ seems to use similar phrasing: https://eel.is/c++draft/expr.reinterpret.cast#5 Is the description of the |
Yeah, and C/C++ are notoriously bad at documenting how pointers behave, which leads to the kinds of problems I have described in my blog post. I view C and C++ as bad examples in this regard, not as something we should copy. I think Rust can and should do better here. (C is in the process of improving; I hope C++ will follow eventually.)
I could agree to saying something like
(I removed "of the same type" since it is not relevant in Rust.) I'd prefer to explicitly talk about provenance, and I think a basic understanding of provenance is required to properly work with int-ptr-casts, but I don't have time to draft all that text right now. It might be worth to at least explicitly state that "two pointers point to the same location" does not imply that the two pointers have the same value, i.e., to explicitly say that there's more to a pointer than the location it points to. I am also not sure if "location" is the best term and am open to suggestions for other terms to use here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't actually discuss provenance anywhere in the reference currently. It'd be part of the memory model, but uhh, we've avoided that area. I'm not actually sure we want to talk about it here. cc @RalfJung against for what they think.
I'm mostly giving stylistic nits here, and you should wait on Ralf before applying them since he might give better non-stylstic review.
Thanks for the stylistic pointers! I also don't want to put a discussion of the memory model into the "what does a cast do" section, but I definitely don't want to trick readers into thinking it's easier than it is. Since a lot of it isn't entirely hammered out as far as I understand, I just wanted to get the basics written down. |
I think this needs a decision from @rust-lang/lang.^^ I cannot make the call here, all I can say is whether a proposed spec is consistent with what I know about how to mathematically formalize ptr-int-casts and pointer provenance, and I can help flesh out the formulation. |
Nominating as per the previous comment. |
We discussed this in our 2021-04-06 meeting. The consensus in that meeting was that we did not want to merge this PR, because we didn't feel ready to make commitments one way or the other on this question. It seems like making statements about provenance would be best done when we've worked out more of the stack borrows story etc. We would however be interested in having a design meeting to talk about the overall plan forward regarding provenance, correct pointer usage, etc. |
(We'd be inclined to close this PR, as a result) |
This PR wasn't meant to be about provenance. Currently there seems to be no official documentation or specification about what pointer<->usize casts do at all. Provenance wording happened because "you obviously just get the same pointer back if you go back and forth" was too ambitious, but of course I would be happy to write down something much weaker. Is there any behavior, wording aside, that we'd be happy to commit to right now, or would you prefer to leave pointer<->usize casts completely undefined until the provenance/stacked borrows thing is fully hashed out? |
Hmm, I think it would be useful to document them! I imagine something that leaves a lot of open-ended would work. Something like this?
|
Great, I'll amend this PR! I think it may be good to include some basically "blessed" examples of roundtrips that don't go anywhere near the tricky cases, but the table/list-based structure of this section makes it a bit awkward. hm. |
Now it's not one big list.
ok so this also changes the formatting a bunch, breaking up the big nested list into sections with headings, so that I could fit in an example. The actual wording should not be overreaching, I hope. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking much better to me, modulo @RalfJung's nits.
Okay, "actually sound". Hopefully.
My comments have been resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving per nikomatsakis and RalfJung.
I pushed a small change to change a link to markdown.
Thanks!
Update books ## reference 4 commits in 5aa457bf1b54bd2cd5d4cf49797f29299bdf89a7..9c68af3ce6ccca2395e1868addef26a0542e9ddd 2021-05-05 08:39:22 -0700 to 2021-05-24 09:53:32 -0700 - missing parameter name in Trait Implementations (rust-lang/reference#1030) - Add more content to impl-trait.md (rust-lang/reference#1017) - Document extended key-value attributes (rust-lang/reference#1029) - Document raw pointer <-> usize casts. (rust-lang/reference#970) ## rust-by-example 1 commits in 5f8c6da200ada77760a2fe1096938ef58151c9a6..805e016c5792ad2adabb66e348233067d5ea9f10 2021-04-29 08:08:01 -0300 to 2021-05-20 17:08:34 -0300 - Update structs.md (rust-lang/rust-by-example#1440) ## rustc-dev-guide 4 commits in 1e6c7fbda4c45e85adf63ff3f82fa9c870b1447f..50de7f0682adc5d95ce858fe6318d19b4b951553 2021-05-10 13:38:24 +0900 to 2021-05-20 15:02:20 +0200 - update rustfmt references to reflect change from submod to subtree (rust-lang/rustc-dev-guide#1129) - Remove `--stage 1` argument from `doc` invocations (rust-lang/rustc-dev-guide#1125) - Update coverage docs (rust-lang/rustc-dev-guide#1122) - Document -Zunpretty=thir-tree (rust-lang/rustc-dev-guide#1128) ## edition-guide 1 commits in 1da3c411f17adb1ba5de1683bb6acee83362b54a..302a115e8f71876dfc884aebb0ca5ccb02b8a962 2021-02-16 16:46:40 -0800 to 2021-05-21 10:46:11 -0400 - Minimize the edition guide (rust-lang/edition-guide#232) ## embedded-book 1 commits in 569c3391f5c0cc43433bc77831d17f8ff4d76602..7349d173fa28a0bb834cf0264a05286620ef0923 2021-04-07 08:32:11 +0000 to 2021-05-25 13:59:05 +0000 - Remove $ from cargo-binutils (rust-embedded/book#292)
Update books ## reference 4 commits in 5aa457bf1b54bd2cd5d4cf49797f29299bdf89a7..9c68af3ce6ccca2395e1868addef26a0542e9ddd 2021-05-05 08:39:22 -0700 to 2021-05-24 09:53:32 -0700 - missing parameter name in Trait Implementations (rust-lang/reference#1030) - Add more content to impl-trait.md (rust-lang/reference#1017) - Document extended key-value attributes (rust-lang/reference#1029) - Document raw pointer <-> usize casts. (rust-lang/reference#970) ## rust-by-example 1 commits in 5f8c6da200ada77760a2fe1096938ef58151c9a6..805e016c5792ad2adabb66e348233067d5ea9f10 2021-04-29 08:08:01 -0300 to 2021-05-20 17:08:34 -0300 - Update structs.md (rust-lang/rust-by-example#1440) ## rustc-dev-guide 4 commits in 1e6c7fbda4c45e85adf63ff3f82fa9c870b1447f..50de7f0682adc5d95ce858fe6318d19b4b951553 2021-05-10 13:38:24 +0900 to 2021-05-20 15:02:20 +0200 - update rustfmt references to reflect change from submod to subtree (rust-lang/rustc-dev-guide#1129) - Remove `--stage 1` argument from `doc` invocations (rust-lang/rustc-dev-guide#1125) - Update coverage docs (rust-lang/rustc-dev-guide#1122) - Document -Zunpretty=thir-tree (rust-lang/rustc-dev-guide#1128) ## edition-guide 1 commits in 1da3c411f17adb1ba5de1683bb6acee83362b54a..302a115e8f71876dfc884aebb0ca5ccb02b8a962 2021-02-16 16:46:40 -0800 to 2021-05-21 10:46:11 -0400 - Minimize the edition guide (rust-lang/edition-guide#232) ## embedded-book 1 commits in 569c3391f5c0cc43433bc77831d17f8ff4d76602..7349d173fa28a0bb834cf0264a05286620ef0923 2021-04-07 08:32:11 +0000 to 2021-05-25 13:59:05 +0000 - Remove $ from cargo-binutils (rust-embedded/book#292)
No description provided.