From 7b78086ba4c6bfc656bc0d8b3171aa9929f593dc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 7 May 2015 10:49:39 -0700 Subject: [PATCH 1/4] std: Mark `mem::forget` as a safe function This commit is an implementation of [RFC 1066][rfc] where the conclusion was that leaking a value is a safe operation in Rust code, so updating the signature of this function follows suit. [rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1066-safe-mem-forget.md Closes #25186 --- src/libcollections/vec.rs | 4 +-- src/libcore/intrinsics.rs | 4 --- src/libcore/mem.rs | 51 ++++++++++++++++++++++++++++---- src/libstd/sys/unix/fd.rs | 2 +- src/libstd/sys/windows/handle.rs | 2 +- 5 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index 6195056579dfc..d5112c9fdff9b 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -637,7 +637,7 @@ impl Vec { // zero-size types consume no memory, so we can't rely on the // address space running out self.len = self.len.checked_add(1).expect("length overflow"); - unsafe { mem::forget(value); } + mem::forget(value); return } @@ -959,7 +959,7 @@ impl Vec { num_u: 0, marker: PhantomData, }; - unsafe { mem::forget(vec); } + mem::forget(vec); while pv.num_t != 0 { unsafe { diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 8ed89adec5b69..077d6367ff3c0 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -217,10 +217,6 @@ extern "rust-intrinsic" { pub fn uninit() -> T; /// Moves a value out of scope without running drop glue. - /// - /// `forget` is unsafe because the caller is responsible for - /// ensuring the argument is deallocated already. - #[stable(feature = "rust1", since = "1.0.0")] pub fn forget(_: T) -> (); /// Unsafely transforms a value of one type into a value of another type. diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index c4128e79765c8..a149af3a44063 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -22,15 +22,54 @@ use ptr; #[stable(feature = "rust1", since = "1.0.0")] pub use intrinsics::transmute; -/// Moves a thing into the void. +/// Leaks a value into the void, consuming ownership and never running its +/// destructor. /// -/// The forget function will take ownership of the provided value but neglect -/// to run any required cleanup or memory management operations on it. +/// This function will take ownership of its argument, but is distinct from the +/// `mem::drop` function in that it **does not run the destructor**, leaking the +/// value and any resources that it owns. /// -/// This function is the unsafe version of the `drop` function because it does -/// not run any destructors. +/// # Safety +/// +/// This function is not marked as `unsafe` as Rust does not guarantee that the +/// `Drop` implementation for a value will always run. Note, however, that +/// leaking resources such as memory or I/O objects is likely not desired, so +/// this function is only recommended for specialized use cases. +/// +/// The safety of this function implies that when writing `unsafe` code +/// yourself care must be taken when leveraging a destructor that is required to +/// run to preserve memory safety. There are known situations where the +/// destructor may not run (such as if ownership of the object with the +/// destructor is returned) which must be taken into account. +/// +/// # Other forms of Leakage +/// +/// It's important to point out that this function is not the only method by +/// which a value can be leaked in safe Rust code. Other known sources of +/// leakage are: +/// +/// * `Rc` and `Arc` cycles +/// * `mpsc::{Sender, Receiver}` cycles (they use `Arc` internally) +/// * Panicking destructors are likely to leak local resources +/// +/// # Example +/// +/// ```rust,no_run +/// use std::mem; +/// use std::fs::File; +/// +/// // Leak some heap memory by never deallocating it +/// let heap_memory = Box::new(3); +/// mem::forget(heap_memory); +/// +/// // Leak an I/O object, never closing the file +/// let file = File::open("foo.txt").unwrap(); +/// mem::forget(file); +/// ``` #[stable(feature = "rust1", since = "1.0.0")] -pub use intrinsics::forget; +pub fn forget(t: T) { + unsafe { intrinsics::forget(t) } +} /// Returns the size of a type in bytes. /// diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index e5bdb554359df..026380027d29e 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -31,7 +31,7 @@ impl FileDesc { /// Extracts the actual filedescriptor without closing it. pub fn into_raw(self) -> c_int { let fd = self.fd; - unsafe { mem::forget(self) }; + mem::forget(self); fd } diff --git a/src/libstd/sys/windows/handle.rs b/src/libstd/sys/windows/handle.rs index 9481e180ce578..c835d503388c7 100644 --- a/src/libstd/sys/windows/handle.rs +++ b/src/libstd/sys/windows/handle.rs @@ -32,7 +32,7 @@ impl Handle { pub fn into_raw(self) -> HANDLE { let ret = self.0; - unsafe { mem::forget(self) } + mem::forget(self); return ret; } From 5d412c6a4b78e4003c2f010fc658d9e75dfc45c9 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 6 May 2015 15:53:34 -0700 Subject: [PATCH 2/4] core: impl AsRef<[u8]> for str --- src/libcore/str/mod.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 31431f4496226..bdd9f5a7ba798 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -23,6 +23,7 @@ use self::pattern::{Searcher, ReverseSearcher, DoubleEndedSearcher}; use char::CharExt; use clone::Clone; use cmp::{self, Eq}; +use convert::AsRef; use default::Default; use fmt; use iter::ExactSizeIterator; @@ -1843,6 +1844,14 @@ impl StrExt for str { fn parse(&self) -> Result { FromStr::from_str(self) } } +#[stable(feature = "rust1", since = "1.0.0")] +impl AsRef<[u8]> for str { + #[inline] + fn as_ref(&self) -> &[u8] { + self.as_bytes() + } +} + /// Pluck a code point out of a UTF-8-like byte slice and return the /// index of the next code point. #[inline] From 91f1e5135510e8675ef5c70ecfd753e7f438c6b6 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 6 May 2015 15:53:53 -0700 Subject: [PATCH 3/4] collections: impl AsRef<[u8]> for String --- src/libcollections/string.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libcollections/string.rs b/src/libcollections/string.rs index 41ad7c2d783bd..66fa4573bd7c6 100644 --- a/src/libcollections/string.rs +++ b/src/libcollections/string.rs @@ -1005,6 +1005,14 @@ impl AsRef for String { } } +#[stable(feature = "rust1", since = "1.0.0")] +impl AsRef<[u8]> for String { + #[inline] + fn as_ref(&self) -> &[u8] { + self.as_bytes() + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl<'a> From<&'a str> for String { #[inline] From f245929834543ca9a00278827a82d29ae3b2770f Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Fri, 8 May 2015 17:13:35 -0700 Subject: [PATCH 4/4] collections: change bounds of SliceConcatExt implementations to use Borrow instead of AsRef Conflicts: src/libcollections/slice.rs src/libcollections/str.rs --- src/libcollections/slice.rs | 11 +++++------ src/libcollections/str.rs | 11 +++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/libcollections/slice.rs b/src/libcollections/slice.rs index 6622d8a9c4063..8a566445e388e 100644 --- a/src/libcollections/slice.rs +++ b/src/libcollections/slice.rs @@ -80,7 +80,6 @@ #![stable(feature = "rust1", since = "1.0.0")] use alloc::boxed::Box; -use core::convert::AsRef; use core::clone::Clone; use core::cmp::Ordering::{self, Greater, Less}; use core::cmp::{self, Ord, PartialEq}; @@ -1028,23 +1027,23 @@ pub trait SliceConcatExt { fn connect(&self, sep: &T) -> U; } -impl> SliceConcatExt> for [V] { +impl> SliceConcatExt> for [V] { fn concat(&self) -> Vec { - let size = self.iter().fold(0, |acc, v| acc + v.as_ref().len()); + let size = self.iter().fold(0, |acc, v| acc + v.borrow().len()); let mut result = Vec::with_capacity(size); for v in self { - result.push_all(v.as_ref()) + result.push_all(v.borrow()) } result } fn connect(&self, sep: &T) -> Vec { - let size = self.iter().fold(0, |acc, v| acc + v.as_ref().len()); + let size = self.iter().fold(0, |acc, v| acc + v.borrow().len()); let mut result = Vec::with_capacity(size + self.len()); let mut first = true; for v in self { if first { first = false } else { result.push(sep.clone()) } - result.push_all(v.as_ref()) + result.push_all(v.borrow()) } result } diff --git a/src/libcollections/str.rs b/src/libcollections/str.rs index 7a28b56b7c1bb..6edc62785e862 100644 --- a/src/libcollections/str.rs +++ b/src/libcollections/str.rs @@ -61,7 +61,6 @@ use core::str::pattern::Pattern; use core::str::pattern::{Searcher, ReverseSearcher, DoubleEndedSearcher}; use unicode::str::{UnicodeStr, Utf16Encoder}; -use core::convert::AsRef; use vec_deque::VecDeque; use borrow::{Borrow, ToOwned}; use string::String; @@ -85,18 +84,18 @@ pub use core::str::pattern; Section: Creating a string */ -impl> SliceConcatExt for [S] { +impl> SliceConcatExt for [S] { fn concat(&self) -> String { if self.is_empty() { return String::new(); } // `len` calculation may overflow but push_str will check boundaries - let len = self.iter().map(|s| s.as_ref().len()).sum(); + let len = self.iter().map(|s| s.borrow().len()).sum(); let mut result = String::with_capacity(len); for s in self { - result.push_str(s.as_ref()) + result.push_str(s.borrow()) } result @@ -115,7 +114,7 @@ impl> SliceConcatExt for [S] { // this is wrong without the guarantee that `self` is non-empty // `len` calculation may overflow but push_str but will check boundaries let len = sep.len() * (self.len() - 1) - + self.iter().map(|s| s.as_ref().len()).sum::(); + + self.iter().map(|s| s.borrow().len()).sum::(); let mut result = String::with_capacity(len); let mut first = true; @@ -125,7 +124,7 @@ impl> SliceConcatExt for [S] { } else { result.push_str(sep); } - result.push_str(s.as_ref()); + result.push_str(s.borrow()); } result }