From 185036686f1ae0643f44f5154804b5074ffe31f1 Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Wed, 27 Oct 2021 21:13:18 -0400 Subject: [PATCH 1/9] init --- text/0000-take-on-bool.md | 100 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 text/0000-take-on-bool.md diff --git a/text/0000-take-on-bool.md b/text/0000-take-on-bool.md new file mode 100644 index 00000000000..852a04603bb --- /dev/null +++ b/text/0000-take-on-bool.md @@ -0,0 +1,100 @@ +- Feature Name: `take-on-bool` +- Start Date: 2021-10-27 +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) + +# Summary + +This RFC proposes adding a method to `bool` called `take`, which will save the value of the bool to a variable, set the bool to false, and then return that variable. + +# Motivation + +In many applications, deferred execution of routines is preferable, especially around long standing state. Using flags, or booleans which indicate a condition, is a good way to raise those routines. Taking that flag, while also resetting it, makes this pattern more elegant, without complexifying the `std` inordinately. + +# Guide-level explanation + +All `.take` does is return the value of a boolean while also setting that value internally to `false`. It is exactly similar to `Option::take`, except that, of course, it only returns `true/false` instead of some inner value. (In this sense, this `.take` is effectively the same as `Option::take().is_some()`). In any example where flags are commonly read while also being reset, this method is useful. + +For example, imagine a common game structure: + +```rs +/// A recursive data structure of positions, commonly used in games to allow a parent/child relationship between transforms. +pub struct SceneNode { + /// a position relative to this Node's parent. + pub local_position: [f32; 2], + /// an authoritative position + pub world_position: [f32; 2], + + pub dirty: bool, +} + +impl SceneNode { + /// We want a new local position! + pub fn set_local_position(&mut self, new_pos: [f32; 2]) { + self.local_position = new_pos; + self.dirty = true; + } + + /// we magically have the parent in this example. + pub fn calculuate_world_position(&mut self, parent: &SceneNode) { + // we can take the flag and also unset it in one method call + if self.dirty.take() { + self.world_position = [ + self.local_position[0] + parent.local_position[0], + self.local_position[1] + parent.local_position[1], + ]; + } + + /* + // without this RFC, our code would be slightly uglier like this: + if self.dirty { + self.dirty = false; + self.world_position = [ + self.local_position[0] + parent.local_position[0], + self.local_position[1] + parent.local_position[1], + ]; + } + */ + } +} +``` + +# Reference-level explanation + +Implementation should be the following: + +```rs +pub fn take(&mut self) -> bool { + // save the old value to a variable + let val = *self; + // and reset ourselves to false. If we are already false, + // then this doesn't matter. + *self = false; + + val +} +``` + +# Drawbacks + +Save the usual drawbacks of adding a new method to the standard library, there are no drawbacks. + +# Rationale and alternatives + +There is also a possible implementation that only conditionally resets `self` if self is `true` -- in practice, the above implementation, and that implementation, will make the same machine code for users (ie, the user is probably doing an `if` check themselves). I think the above is more readable. + +Users can also use `Option::<()>` instead of `bool` to get this functionality now, for free. Additionally, they could simply wrap `bool` and Deref into it, with the added `.take`. + +This functionality could instead be provided by a crate (e.g. boolinator), but this functionality can be commonly desired and is in keeping with `Option::take`. A crate, however, could provide even more `bool` methods, like `toggle`, which may be useful to some users. + +# Prior art + +None. + +# Unresolved questions + +None + +# Future possibilities + +We could later add `toggle`. From 22f62345be9e41a3d6927c2742c1f334f26c9c57 Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Wed, 27 Oct 2021 21:14:50 -0400 Subject: [PATCH 2/9] equivicate --- text/0000-take-on-bool.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-take-on-bool.md b/text/0000-take-on-bool.md index 852a04603bb..dde93fbce02 100644 --- a/text/0000-take-on-bool.md +++ b/text/0000-take-on-bool.md @@ -89,7 +89,7 @@ This functionality could instead be provided by a crate (e.g. boolinator), but t # Prior art -None. +None, as far as I know. # Unresolved questions From 1bee972a16f779fd5006ed28f0720a531952e12e Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Fri, 29 Oct 2021 11:01:55 -0400 Subject: [PATCH 3/9] expanded rationals --- text/0000-take-on-bool.md | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/text/0000-take-on-bool.md b/text/0000-take-on-bool.md index dde93fbce02..087acf36ec3 100644 --- a/text/0000-take-on-bool.md +++ b/text/0000-take-on-bool.md @@ -81,7 +81,33 @@ Save the usual drawbacks of adding a new method to the standard library, there a # Rationale and alternatives -There is also a possible implementation that only conditionally resets `self` if self is `true` -- in practice, the above implementation, and that implementation, will make the same machine code for users (ie, the user is probably doing an `if` check themselves). I think the above is more readable. +There are two other possible implementations of the method: + +1. Conditionally branching: + + ```rs + pub fn take(&mut self) -> bool { + if *self { + *self = false; + true + } else { + false + } + } + ``` + +2. Using `mem::replace` or `mem::take`: + + ```rs + pub fn take(&mut self) -> bool { + // or core::mem::take(self) + core::mem::replace(self, false) + } + ``` + +In practice, the proposed implementation produces identical code using Godbolt to #2, and the proposed implementation and #2 seem to always produce better code than the #1 alternative above (specifically, they tend to elide jumps more easily). However, in more complex code, these all seem to resolve to more or less the same code, so it's a fairly bike-sheddable difference. + +Alternatives to this metohd entirely for users are, of course, just writing the code out themselves. Sometimes they may even be preferable for simplicity. Users can also use `Option::<()>` instead of `bool` to get this functionality now, for free. Additionally, they could simply wrap `bool` and Deref into it, with the added `.take`. From e316f5f67887a13b229ae13425412cd80e9a809b Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Fri, 29 Oct 2021 11:06:29 -0400 Subject: [PATCH 4/9] updating rfc number --- text/{0000-take-on-bool.md => 3189-take-on-bool.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename text/{0000-take-on-bool.md => 3189-take-on-bool.md} (100%) diff --git a/text/0000-take-on-bool.md b/text/3189-take-on-bool.md similarity index 100% rename from text/0000-take-on-bool.md rename to text/3189-take-on-bool.md From 2673b666f9c9cc5da5448583d3edfe51c574c8b4 Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Fri, 29 Oct 2021 11:06:46 -0400 Subject: [PATCH 5/9] bumping internal pr --- text/3189-take-on-bool.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/3189-take-on-bool.md b/text/3189-take-on-bool.md index 087acf36ec3..7549290f6ec 100644 --- a/text/3189-take-on-bool.md +++ b/text/3189-take-on-bool.md @@ -1,6 +1,6 @@ - Feature Name: `take-on-bool` - Start Date: 2021-10-27 -- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- RFC PR: [rust-lang/rfcs#3189](https://github.com/rust-lang/rfcs/pull/0000) - Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) # Summary From ef97acecac586d524da0f6aae71b22e8589a6e3d Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Fri, 29 Oct 2021 11:08:09 -0400 Subject: [PATCH 6/9] updating pr number --- text/3189-take-on-bool.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/3189-take-on-bool.md b/text/3189-take-on-bool.md index 7549290f6ec..b4a34e4598a 100644 --- a/text/3189-take-on-bool.md +++ b/text/3189-take-on-bool.md @@ -1,6 +1,6 @@ - Feature Name: `take-on-bool` - Start Date: 2021-10-27 -- RFC PR: [rust-lang/rfcs#3189](https://github.com/rust-lang/rfcs/pull/0000) +- RFC PR: [rust-lang/rfcs#3189](https://github.com/rust-lang/rfcs/pull/3189) - Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) # Summary From 8b42cd0e5e33d58628c1a7aae7f40e33a39616ef Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Fri, 29 Oct 2021 11:19:40 -0400 Subject: [PATCH 7/9] typo --- text/3189-take-on-bool.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/3189-take-on-bool.md b/text/3189-take-on-bool.md index b4a34e4598a..81c29f7f193 100644 --- a/text/3189-take-on-bool.md +++ b/text/3189-take-on-bool.md @@ -107,7 +107,7 @@ There are two other possible implementations of the method: In practice, the proposed implementation produces identical code using Godbolt to #2, and the proposed implementation and #2 seem to always produce better code than the #1 alternative above (specifically, they tend to elide jumps more easily). However, in more complex code, these all seem to resolve to more or less the same code, so it's a fairly bike-sheddable difference. -Alternatives to this metohd entirely for users are, of course, just writing the code out themselves. Sometimes they may even be preferable for simplicity. +Alternatives to this method entirely for users are, of course, just writing the code out themselves. Sometimes that may even be preferable for simplicity. Users can also use `Option::<()>` instead of `bool` to get this functionality now, for free. Additionally, they could simply wrap `bool` and Deref into it, with the added `.take`. From 730d6f8fd77c9191eff2807b9d5a5bb5b04a9d5b Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Thu, 4 Nov 2021 18:05:34 -0400 Subject: [PATCH 8/9] updated with feedback --- text/3189-take-on-bool.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/3189-take-on-bool.md b/text/3189-take-on-bool.md index 81c29f7f193..c70485d1518 100644 --- a/text/3189-take-on-bool.md +++ b/text/3189-take-on-bool.md @@ -13,7 +13,7 @@ In many applications, deferred execution of routines is preferable, especially a # Guide-level explanation -All `.take` does is return the value of a boolean while also setting that value internally to `false`. It is exactly similar to `Option::take`, except that, of course, it only returns `true/false` instead of some inner value. (In this sense, this `.take` is effectively the same as `Option::take().is_some()`). In any example where flags are commonly read while also being reset, this method is useful. +All .take() does is return the value of a boolean while also setting that value internally to false. It is just like `mem::take`, except it is called as a method instead of a free function. In places where booleans are commonly read and then reset, like dirty flags, this method is useful. For example, imagine a common game structure: @@ -36,7 +36,7 @@ impl SceneNode { } /// we magically have the parent in this example. - pub fn calculuate_world_position(&mut self, parent: &SceneNode) { + pub fn calculate_world_position(&mut self, parent: &SceneNode) { // we can take the flag and also unset it in one method call if self.dirty.take() { self.world_position = [ @@ -115,7 +115,7 @@ This functionality could instead be provided by a crate (e.g. boolinator), but t # Prior art -None, as far as I know. +Prior art: `AtomicBool::compare_exchange`, which is used for similar purposes. It is a more complex operation, because it allows specifying memory orderings (irrelevant here) and because it can set the boolean to either true or false, or act effectively as a read without modifying. But, it is closely related in that, for example, code being migrated towards or away from Sync support might replace one with the other. # Unresolved questions From d8f615b3f3841a5130c5a18edefd80662149c83b Mon Sep 17 00:00:00 2001 From: Jack OntheGoMac Date: Fri, 5 Nov 2021 16:11:07 -0400 Subject: [PATCH 9/9] added take rational --- text/3189-take-on-bool.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/text/3189-take-on-bool.md b/text/3189-take-on-bool.md index c70485d1518..5f1c46e941f 100644 --- a/text/3189-take-on-bool.md +++ b/text/3189-take-on-bool.md @@ -11,6 +11,10 @@ This RFC proposes adding a method to `bool` called `take`, which will save the v In many applications, deferred execution of routines is preferable, especially around long standing state. Using flags, or booleans which indicate a condition, is a good way to raise those routines. Taking that flag, while also resetting it, makes this pattern more elegant, without complexifying the `std` inordinately. +The current one liner to get this same effect is `mem::take`. Adding `mem` into a project often implies a more complex operation than `.take` on a `Copy` tends to be, since `mem::take` (and `mem::replace` in general) is most useful when the type isn't Copy like `bool` is, which allows users to trivially do operations which otherwise require some non-trivial unsafe code. Moreover, in general, the `mem` module has a collection of relatively low-level operations on memory patterns. This makes it a more intimidating toolbox to reach for than a simple method on `bool` could be. + +There are two places where a "Take" pattern is used that is more about API usability, rather than memory handling, but where `mem::take` could be used: `Option` and `bool`. For the former, we already have the `.take` method (and in my Rust experience, anecdotally, is used often). We don't have anything for the latter. This PR's motivation is adding such a method for `bool`. + # Guide-level explanation All .take() does is return the value of a boolean while also setting that value internally to false. It is just like `mem::take`, except it is called as a method instead of a free function. In places where booleans are commonly read and then reset, like dirty flags, this method is useful.