From fc4f904ff668621853087598a30a698a3458a887 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 6 Jul 2021 12:51:36 -0400 Subject: [PATCH 1/6] add design notes from lang-team#86 --- src/SUMMARY.md | 1 + src/design_notes/eager_drop.md | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 src/design_notes/eager_drop.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 77d9658..142d44a 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -12,3 +12,4 @@ - [Generalizing coroutines](./design_notes/general_coroutines.md) - [Extending the capabilities of compiler-generated function types](./design_notes/fn_type_trait_impls.md) - [Auto traits](./design_notes/auto_traits.md) + - [Eager drop](./design_notes/eager_drop.md) diff --git a/src/design_notes/eager_drop.md b/src/design_notes/eager_drop.md new file mode 100644 index 0000000..63f214b --- /dev/null +++ b/src/design_notes/eager_drop.md @@ -0,0 +1,63 @@ +# Eager drop design note + +- Project proposal [rust-lang/lang-team#86](https://github.com/rust-lang/lang-team/issues/86) + +## Observations + +### Any attempt to make drop run more eagerly will have to take borrows into account + +The original proposal was to use "CFG dead" as a criteria, but it's pretty clear that this will not work well. Example: + +```rust= +{ + let x = String::new(); + let y = &x; + // last use of x is here + println!("{}", y); + // but we use y here + ... +} +``` + +Here, the fact that `y` (indirectly) uses `x` feels like an important thing to take into account. + +### Some destructors can be run "at any time"... + +Some destructors have very significant side-effects. The most notable example is dropping a lock guard. + +Others correspond solely to "releasing resources": freeing memory is the most common example, but another might be replacing an entry in a table because you are done using it. + +### ...but sometimes that significance is only known at the call site + +However, it can be hard to know what is significant. For a lock guard, for example, if the lock is just being used to guard the data, then moving the lock release early is actually _desirable_, because you want to release the lock as soon as you are doing changing the data. But sometimes you have a `Mutex<()>`, in which case the lock has extra semantics. It's hard to know for sure. + +### Smarter drop placement will mean that adding uses of a variable changes when its destructor runs + +This is not necesarily a problem, but it's an obvious implication: right now, the drop always runs when we exit the scope, so adding further uses to a variable has no effect, but that would have to change. That could be surprising (e.g., adding a debug printout changes the time when a lock is released). + +In contrast, if you add an early drop `drop(foo)` today, you get helpful error messages when you try to use it again. + +In other words, it's useful to have the _destructor_ occurring at a known time (sometimes...). + +### Today's drop rules are, however, a source of confusion + +The `let _guard = foo` pattern can be easily confused with `let _ = foo`. `let guard = foo; ...; drop(guard);` has the advantage of explicitness, so does something like `foo.with(|guard| ...)` + +Temporary rules interact with `match` in ways that surprise people: + +```rust= +match foo.lock().data.copy_out() { + ... +} // lock released here! +``` + +Temporary rules interact poorly with unsafe code today, e.g. `CString::new().as_ptr()` is a known footgun. Would this change help? Or perhaps just change the footguns around. (The change as _written_ would not help here, because the temporary would be dropped probably _even earlier_.) + +## Alternatives + +- Scoped methods +- let blocks +- "Defer" type constructs or scoped guard type constructs from other languages + - Go + - D + - Python From 0c1166367a6fefc4a5f26496dc5e5bb5954658a9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 6 Jul 2021 17:04:50 -0400 Subject: [PATCH 2/6] minor edits in response to feedback --- src/design_notes/eager_drop.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/design_notes/eager_drop.md b/src/design_notes/eager_drop.md index 63f214b..c23ac53 100644 --- a/src/design_notes/eager_drop.md +++ b/src/design_notes/eager_drop.md @@ -41,9 +41,7 @@ In other words, it's useful to have the _destructor_ occurring at a known time ( ### Today's drop rules are, however, a source of confusion -The `let _guard = foo` pattern can be easily confused with `let _ = foo`. `let guard = foo; ...; drop(guard);` has the advantage of explicitness, so does something like `foo.with(|guard| ...)` - -Temporary rules interact with `match` in ways that surprise people: +The fact that `let _ = foo` drops `foo` immediately is a known source of confusion, along with the lifetimes of temporaries in `match` statements and the like: ```rust= match foo.lock().data.copy_out() { @@ -51,7 +49,13 @@ match foo.lock().data.copy_out() { } // lock released here! ``` -Temporary rules interact poorly with unsafe code today, e.g. `CString::new().as_ptr()` is a known footgun. Would this change help? Or perhaps just change the footguns around. (The change as _written_ would not help here, because the temporary would be dropped probably _even earlier_.) +The `let _guard = foo` pattern is probably what prople want, but it's not necessarily obvious to readers when the destructor runs. + +`let guard = foo; ...; drop(guard);` has the advantage of explicitness, so does something like `foo.with(|guard| ...)` + +### Clarify for unsafe code can be quite important + +There are known footguns today with the timing of destructors and unsafe code. For example, `CString::new().as_ptr()` is a common thing people try to do that does not work. Eager destructors would enable more motion, which might exacerbate the problem. ## Alternatives From cc8033fa5b73acdcc241a7c4cb721f286fbb30af Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 6 Jul 2021 20:24:49 -0400 Subject: [PATCH 3/6] Update src/design_notes/eager_drop.md Co-authored-by: Daniel Henry-Mantilla --- src/design_notes/eager_drop.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/design_notes/eager_drop.md b/src/design_notes/eager_drop.md index c23ac53..8988b14 100644 --- a/src/design_notes/eager_drop.md +++ b/src/design_notes/eager_drop.md @@ -53,7 +53,7 @@ The `let _guard = foo` pattern is probably what prople want, but it's not necess `let guard = foo; ...; drop(guard);` has the advantage of explicitness, so does something like `foo.with(|guard| ...)` -### Clarify for unsafe code can be quite important +### Clarity for unsafe code can be quite important There are known footguns today with the timing of destructors and unsafe code. For example, `CString::new().as_ptr()` is a common thing people try to do that does not work. Eager destructors would enable more motion, which might exacerbate the problem. From 9fc84e1d9c5c424909c579199d8253714cb03c43 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 6 Jul 2021 20:30:54 -0400 Subject: [PATCH 4/6] clarify further --- src/design_notes/eager_drop.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/design_notes/eager_drop.md b/src/design_notes/eager_drop.md index 8988b14..dc22f63 100644 --- a/src/design_notes/eager_drop.md +++ b/src/design_notes/eager_drop.md @@ -41,16 +41,21 @@ In other words, it's useful to have the _destructor_ occurring at a known time ( ### Today's drop rules are, however, a source of confusion -The fact that `let _ = foo` drops `foo` immediately is a known source of confusion, along with the lifetimes of temporaries in `match` statements and the like: +The semantics of `let _ = ` have been known to caught a lot of confusion, particularly given the interaction of place expressions and value expresssions: -```rust= +- `let _ = foo` -- no effect +- `let _ = foo()` -- immediately drops the result of invoking `foo()` +- `let _guard = foo` -- moves `foo` into `_guard` and drops at the end of the block +- `let _guard = foo()` -- moves `foo()` into `_guard` and drops at the end of the block + +Another common source of confusion is the lifetimes of temporaries in `match` statements and the like: + +```rust match foo.lock().data.copy_out() { ... } // lock released here! ``` -The `let _guard = foo` pattern is probably what prople want, but it's not necessarily obvious to readers when the destructor runs. - `let guard = foo; ...; drop(guard);` has the advantage of explicitness, so does something like `foo.with(|guard| ...)` ### Clarity for unsafe code can be quite important @@ -65,3 +70,4 @@ There are known footguns today with the timing of destructors and unsafe code. F - Go - D - Python +- Note that the [scopeguard](https://crates.io/crates/scopeguard) crate offers macros like `defer!` that inject a let into the block. From d952240a6f24eb77f421f180ef4fbf72edcf64a8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 16 Jul 2021 06:49:57 -0400 Subject: [PATCH 5/6] Update src/design_notes/eager_drop.md Co-authored-by: Josh Triplett --- src/design_notes/eager_drop.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/design_notes/eager_drop.md b/src/design_notes/eager_drop.md index dc22f63..88f2e04 100644 --- a/src/design_notes/eager_drop.md +++ b/src/design_notes/eager_drop.md @@ -62,6 +62,8 @@ match foo.lock().data.copy_out() { There are known footguns today with the timing of destructors and unsafe code. For example, `CString::new().as_ptr()` is a common thing people try to do that does not work. Eager destructors would enable more motion, which might exacerbate the problem. +In addition, unsafe code means we might not be able to know the semantics associated with a destructor, such as what precisely a `Mutex<()>` guards, and moving a drop earlier *will* break some unsafe code in hard-to-detect ways. + ## Alternatives - Scoped methods From f434d73b070ae53582a3c66423cf5f67e20853db Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 16 Jul 2021 06:50:06 -0400 Subject: [PATCH 6/6] Update src/design_notes/eager_drop.md Co-authored-by: Josh Triplett --- src/design_notes/eager_drop.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/design_notes/eager_drop.md b/src/design_notes/eager_drop.md index 88f2e04..685cd7d 100644 --- a/src/design_notes/eager_drop.md +++ b/src/design_notes/eager_drop.md @@ -72,4 +72,5 @@ In addition, unsafe code means we might not be able to know the semantics associ - Go - D - Python -- Note that the [scopeguard](https://crates.io/crates/scopeguard) crate offers macros like `defer!` that inject a let into the block. +- Built-in macros or RAII/closure-based helpers in the standard library. + - Note that the [scopeguard](https://crates.io/crates/scopeguard) crate offers macros like `defer!` that inject a let into the block.