Skip to content

Add Box::into_inner. #80438

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 10, 2021
Merged

Add Box::into_inner. #80438

merged 1 commit into from
Feb 10, 2021

Conversation

crlf0710
Copy link
Member

@crlf0710 crlf0710 commented Dec 28, 2020

This adds a Box::into_inner method to the Box type. I actually suggest deprecating the compiler magic of *b if this gets stablized in the future.

r? @m-ou-se

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 28, 2020
@KamilaBorowska
Copy link
Contributor

Personally I would rather prefer to have DerefMove to allow other types to be special like Box, but that's just my opinion.

@fmease
Copy link
Member

fmease commented Dec 28, 2020

Should the documentation mention deref'ing the box as an alternative? This might prevent confusion.

@camelid camelid added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 29, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Dec 30, 2020

I actually suggest deprecating the compiler magic of *b if this gets stablized in the future.

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 m-ou-se 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 30, 2020
@crlf0710
Copy link
Member Author

@m-ou-se Yes, there is still a use case (which i think is good).
if i have a type S which is Copy and i have a Box<S> value, and i want to take the S value out.

#[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 Copy. With this function i can easily do what i want. A moving-out-of-deref trait won't be able to fix this.

(Deprecating the compiler magic of *b)
That's a rather large change; one that should be discussed with the lang team first.

Indeed! But i think whether or not that change will happen, this convenience function serves its purpose well. And there's already Pin::into_inner(and many others) in the standard library so this is not an inconsistent addition. And i'm only purposing adding it as an unstable method.

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 31, 2020
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2021
@JohnCSimon
Copy link
Member

merge conflict
@JohnCSimon label: +S-waiting-on-author -S-waiting-on-author

@crlf0710
Copy link
Member Author

crlf0710 commented Feb 9, 2021

Rebased and squashed.

///
/// let c = Box::new(5);
///
/// assert_eq!(Box::into_inner(c), 5);
Copy link
Contributor

@pickfire pickfire Feb 9, 2021

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?

Copy link
Member

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.

@m-ou-se
Copy link
Member

m-ou-se commented Feb 9, 2021

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 9, 2021

📌 Commit ce7de07 has been approved by m-ou-se

@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 Feb 9, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 10, 2021
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
@bors bors merged commit a28f2af into rust-lang:master Feb 10, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 10, 2021
@crlf0710 crlf0710 deleted the box_into_inner branch February 12, 2021 02:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.