implement Box::take by edwloef · Pull Request #147227 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation6 Commits1 Checks10 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Tracking issue: #147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
rustbot added S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
r? @joboet
rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
joboet left a comment • Loading
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current wording is great! I'd maybe add a note (and a line in the example) about using Box::write to reinitialise the Box, as these functions are meant to be used together.
View changes since this review
| let (raw, alloc) = Box::into_raw_with_allocator(boxed); |
|---|
| let value = raw.read(); |
| let uninit = Box::from_raw_in(raw.cast::<mem::MaybeUninit<T>>(), alloc); |
| (value, uninit) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use Box::into_non_null_with_allocator and NonNull::cast_uninit?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! However since this has already been r+'ed, I think I'll save that change for the inevitable renaming of the function, since as far as I can tell the semantics are identical from the outside.
📌 Commit 8dfea22 has been approved by joboet
It is now in the queue for this repository.
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
Zalathar added a commit to Zalathar/rust that referenced this pull request
implement Box::take
Tracking issue: rust-lang#147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
bors added a commit that referenced this pull request
Rollup of 10 pull requests
Successful merges:
- #146281 (Support
#[rustc_align_static]insidethread_local!) - #146535 (mbe: Implement
unsafeattribute rules) - #146585 (indexing: reword help)
- #147004 (Tweak handling of "struct like start" where a struct isn't supported)
- #147221 (Forbid
//@ compile-flags: -Cincremental=in tests) - #147225 (Don't enable shared memory by default with Wasm atomics)
- #147227 (implement
Box::take) - #147231 (Extending
#[rustc_force_inline]to be applicable to inherent methods) - #147233 (Initialize llvm submodule if not already the case to run citool)
- #147236 (Update books)
r? @ghost
@rustbot modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
implement Box::take
Tracking issue: rust-lang#147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
jhpratt added a commit to jhpratt/rust that referenced this pull request
implement Box::take
Tracking issue: rust-lang#147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
bors added a commit that referenced this pull request
Rollup of 9 pull requests
Successful merges:
- #146281 (Support
#[rustc_align_static]insidethread_local!) - #146535 (mbe: Implement
unsafeattribute rules) - #146585 (indexing: reword help)
- #147004 (Tweak handling of "struct like start" where a struct isn't supported)
- #147221 (Forbid
//@ compile-flags: -Cincremental=in tests) - #147225 (Don't enable shared memory by default with Wasm atomics)
- #147227 (implement
Box::take) - #147233 (Initialize llvm submodule if not already the case to run citool)
- #147236 (Update books)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit that referenced this pull request
Rollup merge of #147227 - edwloef:box_take, r=joboet
implement Box::take
Tracking issue: #147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
implement Box::take
Tracking issue: rust-lang#147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
rust-cloud-vms bot pushed a commit to makai410/rustc_public that referenced this pull request
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request
makai410 pushed a commit to makai410/rust that referenced this pull request
implement Box::take
Tracking issue: rust-lang#147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
makai410 pushed a commit to makai410/rust that referenced this pull request
makai410 pushed a commit to makai410/rust that referenced this pull request
implement Box::take
Tracking issue: rust-lang#147212
I'm not entirely sure about the wording of the doc comment, if anyone has any suggestions that'd be great :)
makai410 pushed a commit to makai410/rust that referenced this pull request
Zalathar added a commit to Zalathar/rust that referenced this pull request
Zalathar added a commit to Zalathar/rust that referenced this pull request
Zalathar added a commit to Zalathar/rust that referenced this pull request
Zalathar added a commit to Zalathar/rust that referenced this pull request
rust-timer added a commit that referenced this pull request
makai410 pushed a commit to makai410/rustc_public that referenced this pull request
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the library team, which will review and decide on the PR/issue.