Skip to content

Commit 47ec919

Browse files
committed
Merge pull request #247 from Gankro/entries-update
Merging as per [weekly meeting decision](https://github.com/rust-lang/meeting-minutes/blob/master/weekly-meetings/2014-09-23.md#entry-revisions).
2 parents 320cc72 + 2fe1cf0 commit 47ec919

File tree

1 file changed

+34
-31
lines changed

1 file changed

+34
-31
lines changed

active/0060-collection-views.md

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ is *already* in the map, and when it's *not*. One way to do this is the followin
1919

2020
```
2121
if map.contains_key(&key) {
22-
let v = map.find_mut(&key).unwrap();
23-
let new_v = *v + 1;
24-
*v = new_v;
22+
*map.find_mut(&key).unwrap() += 1;
2523
} else {
2624
map.insert(key, 1);
2725
}
@@ -58,8 +56,8 @@ composability. Thus, this RFC proposes the same solution to the internal mutatio
5856
# Detailed design
5957

6058
A fully tested "proof of concept" draft of this design has been implemented on top of hashmap,
61-
as it seems to be the worst offender, while still being easy to work with. You can
62-
[read the diff here](https://github.com/Gankro/rust/commit/39a1fa7c7362a3e22e59ab6601ac09475daff39b).
59+
as it seems to be the worst offender, while still being easy to work with. It sits as a pull request
60+
[here](https://github.com/rust-lang/rust/pull/17378).
6361

6462
All the internal mutation methods are replaced with a single method on a collection: `entry`.
6563
The signature of `entry` will depend on the specific collection, but generally it will be similar to
@@ -87,26 +85,29 @@ pub enum Entry<'a, K, V> {
8785
}
8886
```
8987

90-
Of course, the real meat of the API is in the View's interface (impl details removed):
88+
Of course, the real meat of the API is in the Entry's interface (impl details removed):
9189

9290
```
9391
impl<'a, K, V> OccupiedEntry<'a, K, V> {
94-
/// Get a reference to the value of this Entry
92+
/// Gets a reference to the value of this Entry
9593
pub fn get(&self) -> &V;
9694
97-
/// Get a mutable reference to the value of this Entry
95+
/// Gets a mutable reference to the value of this Entry
9896
pub fn get_mut(&mut self) -> &mut V;
9997
100-
/// Set the value stored in this Entry
101-
pub fn set(mut self, value: V) -> V;
98+
/// Converts the entry into a mutable reference to its value
99+
pub fn into_mut(self) -> &'a mut V;
102100
103-
/// Take the value stored in this Entry
101+
/// Sets the value stored in this Entry
102+
pub fn set(&mut self, value: V) -> V;
103+
104+
/// Takes the value stored in this Entry
104105
pub fn take(self) -> V;
105106
}
106107
107108
impl<'a, K, V> VacantEntry<'a, K, V> {
108-
/// Set the value stored in this Entry
109-
pub fn set(self, value: V);
109+
/// Set the value stored in this Entry, and returns a reference to it
110+
pub fn set(self, value: V) -> &'a mut V;
110111
}
111112
```
112113

@@ -129,27 +130,35 @@ the guarantor, and destroy the Entry. This is to avoid the costs of maintaining
129130
otherwise isn't particularly interesting anymore.
130131

131132
If there is a match, a more robust set of options is provided. `get` and `get_mut` provide access to the
132-
value found in the location. `set` behaves as the vacant variant, but also yields the old value. `take`
133-
simply removes the found value, and destroys the entry for similar reasons as `set`.
133+
value found in the location. `set` behaves as the vacant variant, but without destroying the entry.
134+
It also yields the old value. `take` simply removes the found value, and destroys the entry for similar reasons as `set`.
134135

135136
Let's look at how we one now writes `insert_or_update`:
136137

138+
There are two options. We can either do the following:
139+
140+
```
141+
// cleaner, and more flexible if logic is more complex
142+
let val = match map.entry(key) {
143+
Vacant(entry) => entry.set(0),
144+
Occupied(entry) => entry.into_mut(),
145+
};
146+
*val += 1;
147+
```
148+
149+
or
150+
137151
```
152+
// closer to the original, and more compact
138153
match map.entry(key) {
139-
Occupied(entry) => {
140-
let v = entry.get_mut();
141-
let new_v = *v + 1;
142-
*v = new_v;
143-
}
144-
Vacant(entry) => {
145-
entry.set(1);
146-
}
154+
Vacant(entry) => { entry.set(1); },
155+
Occupied(mut entry) => { *entry.get_mut() += 1; },
147156
}
148157
```
149158

150-
One can now write something equivalent to the "intuitive" inefficient code, but it is now as efficient as the complex
159+
Either way, one can now write something equivalent to the "intuitive" inefficient code, but it is now as efficient as the complex
151160
`insert_or_update` methods. In fact, this matches so closely to the inefficient manipulation
152-
that users could reasonable ignore Entries *until performance becomes an issue*. At which point
161+
that users could reasonable ignore Entries *until performance becomes an issue*, at which point
153162
it's an almost trivial migration. Closures also aren't needed to dance around the fact that one may
154163
want to avoid generating some values unless they have to, because that falls naturally out of
155164
normal control flow.
@@ -195,11 +204,5 @@ However it had some interesting ideas about Key manipulation, so we mention it h
195204
historical purposes.
196205

197206
# Unresolved questions
198-
The internal mutation methods cannot actually be implemented in terms of the View, because
199-
they return a mutable reference at the end, and there's no way to do that with the current
200-
View design. However, it's not clear why this is done by them. We believe it's simply to
201-
validate what the method *actually did*. If this is the case, then Views make this functionality
202-
obsolete. However, if this is *still* desirable, `set` could be tweaked to do this as well.
203-
However for some structures it may incur additional cost. Is this desirable functionality?
204207

205208
Naming bikesheds!

0 commit comments

Comments
 (0)