Skip to content

Update boxed::Box docs on memory layout #60963

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

Merged
merged 3 commits into from
May 22, 2019
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 45 additions & 14 deletions src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,24 +127,38 @@ impl<T: ?Sized> Box<T> {
///
/// After calling this function, the raw pointer is owned by the
/// resulting `Box`. Specifically, the `Box` destructor will call
/// the destructor of `T` and free the allocated memory. Since the
/// way `Box` allocates and releases memory is unspecified, the
/// only valid pointer to pass to this function is the one taken
/// from another `Box` via the [`Box::into_raw`] function.
/// the destructor of `T` and free the allocated memory. For this
/// to be safe, the memory must have been allocated in the precise
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this perhaps avoid duplicating the module docs? Maybe a section could be created in the module docs specifying the global allocator business, and this coul djust point there indicating how to interact with it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great! I've just pushed a commit up to do this, creating a section called "Memory Layout" in the module docs. I also moved this section down to the bottom since it seemed better to have the simple examples first, before the more detailed information on memory layout.

/// way that `Box` expects, namely, using the global allocator
/// with the correct [`Layout`] for holding a value of type `T`. In
/// particular, this will be satisfied for a pointer obtained
/// from a previously existing `Box` using [`Box::into_raw`].
///
/// # Safety
///
/// This function is unsafe because improper use may lead to
/// memory problems. For example, a double-free may occur if the
/// function is called twice on the same raw pointer.
///
/// [`Box::into_raw`]: struct.Box.html#method.into_raw
///
/// # Examples
///
/// Recreate a `Box` which was previously converted to a raw pointer using [`Box::into_raw`]:
/// ```
/// let x = Box::new(5);
/// let ptr = Box::into_raw(x);
/// let x = unsafe { Box::from_raw(ptr) };
/// ```
/// Manually create a `Box` from scratch by using the global allocator:
/// ```
/// use std::alloc::{Layout, alloc};
///
/// let ptr = unsafe{ alloc(Layout::new::<i32>()) } as *mut i32;
/// unsafe{ *ptr = 5; }
/// let x = unsafe{ Box::from_raw(ptr) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of an unsafe on each line, could this just be wrapped in one block? Additionally could "rustfmt be run over the code examples" here? I think that there's typically a space after the unsafe word, for example.

Copy link
Member

@shepmaster shepmaster May 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rustfmt be run over the code examples

This can actually be done with an unstable rustfmt option. You'll want to make sure to only add the formatting changes to the code you were already changing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! And thanks for the tip on the new rustfmt feature; it worked!

/// ```
///
/// [`Layout`]: ../alloc/struct.Layout.html
/// [`Box::into_raw`]: struct.Box.html#method.into_raw
///
#[stable(feature = "box_raw", since = "1.4.0")]
#[inline]
pub unsafe fn from_raw(raw: *mut T) -> Self {
Expand All @@ -158,21 +172,34 @@ impl<T: ?Sized> Box<T> {
/// After calling this function, the caller is responsible for the
/// memory previously managed by the `Box`. In particular, the
/// caller should properly destroy `T` and release the memory. The
/// proper way to do so is to convert the raw pointer back into a
/// `Box` with the [`Box::from_raw`] function.
/// easiest way to do so is to convert the raw pointer back into a `Box`
/// with the [`Box::from_raw`] function.
///
/// Note: this is an associated function, which means that you have
/// to call it as `Box::into_raw(b)` instead of `b.into_raw()`. This
/// is so that there is no conflict with a method on the inner type.
///
/// [`Box::from_raw`]: struct.Box.html#method.from_raw
///
/// # Examples
///
/// Converting the raw pointer back into a `Box` with [`Box::from_raw`]
/// for automatic cleanup:
/// ```
/// let x = Box::new(5);
/// let x = Box::new(String::from("Hello"));
/// let ptr = Box::into_raw(x);
/// let x = unsafe{ Box::from_raw(ptr) };
/// ```
/// Manual cleanup by running the destructor and deallocating the memory:
/// ```
/// use std::alloc::{Layout, dealloc};
/// use std::ptr;
///
/// let x = Box::new(String::from("Hello"));
/// let p = Box::into_raw(x);
/// unsafe{ ptr::drop_in_place(p); }
/// unsafe{ dealloc(p as *mut u8, Layout::new::<String>()); }
/// ```
///
/// [`Box::from_raw`]: struct.Box.html#method.from_raw
///
#[stable(feature = "box_raw", since = "1.4.0")]
#[inline]
pub fn into_raw(b: Box<T>) -> *mut T {
Expand All @@ -184,7 +211,7 @@ impl<T: ?Sized> Box<T> {
/// After calling this function, the caller is responsible for the
/// memory previously managed by the `Box`. In particular, the
/// caller should properly destroy `T` and release the memory. The
/// proper way to do so is to convert the `NonNull<T>` pointer
/// easiest way to do so is to convert the `NonNull<T>` pointer
/// into a raw pointer and back into a `Box` with the [`Box::from_raw`]
/// function.
///
Expand All @@ -203,6 +230,10 @@ impl<T: ?Sized> Box<T> {
/// fn main() {
/// let x = Box::new(5);
/// let ptr = Box::into_raw_non_null(x);
///
/// // Clean up the memory by converting the NonNull pointer back
/// // into a Box and letting the Box be dropped.
/// let x = unsafe{ Box::from_raw(ptr.as_ptr()) };
/// }
/// ```
#[unstable(feature = "box_into_raw_non_null", issue = "47336")]
Expand Down