From 318c5d6c963d0e5d8ece89e804b4e696c6011955 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 8 Jul 2019 21:34:36 -0700 Subject: Clarify `Box` representation and its use in FFI This officializes what was only shown as a code example in [the unsafe code guidelines](https://rust-lang.github.io/unsafe-code-guidelines/layout/function-pointers.html?highlight=box#use) and follows [the discussion](https://github.com/rust-lang/unsafe-code-guidelines/issues/157) in the corresponding repository. It is also related to [the issue](https://github.com/rust-lang/rust/issues/52976) regarding marking `Box` `#[repr(transparent)]`. --- src/liballoc/boxed.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 01dee0a3943..50174c3f279 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,6 +63,28 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! +//! `Box` has the same representation as `*mut T`. In particular, when +//! `T: Sized`, this means that `Box` has the same representation as +//! a C pointer, making the following code valid in FFI: +//! +//! ```c +//! /* C header */ +//! struct Foo* foo(); /* Returns ownership */ +//! void bar(struct Foo*); /* `bar` takes ownership */ +//! ``` +//! +//! ``` +//! #[repr(C)] +//! pub struct Foo; +//! +//! #[no_mangle] +//! pub extern "C" fn foo() -> Box { +//! Box::new(Foo) +//! } +//! +//! #[no_mangle] +//! pub extern "C" fn bar(_: Option>) {} +//! ``` //! //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html -- cgit 1.4.1-3-g733a5 From aea94230c476fea2ee073a1bc672125e4586d4b5 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Sun, 25 Aug 2019 23:25:56 -0700 Subject: Update Box representation comment based on reviews --- src/liballoc/boxed.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 50174c3f279..b2b23cc9671 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,9 +63,8 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! `Box` has the same representation as `*mut T`. In particular, when -//! `T: Sized`, this means that `Box` has the same representation as -//! a C pointer, making the following code valid in FFI: +//! `Box` has the same ABI as `&mut T`. In particular, when `T: Sized`, +//! this allows using `Box` in FFI: //! //! ```c //! /* C header */ -- cgit 1.4.1-3-g733a5 From 812ec6a3bf775c1564ed3b12374c4ee81bfa94b8 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Wed, 30 Oct 2019 09:43:04 -0700 Subject: Update FFI example - Use meaningful names - Clarify comments - Fix C function declaration --- src/liballoc/boxed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index b2b23cc9671..a502a5b0a0b 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -68,8 +68,8 @@ //! //! ```c //! /* C header */ -//! struct Foo* foo(); /* Returns ownership */ -//! void bar(struct Foo*); /* `bar` takes ownership */ +//! struct Foo* foo_new(void); /* Returns ownership to the caller */ +//! void foo_delete(struct Foo*); /* Takes ownership from the caller */ //! ``` //! //! ``` @@ -77,12 +77,12 @@ //! pub struct Foo; //! //! #[no_mangle] -//! pub extern "C" fn foo() -> Box { +//! pub extern "C" fn foo_new() -> Box { //! Box::new(Foo) //! } //! //! #[no_mangle] -//! pub extern "C" fn bar(_: Option>) {} +//! pub extern "C" fn foo_delete(_: Option>) {} //! ``` //! //! [dereferencing]: ../../std/ops/trait.Deref.html -- cgit 1.4.1-3-g733a5 From ead115949017533de244049c58f4b6886243eda7 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 9 Dec 2019 22:49:59 -0800 Subject: Use Niko's wording --- src/liballoc/boxed.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index a502a5b0a0b..a7e09d72b4a 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,14 +63,25 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! `Box` has the same ABI as `&mut T`. In particular, when `T: Sized`, -//! this allows using `Box` in FFI: +//! So long as `T: Sized`, a `Box` 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 +//! `Box` to C code by using `Box` as the type on the Rust side, and +//! `T*` as the corresponding type on the C side. As an example, consider this +//! C header which declares functions that create and destroy some kind of +//! `Foo` value: //! //! ```c //! /* C header */ //! struct Foo* foo_new(void); /* Returns ownership to the caller */ //! void foo_delete(struct Foo*); /* Takes ownership from the caller */ //! ``` +//! +//! These two functions might be implemented in Rust as follows. Here, the +//! `struct Foo*` type from C is translated to `Box`, which captures +//! the ownership constraints. Note also that the nullable argument to +//! `foo_delete` is represented in Rust as `Option>`, since `Box` +//! cannot be null. //! //! ``` //! #[repr(C)] @@ -84,6 +95,14 @@ //! #[no_mangle] //! pub extern "C" fn foo_delete(_: Option>) {} //! ``` +//! +//! Even though `Box` 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` +//! and expect things to work. `Box` values will always be fully aligned, +//! non-null pointers. Moreover, the destructor for `Box` will attempt to +//! free the value with the global allocator. In general, the best practice +//! is to only use `Box` for pointers that originated from the global +//! allocator. //! //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html -- cgit 1.4.1-3-g733a5 From fe6ddd5d15c4f0108c32fb731b688f34fbeba788 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 9 Dec 2019 22:56:37 -0800 Subject: Specify behavior when passed a null pointer --- src/liballoc/boxed.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index a7e09d72b4a..2f24086bf2c 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -73,8 +73,12 @@ //! //! ```c //! /* C header */ -//! struct Foo* foo_new(void); /* Returns ownership to the caller */ -//! void foo_delete(struct Foo*); /* Takes ownership from the caller */ +//! +//! /* 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 -- cgit 1.4.1-3-g733a5 From 1a26df77272d3bafc9e9e689a5a4a314896050f5 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Tue, 10 Dec 2019 00:00:25 -0800 Subject: Remove trailing whitespace --- src/liballoc/boxed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 2f24086bf2c..c3732b245f4 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -73,14 +73,14 @@ //! //! ```c //! /* C header */ -//! +//! //! /* 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`, which captures //! the ownership constraints. Note also that the nullable argument to @@ -99,7 +99,7 @@ //! #[no_mangle] //! pub extern "C" fn foo_delete(_: Option>) {} //! ``` -//! +//! //! Even though `Box` 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` //! and expect things to work. `Box` values will always be fully aligned, -- cgit 1.4.1-3-g733a5 From cb1cc1181e884d0f53c444af0b6a21189af83bea Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Tue, 10 Dec 2019 22:18:17 -0800 Subject: Fix description based on review --- src/liballoc/boxed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index c3732b245f4..d50b4b8dc4a 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -65,7 +65,7 @@ //! //! So long as `T: Sized`, a `Box` 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 +//! `T*`). This means that you can have Rust code which passes ownership of a //! `Box` to C code by using `Box` as the type on the Rust side, and //! `T*` as the corresponding type on the C side. As an example, consider this //! C header which declares functions that create and destroy some kind of -- cgit 1.4.1-3-g733a5 From fafa4897985f932490960e90ddd2ff39134e967e Mon Sep 17 00:00:00 2001 From: Nicholas Matsakis Date: Wed, 11 Dec 2019 10:32:11 -0500 Subject: clarify that `Box` should only be used when defined *in Rust* --- src/liballoc/boxed.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index d50b4b8dc4a..c25495bec41 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,13 +63,14 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! So long as `T: Sized`, a `Box` 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 can have Rust code which passes ownership of a -//! `Box` to C code by using `Box` as the type on the Rust side, and -//! `T*` as the corresponding type on the C side. As an example, consider this -//! C header which declares functions that create and destroy some kind of -//! `Foo` value: +//! So long as `T: Sized`, a `Box` 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 if you have extern "C" +//! Rust functions that will be called from C, you can define those +//! Rust functions using `Box` types, and use `T*` as corresponding +//! type on the C side. As an example, consider this C header which +//! declares functions that create and destroy some kind of `Foo` +//! value: //! //! ```c //! /* C header */ @@ -108,6 +109,14 @@ //! is to only use `Box` for pointers that originated from the global //! allocator. //! +//! **Important.** At least at present, you should avoid using +//! `Box` types for functions that are defined in C but invoked +//! from Rust. In those cases, you should directly mirror the C types +//! as closely as possible. Using types like `Box` where the C +//! definition is just using `T*` can lead to undefined behavior, as +//! described in [rust-lang/unsafe-code-guidelines#198][ucg#198]. +//! +//! [ucg#198]: https://github.com/rust-lang/unsafe-code-guidelines/issues/198 //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html //! [`Global`]: ../alloc/struct.Global.html -- cgit 1.4.1-3-g733a5 From 24227977852b22eb0ab3a228b7135611ae49bb8d Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 11 Dec 2019 21:01:33 +0100 Subject: Small Cow improvements --- src/liballoc/borrow.rs | 18 +++++------------- src/liballoc/tests/cow_str.rs | 3 +++ 2 files changed, 8 insertions(+), 13 deletions(-) (limited to 'src/liballoc') diff --git a/src/liballoc/borrow.rs b/src/liballoc/borrow.rs index d2bdda83fa9..fc960451968 100644 --- a/src/liballoc/borrow.rs +++ b/src/liballoc/borrow.rs @@ -195,14 +195,10 @@ impl Clone for Cow<'_, B> { } fn clone_from(&mut self, source: &Self) { - if let Owned(ref mut dest) = *self { - if let Owned(ref o) = *source { - o.borrow().clone_into(dest); - return; - } + match (self, source) { + (&mut Owned(ref mut dest), &Owned(ref o)) => o.borrow().clone_into(dest), + (t, s) => *t = s.clone(), } - - *self = source.clone(); } } @@ -449,9 +445,7 @@ impl<'a> AddAssign<&'a str> for Cow<'a, str> { fn add_assign(&mut self, rhs: &'a str) { if self.is_empty() { *self = Cow::Borrowed(rhs) - } else if rhs.is_empty() { - return; - } else { + } else if !rhs.is_empty() { if let Cow::Borrowed(lhs) = *self { let mut s = String::with_capacity(lhs.len() + rhs.len()); s.push_str(lhs); @@ -467,9 +461,7 @@ impl<'a> AddAssign> for Cow<'a, str> { fn add_assign(&mut self, rhs: Cow<'a, str>) { if self.is_empty() { *self = rhs - } else if rhs.is_empty() { - return; - } else { + } else if !rhs.is_empty() { if let Cow::Borrowed(lhs) = *self { let mut s = String::with_capacity(lhs.len() + rhs.len()); s.push_str(lhs); diff --git a/src/liballoc/tests/cow_str.rs b/src/liballoc/tests/cow_str.rs index 6f357eda9b8..62a5c245a54 100644 --- a/src/liballoc/tests/cow_str.rs +++ b/src/liballoc/tests/cow_str.rs @@ -138,4 +138,7 @@ fn check_cow_clone_from() { let c2: Cow<'_, str> = Cow::Owned(s); c1.clone_from(&c2); assert!(c1.into_owned().capacity() >= 25); + let mut c3: Cow<'_, str> = Cow::Borrowed("bye"); + c3.clone_from(&c2); + assert_eq!(c2, c3); } -- cgit 1.4.1-3-g733a5