original key in RustcOccupiedEntry

[malezjaa]

This is required for rust-lang/rust#155360
Adds an original_key field to RustcOccupiedEntry

[cuviper]

Note that this used to hold the lookup key, but that was removed in #535.

See also rust-lang/rust#44286

[cuviper]

I gave some alternate suggestions on rust-lang/rust#155360

[Amanieu]

Could you instead change RustcEntry::Occupied to contain 2 values (the key and the OccupiedEntry)? This is much cleaner and avoids issues with other users of OccupiedEntry in the API.

[cuviper]

Tangentially -- I don't think we need any of these RustcEntry methods, because all std does is immediately deconstruct the variants into its own enum. Maybe we could even remove enum RustcEntry altogether and use a Result, like fn rustc_entry(..) -> Result<(Occupied, key), Vacant>.

[clarfonthey]

Just so we're on the same page, I was under the impression that RustcEntry and co. were different from the existing entries because the behaviour on std is different from the behaviour here?

One thing that had come to mind was just having multiple entry types based upon what functionality is desired, and having both be public API for this crate so we don't need to make this an internal module.

Not that this move should happen here, just, a potential justification for not just making methods return a key + entry and instead making it part of the entry, if it's going to be needed longer term for this.

[Amanieu]

The main reason RustcEntry exists is that the Entry type in the standard library (which dates back to Rust 1.0) is not generic over the hasher, which limits what can be done with it. Notably it means that that in std we have to always call reserve(1) before creating the entry since it is unable to resize the hash table by itself.

[Amanieu]

Could you instead change RustcEntry::Occupied to contain 2 values (the key and the OccupiedEntry)? This is much cleaner and avoids issues with other users of OccupiedEntry in the API.

I'm actually having second thoughts about this since it would mean yet another major version bump for hashbrown. @clarfonthey @cuviper what do you think?

[tgross35]

Isn't RustcEntry and similar API semver-exempt by being gated behind rustc-internal-api?

[cuviper]

Does cargo -Zbuild-std always respect the lock file? (edit: seems not) If there's any doubt about that, then I don't think we can just wave away semver issues without having std pinned to an exact version.

An alternative is to add a new method, so we avoid perturbing the performance of the normal entry path at all.

pub fn rustc_try_insert(&mut self, key: K, value: V) -> Result<&mut V, (RustcOccupiedEntry<'_, K, V, A>, K, V)>

Then the parts in the Err can be mapped to std's OccupiedError. We could wrap that in a RustcOccupiedError here if you really want.

[cuviper]

See #722 for rustc_try_insert -- I think this is a cleaner approach.