Skip to content
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

Clarify Box<T> representation and its use in FFI #62514

Merged
merged 8 commits into from
Dec 12, 2019
44 changes: 44 additions & 0 deletions src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,50 @@
//! T` obtained from `Box::<T>::into_raw` may be deallocated using the
//! [`Global`] allocator with `Layout::for_value(&*value)`.
//!
//! So long as `T: Sized`, a `Box<T>` is guaranteed to be represented as a
//! single pointer and is also ABI-compatible with C pointers (i.e. the C type
//! `T*`). This means that you have Rust code which passes ownership of a
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//! `T*`). This means that you have Rust code which passes ownership of a
//! `T*`). This means that you can have Rust code which passes ownership of a

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. Thank you!

//! `Box<T>` to C code by using `Box<T>` as the type on the Rust side, and
//! `T*` as the corresponding type on the C side. As an example, consider this
Copy link
Contributor

Choose a reason for hiding this comment

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

So at this point, @gnzlbg has convinced me that there is one remaining issue that we may want to address here. In particular, it seems to me that it is only safe to use Rust types if all callers to the function obey the Rust restrictions (e.g., non-null, valid, etc). The easiest way to express that is to say that it is safe to use Rust in types in cases where the function is defined in Rust, but not in cases where the function is defined in C. I'm debating how much detail to go into here and where.

//! C header which declares functions that create and destroy some kind of
//! `Foo` value:
//!
//! ```c
//! /* C header */
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
//!
//! /* Returns ownership to the caller */
//! struct Foo* foo_new(void);
//!
//! /* Takes ownership from the caller; no-op when invoked with NULL */
//! void foo_delete(struct Foo*);
//! ```
//!
//! These two functions might be implemented in Rust as follows. Here, the
//! `struct Foo*` type from C is translated to `Box<Foo>`, which captures
//! the ownership constraints. Note also that the nullable argument to
//! `foo_delete` is represented in Rust as `Option<Box<Foo>>`, since `Box<Foo>`
//! cannot be null.
//!
//! ```
//! #[repr(C)]
//! pub struct Foo;
//!
//! #[no_mangle]
//! pub extern "C" fn foo_new() -> Box<Foo> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually a function like foo_new() requires that the returned pointer be destroyed by calling the foo_delete() function. Returning a Box is almost always the wrong thing to do here since Box doesn't enforce that foo_delete is called when the Box is dropped. So this example is documenting a pattern that is usually the wrong thing to do.

Copy link
Member

Choose a reason for hiding this comment

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

@briansmith this is a long-merged PR, so new comments here are bound to get lost... please open an issue if you think this needs to be tracked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Returning a Box is almost always the wrong thing to do here since Box doesn't enforce that foo_delete is called when the Box is dropped.

It's no different than "regular" FFI code where one returns raw pointer and requires the caller to call the corresponding _delete function with the same pointer, but returning & accepting Box better documents semantics.

If you open a new issue, please cc me, this is a pattern close to my heart :)

//! Box::new(Foo)
//! }
//!
//! #[no_mangle]
//! pub extern "C" fn foo_delete(_: Option<Box<Foo>>) {}
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
//! ```
//!
//! Even though `Box<T>` has the same representation and C ABI as a C pointer,
//! this does not mean that you can convert an arbitrary `T*` into a `Box<T>`
//! and expect things to work. `Box<T>` values will always be fully aligned,
//! non-null pointers. Moreover, the destructor for `Box<T>` will attempt to
//! free the value with the global allocator. In general, the best practice
//! is to only use `Box<T>` for pointers that originated from the global
//! allocator.
//!
//! [dereferencing]: ../../std/ops/trait.Deref.html
//! [`Box`]: struct.Box.html
Expand Down