Is there a reason this is not a thing?
Yes, should HashMap insert if the key doesn't exist in these two cases?
&mut map[key]map[key] = value
If you answered differently for the two cases, then you can't implement IndexMut for HashMap. We would need a IndexAssign or similar.
1 Like
Well if you allow overloading assignment then you can return an EntryMut or similar and let assignment do the insertion /s
Yes, but we don't have overloaded assign, and I don't think that we will ever get that.
2 Likes
(Thank Goodness.)
1 Like
On an unrelated note, I've seen this come up at least twice from different people in different threads; maybe this should be considered a FAQ and accordingly, addressed in the documentation of HashMap?
2 Likes
I don't think it will be missing forever. We just need to resolve the question of IndexAssign, since it's assumed that IndexMut and IndexAssign will conflict conceptually and syntactically if they both exist.
1 Like
I'd actually really really like this when V: Default. Being able to write my_map[foo].push(..) like in C++ would be so so much better than my_map.entry(foo).or_insert(Vec::new()).push(..).
++++ operator= is cursed and a mistake.
2 Likes
@mcy I'd prefer for my_map[foo] to panic if there's no key in the map, as it helps to find out bugs. I like that inserting a new key in the map is explicit, as, most of the time, a relatively small fraction of code needs to insert values in the map, while the rest of the code benefits from runtime checks that lookup keys are reasonable.
5 Likes
There was a talk at cppcon 2017 about recurring bugs in Facebook's c++ codebase, and one of the first thing that came up was that map[key].mutate() inserts if key doesn't exist. So, there is some precedent, even in C++, that insertion is not the best way to go when you are trying to mutate an element. They even recommend using map.at(key).mutate() which throws instead of inserting to get around this.
One this note, I think we could add a default method to IndexMut,
trait IndexMut<I> {
// ...
fn index_assign(&mut self, index: I, value: Self::Output) where Self::Output: Sized {
*self.index_mut(index) = value;
}
}
Then we could always parse foo[key] = value as foo.index_assign(index, value) and maintain backwards compatibility.
This would allow HashMap to insert on no key if you are assigning, but panic if you are not. We don't need to worry about += etc. because they need the old value, so they must continue to use index_mut.
4 Likes
One thing that came up in RFC discussions about this that made it clear to me that it is not easy to get the syntax right is that:
- When modifying a key-value pair in place, you want to take a reference-to-key
- When inserting a new key-value pair, you want to take ownership of the key.
The following syntax could be either of 1 and 2.
map[k] = value;
It would be problematic if this syntax changed meaning (from overwrite-only to insert) only based on the type of k (is it reference-to-key or key?).
2 Likes
map.entry(...).or_default().push(...) makes this pattern less verbose.
2 Likes
My original use case was mostly replacing; I find the idiom of using insert() to do what's actually more like "set" or "update" or "replace" not great for understanding what's happening.
In any case, my original answer of why the impl doesn't exist yet is definitely answered. 
1 Like
I have no idea what sort of C++ Facebook writes, but of all of the common, recurring bugs in C++, this is the first time I've ever heard of operator[] calling a default ctor being a problem, and, again, being able to write my_map_[foo].emplace_back(...) is considered much preferred to doing the silly dance with find (unless you need heterogenous lookup but then the whole conversation is irrelevant).
I certainly never use (or see) std::vector::at, either, but maybe I'm spoiled by asan and msan.
I don't think it's much of an improvement. Maybe this has to do with how Rust doesn't really treat Default the way you treat the default ctor in C++. shrug
In early rust talks I remember this being brought up with regards to multithreading. As the operator[] mutates the map you'd need to lock it, or risk a data race. This doesn't really apply to rust, though it would make IndexMut and Index function differently.
HashMap has no obligation to be threadsafe; the compiler already ensures that it is not used in a manner that could cause data races. (If you need a threadsafe HashMap, you need to use an external locking mechanism like Mutex or RwLock)
@adamAndMath was talking about C++'s map not being threadsafe, he also mentioned that this wasn't an issue in Rust.
1 Like
I think this is the path that would lead to Rust having the same sneaky bug as C++'s map (mentioned in the video linked above), where it is possible to insert a key just by accessing a place where a key does not exist. Rust's superior mutability semantics aid this issues, but I think it would be better to never introduce it in the first place.
3 Likes
If we design a IndexAssign or extension to IndexMut (like one that I proposed earlier), then this is not a concern since you can only insert a new value on assignment, not on access.
But then there's still a question of how IndexAssign will handle insertion versus update.
2 Likes