-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
box: into_raw, from_raw functions #21318
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
Agree this is a common pattern. r? @alexcrichton are there any alternate approaches to solving this problem? If accepted this should have tests for boxed DSTs as well - I'm not actually sure whether they will work or not here. |
I am very in favour of this. transmutes are at best awful for semantics. |
I suddenly realized, that
int How it should be dealt with? |
@stepancheg This is a tricky case because Box must be a lang item because it currently relies on compiler magic. I'd suggest moving everything but the
This will cause the lang item to be present at the desired path under normal builds and test builds. |
We need to be pretty cautious adding inherent methods to Could the documentation of these methods also be expanded? It would be useful to explain why the function is unsafe as well. |
Updated PR with:
|
/// | ||
/// Function is unsafe, because improper use of this function may | ||
/// lead to memory problems like double-free, for example if the | ||
/// function is called twice on the same raw pointer. |
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.
It may be worth mentioning that the only valid pointers to pass to this function are those created from boxed::into_raw
. I don't think we'd want to bake in any assumptions about how Box
is allocated and letting others think that it can be used to free any old malloc
memory.
cc @aturon, do you have thoughts about these APIs? Am I perhaps too paranoid about inherent methods on smart pointers? |
Functions are needed for safety and convenience. It is a common pattern to use `mem::transmute` to convert between `Box` and raw pointer, like this: ``` let b = Box::new(3); let p = mem::transmute(b); // pass `p` to some C library ``` After this commit, conversion can be written as: ``` let p = boxed::into_raw(b); ``` `into_raw` and `from_raw` functions are still unsafe, but they are much safer than `mem::transmute`, because *raw functions do not convert between incompatible pointers. For example, this likely incorrect code can be successfully compiled: ``` let p: *mut u64 = ... let b: Box<u32> = mem::transmute(p); ``` Using `from_raw` results in compile-time error: ``` let p: *mut u64 = ... let b: Box<u32> = Box::from_raw(p); // compile-time error ``` `into_raw` and `from_raw` functions are similar to C++ `std::unique_ptr` `release` function [1] and constructor from pointer [2]. [1] https://en.cppreference.com/w/cpp/memory/unique_ptr/release [2] https://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr
Updated PR with more docs and rebased to master. |
I'm happy to move forward with this as an |
@bors: r+ dadd97c |
Thanks! |
/// lead to memory problems like double-free, for example if the | ||
/// function is called twice on the same raw pointer. | ||
#[unstable(feature = "alloc", | ||
reason = "may be renamed of moved out of Box scope")] |
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.
s/of/or ?
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.
@sinistersnare fixed, thanks.
Updated with typo fixed. @alexcrichton please, re-r+. |
Functions are needed for safety and convenience. It is a common pattern to use `mem::transmute` to convert between `Box` and raw pointer, like this: ``` let b = Box::new(3); let p = mem::transmute(b); // pass `p` to some C library ``` After this commit, conversion can be written as: ``` let p = b.into_raw(); ``` `into_raw` and `from_raw` functions are still unsafe, but they are much safer than `mem::transmute`, because *raw functions do not convert between incompatible pointers. For example, this likely incorrect code can be successfully compiled: ``` let p: *mut u64 = ... let b: Box<u32> = mem::transmute(p); ``` Using `from_raw` results in compile-time error: ``` let p: *mut u64 = ... let b: Box<u32> = Box::from_raw(p); // compile-time error ``` `into_raw` and `from_raw` functions are similar to C++ `std::unique_ptr` `release` function [1] and constructor from pointer [2]. [1] https://en.cppreference.com/w/cpp/memory/unique_ptr/release [2] https://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr
Why
|
Functions are needed for safety and convenience.
It is a common pattern to use
mem::transmute
to convert betweenBox
and raw pointer, like this:After this commit, conversion can be written as:
into_raw
andfrom_raw
functions are still unsafe, but they aremuch safer than
mem::transmute
, because *raw functions do notconvert between incompatible pointers. For example, this likely
incorrect code can be successfully compiled:
Using
from_raw
results in compile-time error:into_raw
andfrom_raw
functions are similar to C++std::unique_ptr
release
function [1] and constructor from pointer [2].[1] https://en.cppreference.com/w/cpp/memory/unique_ptr/release
[2] https://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr