Skip to content

Document the current aliasing rules for Box<T>. #97229

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 31, 2022
Merged

Document the current aliasing rules for Box<T>. #97229

merged 2 commits into from
May 31, 2022

Conversation

Noratrieb
Copy link
Member

Currently, Box<T> gets noalias, meaning it has the same rules as &mut T. This is sparsely documented, even though it can have quite a big impact on unsafe code using box. Therefore, these rules are documented here, with a big warning that they are not normative and subject to change, since we have not yet committed to an aliasing model and the state of Box<T> is especially uncertain.

If you have any suggestions and improvements, make sure to leave them here. This is mostly intended to inform people about what is currently going on (to prevent misunderstandings such as Jon Gjengset's Box aliasing).

This is supposed to only document current UB and not add any new guarantees or rules.

@rust-highfive
Copy link
Contributor

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label May 20, 2022
@rust-highfive
Copy link
Contributor

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2022
@Noratrieb
Copy link
Member Author

r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 20, 2022
@rust-highfive rust-highfive assigned dtolnay and unassigned kennytm May 20, 2022
@rustbot rustbot removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label May 20, 2022
dtolnay
dtolnay requested changes May 22, 2022
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it needs to be more clear that nothing in this alarming-looking section is relevant to a causal reader who isn't writing their own unsafe code (which is 99%+ of people learning about Box from its rustdoc).

Perhaps a section like # Considerations for unsafe code would be more appropriate, saying "raw pointer" instead of "pointer", and being explicit like "For more guidance on working with Box from unsafe code, see ucg#326".

@Noratrieb
Document the current aliasing rules for Box<T>.
e7c468d 
Currently, `Box<T>` gets `noalias`, meaning it has
the same rules as `&mut T`. This is
sparsely documented, even though it can have quite
a big impact on unsafe code using box. Therefore,
these rules are documented here, with a big warning
that they are not normative and subject to change,
since we have not yet committed to an aliasing model
and the state of `Box<T>` is especially uncertain.
@Noratrieb
Copy link
Member Author

That makes sense, this could definitely be confusing for people not using unsafe code. I changed the header as you suggested. Maybe it makes sense to have a h2 that is still called Aliasing, or move all the layout docs below the Considerations for unsafe code too?

dtolnay
dtolnay requested changes May 30, 2022
View reviewed changes
dtolnay
dtolnay approved these changes May 30, 2022
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :) thanks!

@dtolnay
Copy link
Member

dtolnay commented May 30, 2022

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 30, 2022

📌 Commit ffd7f58 has been approved by dtolnay

@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 May 30, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request May 31, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 31, 2022
@bors
Auto merge of rust-lang#97574 - Dylan-DPC:rollup-jq850l6, r=Dylan-DPC
d35d972 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#97089 (Improve settings theme display)
 - rust-lang#97229 (Document the current aliasing rules for `Box<T>`.)
 - rust-lang#97371 (Suggest adding a semicolon to a closure without block)
 - rust-lang#97455 (Stabilize `toowned_clone_into`)
 - rust-lang#97565 (Add doc alias `memset` to `write_bytes`)
 - rust-lang#97569 (Remove `memset` alias from `fill_with`.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9c72f16 into rust-lang:master May 31, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 31, 2022
@Noratrieb Noratrieb deleted the doc-box-noalias branch May 31, 2022 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
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.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Noratrieb @rust-highfive @dtolnay @bors @kennytm @rustbot