Add Entry API

[apasel422]

No description provided.

[apasel422]

CC @gankro

I'm trying to determine the behavior of OccupiedEntry::insert for LinkedHashMap. When a key is inserted into the map using LinkedHashMap::insert, the node's position in the linked list is updated regardless of whether the key already existed. However, in the case of the entry API, we can't maintain this behavior in OccupiedEntry::{get_mut, into_mut}.

There are two options, then:

1. None of OccupiedEntry::{get_mut, into_mut, insert} affect the node's position.
2. OccupiedEntry::{get_mut, into_mut} do not affect the node's position, while OccupiedEntry::insert does.

I prefer the second option, as it makes the behavior consistent with VacantEntry::insert.

[Gankra]

Sounds good. However why can't the behaviour be maintained? The entry has unique access to the map.

[apasel422]

It's possible to get an OccupiedEntry but never call insert on it, instead doing:

*e.get_mut() = value; // or *e.into_mut() = value;

In that case, the node's position in the list isn't updated, even though calling LinkedHashMap::insert with the same key would update it (as would OccupiedEntry::insert, presumably). To get around this, we could make LinkedHashMap::entry update the node's position in the list before returning the Entry, but that seems inconsistent with that method's key-replacement behavior. (That is, equivalent keys are not replaced.)

[Gankra]

I'm saying get_mut can do the necessary mutation, no?

[apasel422]

It could, but then it always would. In other words, would you expect this to change the node's position, even though neither the key nor the value is changed?

if let Entry::Occupied(mut e) = map.entry(key) {
    e.get_mut();
}

[Gankra]

Seems isomorphic to map.get_mut(); which does update the position, no?

[Gankra]

Oh wait, maybe I'm thinking too much about LRU cache.

[apasel422]

Yeah, that only applies to LruCache.

[Gankra]

Entry::get_mut/into_mut should have isomorphic behaviour to Map::get_mut. Generally OccupiedEntry ops are "just" updates so it seems reasonable to not fiddle with the order.

[apasel422]

Agreed. That's what I was suggesting with option 2 above.

[apasel422]

I will point out that while OccupiedEntry is straightforward to implement for this map, VacantEntry is less so, due to the underlying map having KeyRef<K> as its key type.

[zonyitoo]

Hey guys, what's the status of this issue? When would this .entry API be added into LinkedHashMap?

[dtolnay]

We would be interested in using this for serde_json: serde-rs/json#236.