-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add Box::into_inner
.
#80438
Add Box::into_inner
.
#80438
Conversation
Personally I would rather prefer to have |
158ddc3
to
6876373
Compare
Should the documentation mention deref'ing the box as an alternative? This might prevent confusion. |
That's a rather large change; one that should be discussed with the lang team first. Has there been any discussion on this? How about alternatives like a moving-out-of-deref trait as suggested above? Without that language change, is there a good use case for this new function? |
@m-ou-se Yes, there is still a use case (which i think is good). #[derive(Copy, Clone, Debug)]
struct S;
let boxed_val = Box::new(S);
let val = *boxed_val;
// this copies the value out (with Deref), but the memory is not deallocated.
println!("{:?}", boxed_val); //it's still alive now, needs another `drop` to deallocate.
// do something more. This seems a footgun because the behavior of that line depends on whether S is
Indeed! But i think whether or not that change will happen, this convenience function serves its purpose well. And there's already |
merge conflict |
2747587
to
c4dc26d
Compare
c4dc26d
to
ce7de07
Compare
Rebased and squashed. |
/// | ||
/// let c = Box::new(5); | ||
/// | ||
/// assert_eq!(Box::into_inner(c), 5); |
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.
Isn't this the same as *c
? Why is the deref bad and we want to remove it?
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 was already discussed. See the conversation above.
@bors r+ rollup |
📌 Commit ce7de07 has been approved by |
Rollup of 11 pull requests Successful merges: - rust-lang#79849 (Clarify docs regarding sleep of zero duration) - rust-lang#80438 (Add `Box::into_inner`.) - rust-lang#81466 (Add suggest mut method for loop) - rust-lang#81687 (Make Vec::split_at_spare_mut public) - rust-lang#81904 (Bump stabilization version for const int methods) - rust-lang#81909 ([compiler/rustc_typeck/src/check/expr.rs] Remove unnecessary refs in pattern matching) - rust-lang#81910 (Use format string in bootstrap panic instead of a string directly) - rust-lang#81913 (Rename HIR UnOp variants) - rust-lang#81925 (Add long explanation for E0547) - rust-lang#81926 (add suggestion to use the `async_recursion` crate) - rust-lang#81951 (Update cargo) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This adds a
Box::into_inner
method to theBox
type.I actually suggest deprecating the compiler magic of*b
if this gets stablized in the future.r? @m-ou-se