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.

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

add raw ptr variant of UnsafeCell::get #66248

Merged
merged 5 commits into from
Nov 13, 2019
Merged

add raw ptr variant of UnsafeCell::get #66248

merged 5 commits into from
Nov 13, 2019

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 9, 2019

This has come up recently in #66051 (Cc @Centril @pitdicker) as well as in discussion with @nikomatsakis and in unrelated discussion with @withoutboats.

@rust-highfive
Copy link
Collaborator

r? @sfackler

(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 Nov 9, 2019
@RalfJung
Copy link
Member Author

RalfJung commented Nov 9, 2019

Also Cc @SimonSapin because of the relation to MaybeUninit -- this is needed for gradual initialization of an UnsafeCell.

@SimonSapin
Copy link
Contributor

As the code comment notes, this can be a simple pointer case because of #[repr(transparent)]. Which means that this can be implemented outside of the standard library, so perhaps having it inside is less pressing?

So I suppose the question is: is it easier for users to find a method than to figure out that a pointer cast is the way to go? Even if we have documentation (perhaps on MaybeUninit) that recommends a cast?

@RalfJung
Copy link
Member Author

RalfJung commented Nov 9, 2019

Which means that this can be implemented outside of the standard library, so perhaps having it inside is less pressing?

get can also be implemented outside the standard library with the same argument. But right now we say that get is the only way to actually correctly get a mutable pointer to the inside; using a cast is forbidden.

IOW, this is a case of libstd using its special status.

@SimonSapin
Copy link
Contributor

Good points. r=me with:

  • A tracking issue
  • Maybe mention in the doc-comment that this differs from get in accepting a raw pointer? It’s visible in the signature, but it doesn’t hurt repeating.

@the8472
Copy link
Member

the8472 commented Nov 9, 2019 •
edited
Loading

  • Maybe mention in the doc-comment that this differs from get in accepting a raw pointer? It’s visible in the signature, but it doesn’t hurt repeating.

Also why one may want to use it over get (the dereferencable/valid data rules)

Centril
Centril reviewed Nov 9, 2019
View reviewed changes
src/libcore/cell.rs Outdated Show resolved Hide resolved
src/libcore/cell.rs Outdated Show resolved Hide resolved
@RalfJung @Centril
Trailing full stop
aba385a 
Co-Authored-By: Mazdak Farrokhzad <[email protected]>
@RalfJung RalfJung mentioned this pull request Nov 13, 2019
Closed
1 task
@RalfJung
Copy link
Member Author

@SimonSapin @the8472 I expanded the docs, please check.

@SimonSapin SimonSapin added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2019
@SimonSapin
Copy link
Contributor

Docs look good. r=me, pending the method receiver type discussion above.

@RalfJung
Copy link
Member Author

@bors r=SimonSapin

@bors
Copy link
Contributor

bors commented Nov 13, 2019

📌 Commit 861698a has been approved by SimonSapin

@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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 13, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 13, 2019
@JohnTitor
Rollup merge of rust-lang#66248 - RalfJung:unsafe_cell_raw_get, r=Sim…
0310708 
…onSapin

add raw ptr variant of UnsafeCell::get

This has come up recently in rust-lang#66051 (Cc @Centril @pitdicker) as well as in discussion with @nikomatsakis and in unrelated discussion with @withoutboats.
@JohnTitor JohnTitor mentioned this pull request Nov 13, 2019
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 13, 2019
@JohnTitor
Rollup merge of rust-lang#66248 - RalfJung:unsafe_cell_raw_get, r=Sim…
689cc04 
…onSapin

add raw ptr variant of UnsafeCell::get

This has come up recently in rust-lang#66051 (Cc @Centril @pitdicker) as well as in discussion with @nikomatsakis and in unrelated discussion with @withoutboats.
@JohnTitor JohnTitor mentioned this pull request Nov 13, 2019
Merged
bors added a commit that referenced this pull request Nov 13, 2019
@bors
Auto merge of #66366 - JohnTitor:rollup-xlc1bj2, r=JohnTitor
695fe96 
Rollup of 14 pull requests

Successful merges:

 - #65932 (download .tar.xz if python3 is used)
 - #66094 (Fix documentation for `Iterator::count()`.)
 - #66166 (rename cfg(rustdoc) into cfg(doc))
 - #66186 (Add long error explanation for E0623)
 - #66227 (docs: Fix link to BufWriter::flush)
 - #66248 (add raw ptr variant of UnsafeCell::get)
 - #66292 (add Result::map_or)
 - #66297 (Add a callback that allows compiler consumers to override queries.)
 - #66317 (Use a relative bindir for rustdoc to find rustc)
 - #66330 (Improve non-exhaustiveness handling in usefulness checking)
 - #66331 (Add some tests for fixed ICEs)
 - #66334 (Move Session fields to CrateStore)
 - #66335 (Move self-profile infrastructure to data structures)
 - #66337 (Remove dead code for encoding/decoding lint IDs)

Failed merges:

r? @ghost
@bors bors merged commit 861698a into rust-lang:master Nov 13, 2019
@bors
Copy link
Contributor

bors commented Nov 13, 2019

☔ The latest upstream changes (presumably #66366) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 13, 2019
@RalfJung RalfJung deleted the unsafe_cell_raw_get branch November 14, 2019 08:15
@RalfJung RalfJung mentioned this pull request Dec 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielhenrymantilla danielhenrymantilla danielhenrymantilla left review comments

@Centril Centril Centril left review comments

Assignees

@sfackler sfackler

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@RalfJung @rust-highfive @SimonSapin @the8472 @bors @Centril @danielhenrymantilla @sfackler