From 11a485e3c925b2af93d8e739da6df93bc1821a8c Mon Sep 17 00:00:00 2001 From: ticki Date: Thu, 1 Sep 2016 12:11:11 +0200 Subject: [PATCH 1/2] RFC: `core::mem::replace_with` for temporarily moving out of ownership. This common pattern deserves a place in the standard library. --- text/0000-mem-replace-with.md | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 text/0000-mem-replace-with.md diff --git a/text/0000-mem-replace-with.md b/text/0000-mem-replace-with.md new file mode 100644 index 00000000000..d67cd96d4b9 --- /dev/null +++ b/text/0000-mem-replace-with.md @@ -0,0 +1,64 @@ +- Feature Name: mem-replace-with +- Start Date: 2016-09-01 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Add a new function to `core::mem` (and thus `std::mem`), `replace_with`, which +invokes a closure that takes temporary ownership over some value behind a +mutable reference. + +This is similar to the functions provided by [the take_mut crate](https://crates.io/crates/take_mut). + +# Motivation +[motivation]: #motivation + +A common pattern, especially for low-level programmers, is to acquire ownership +of a value behind a reference without immediately giving it a replacement value. + +`core::mem::replace_with` allows such thing by generalizing +`core::mem::replace`, to allow the replacement value being generated by a +closure and even depend on the old value. + +Unfortunately, there are no safe implementation in the standard library, +leading some Rust users to either reimplement a safe interface for such a +feature (often flawed due to unwinding, see below) or use things like +`ptr::read`. + +This should standardize the ecosystem with respect to that. + +# Detailed design +[design]: #detailed-design + +A complete implementation can be found [here](https://github.com/rust-lang/rust/pull/36186). This implementation is pretty simple, with only one trick which is crucial to safety: handling unwinding. + +Without handling unwinding, the closure could unwind and invoke destructors on +the invalid value. Thus we use "exit guards" which are placed in the stack +frame (as a variable) and holds a destructor exiting the program. + +This behavior is rather specific and it is not certain that it will be kept in +the future, so we leave the unwinding behavior unspecified for now. + +# Drawbacks +[drawbacks]: #drawbacks + +It expands the core library slightly, and the panic semantics might not be +clear for everyone. + +# Alternatives +[alternatives]: #alternatives + +1. Specify the unwinding behavior to abort. + +2. Use `catch_unwind` and print an error on unwinding (in libstd). + +3. Require `T: Default` and replace the value as such on panic, so we are sure the dropping value is in fact valid. + +4. Leave it out of the standard library. + +# Unresolved questions +[unresolved]: #unresolved-questions + +Are there currently any crates relying on invariants that this will break? From d5351e8915e8989f4e1e12b82c5be1b1f54d0bb1 Mon Sep 17 00:00:00 2001 From: ticki Date: Thu, 13 Oct 2016 20:28:03 +0200 Subject: [PATCH 2/2] Change the RFC to add a error message if unwinding. --- text/0000-mem-replace-with.md | 59 ++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/text/0000-mem-replace-with.md b/text/0000-mem-replace-with.md index d67cd96d4b9..00e962b2689 100644 --- a/text/0000-mem-replace-with.md +++ b/text/0000-mem-replace-with.md @@ -38,8 +38,61 @@ Without handling unwinding, the closure could unwind and invoke destructors on the invalid value. Thus we use "exit guards" which are placed in the stack frame (as a variable) and holds a destructor exiting the program. -This behavior is rather specific and it is not certain that it will be kept in -the future, so we leave the unwinding behavior unspecified for now. +This behavior is simply specified to match the one of panicking inside +unwinding destructors. This has the nice property of allowing error messages in +the libcore implementation. + +## Implementation + +```rust +/// A guarding type which will abort upon drop. +/// +/// This is used for catching unwinding and transforming it into abort. +/// +/// The destructor should never be called naturally (use `mem::forget()`), and only when unwinding. +struct ExitGuard; + +impl Drop for ExitGuard { + fn drop(&mut self) { + // To avoid unwinding, we abort (we panic, which is equivalent to abort inside an unwinding + // destructor) the program, which ensures that the destructor of the invalidated value + // isn't runned, since this destructor ought to be called only if unwinding happens. + panic!("`replace_with` closure unwinded. For safety reasons, this will \ + abort your program. Check the documentation"); + } +} + +/// Temporarily takes ownership of a value at a mutable location, and replace it with a new value +/// based on the old one. +/// +/// We move out of reference temporarily, to apply a closure, returning a new value, which is then +/// placed at the original value's location. +/// +/// # An important note +/// +/// The behavior on panic (or to be more precise, unwinding) is specified to match the behavior of +/// panicking inside a destructor, which itself is simply specified to not unwind. +#[inline] +#[unstable(feature = "replace_with", issue = "...")] +pub fn replace_with(val: &mut T, replace: F) + where F: FnOnce(T) -> T { + // Guard against unwinding. Note that this is critical to safety, to avoid the value behind the + // reference `val` is not dropped twice during unwinding. + let guard = ExitGuard; + + unsafe { + // Take out the value behind the pointer. + let old = ptr::read(val); + // Run the closure. + let new = closure(old); + // Put the result back. + ptr::write(val, new); + } + + // Forget the guard, to avoid panicking. + mem::forget(guard); +} +``` # Drawbacks [drawbacks]: #drawbacks @@ -54,8 +107,6 @@ clear for everyone. 2. Use `catch_unwind` and print an error on unwinding (in libstd). -3. Require `T: Default` and replace the value as such on panic, so we are sure the dropping value is in fact valid. - 4. Leave it out of the standard library. # Unresolved questions