From c3051599fbac11f96effb81a97f701e65139c609 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Sun, 6 Mar 2016 12:37:33 -0600 Subject: [PATCH] Keyless Entry --- text/0000-keyless-entry.md | 150 +++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 text/0000-keyless-entry.md diff --git a/text/0000-keyless-entry.md b/text/0000-keyless-entry.md new file mode 100644 index 00000000000..3779151c556 --- /dev/null +++ b/text/0000-keyless-entry.md @@ -0,0 +1,150 @@ +- Feature Name: keyless_entry +- Start Date: 2016-03-06 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Add a variation to the entry API for maps that doesn't store the search key in the `Entry`. + +# Motivation +[motivation]: #motivation + +The `Entry` API for maps allows users to save time by allowing the user to perform arbitrary modifications +at a given key dependent upon whether that key was present and if it was, what value was previously there. +However, although insertion is the only action the user might take that requires a by-value key, there is +no way to create an `Entry` without a fully owned key. When the user only has a by-reference key on hand, +this is inefficient at best, requiring an unnecessary `.to_owned` that may involve an expensive allocation +and/or copy, and unusable at worst, if the key cannot be cloned. + +It is a testament to the flexibility and control afforded by the `Entry` API that nearly every method on +a generic map can be conceptually implemented in terms of `entry`. However, this trick is not used +and cannot be used in practice because of the same issue - since, e.g., `remove` takes a by-reference key, +it cannot call `entry` and therefore cannot completely share its implementation with `entry`. To still recover +as much sharing as possible, map implementations seem to have converged on using an internal variation on the +`Entry` API, only differing in that the key itself is not stored with the `Entry`. For example, `HashMap` has +its `FullBucket`s and `VacantEntryState`s, and after rust-lang/rust#32058 will have an `InternalEntry` type, +while `BTreeMap` has its `Handle`s, and before the rewrite had `SearchStack`s. Making these modified entries +public gives the user exactly the control they need. + +# Detailed design +[design]: #detailed-design + +## API + +Two new types are introduced on each map. `VacantKeylessEntry<'a, K, V>` is kept abstract, while +`KeylessEntry<'a, K, V>` is defined as follows: + +``` +pub enum KeylessEntry<'a, K, V> { + Occupied(OccupiedEntry<'a, K, V>), + Vacant(VacantKeylessEntry<'a, K, V>) +} +``` + +To obtain a `KeylessEntry`, a new method is added to the maps: + +``` +impl Map { + /// Gets the given key's corresponding entry in the map for in-place manipulation. + pub fn keyless_entry<'a, Q: ?Sized>(&'a mut self, key: &Q) -> KeylessEntry<'a, K, V> where K: Borrow; +} +``` + +with the appropriate `Ord` or `Eq + Hash` bound on `Q`. + +The methods on `KeylessEntry`s mirror those on `Entry`s, only differing in that they take keys when doing +insertion: + +``` +impl<'a, K, V> KeylessEntry<'a, K, V> { + /// Ensures a value is in the entry by inserting the default if empty, and returns + /// a mutable reference to the value in the entry. + pub fn or_insert(self, default_key: K, default: V) -> &'a mut V { + match self { + Occupied(entry) => entry.into_mut(), + Vacant(entry) => entry.insert(default), + } + } + + /// Ensures a value is in the entry by inserting the result of the default function if empty, + /// and returns a mutable reference to the value in the entry. + pub fn or_insert_with (K, V)>(self, default: F) -> &'a mut V { + match self { + Occupied(entry) => entry.into_mut(), + Vacant(entry) => { + let (key, value) = default(); + entry.insert(key, value) + }, + } + } +} + +impl<'a, K: 'a, V: 'a> VacantKeylessEntry<'a, K, V> { + /// Sets the value of the entry with the VacantKeylessEntry's key, + /// and returns a mutable reference to it + pub fn insert(self, key: K, value: V) -> &'a mut V; +} +``` + +`KeylessEntry` and `VacantKeylessEntry` also have methods for converting them into more traditional entries: + +``` +impl<'a, K, V> KeylessEntry<'a, K, V> { + /// Associates a key with the `KeylessEntry`, making it no longer keyless and just an `Entry`. + pub fn with_key(self, key: K) -> Entry<'a, K, V> { + match self { + Occupied(entry) => Entry::Occupied(entry), + Vacant(entry) => Entry::Vacant(entry.with_key(key)), + } + } +} + +impl<'a, K: 'a, V: 'a> VacantKeylessEntry<'a, K, V> { + /// Associates a key with the `VacantKeylessEntry`, making it no longer keyless and just + /// a `VacantEntry`. + pub fn with_key(self, key: K) -> VacantEntry<'a, K, V>; +} +``` + +## Implementation + +As mentioned in the motivation section of this RFC, the implementations of `BTreeMap` and `HashMap` already +align quite well with this API. `BTreeMap`'s `VacantKeylessEntry` can be implemented simply as a wrapper around +a `Handle, K, V, marker::Leaf>, marker::Edge>`, while `HashMap`'s `VacantKeylessEntry` +can be a pair of a `SafeHash` and a `VacantEntryState`. + +# Drawbacks +[drawbacks]: #drawbacks + +Having the user plug the key in twice when doing an insertion makes it more easy to shoot oneself in the foot +by putting in two unequal keys. Although this can mess up the internal structure of the map, it cannot cause +memory unsafety, as this possibility was already allowed by having a query where `key` and `key.to_owned()` +are unequal. + +# Alternatives +[alternatives]: #alternatives + +- Replace the `Entry` API entirely. This would get rid of the inherent duplication of having two `Entry` APIs, + but would be backwards incompatible. +- Create an `OccupiedKeylessEntry` that is separate from `OccupiedEntry` to help keep to two different `Entry`s + straight. This might reduce confusion and wouldn't lock maps into dropping search keys immediately upon finding + a match, but would add another type with identical behavior, which would be arguably even more confusing. +- Publicly expose an implementation of `VacantEntry` as a struct containing a key and a `VacantKeylessEntry`. This + would help clarify the relationship between the two types and match more with the reuse of `OccupiedEntry` in + both the keyed and keyless cases. However, it can be done backwards compatibly later and doesn't have any + particularly compelling use cases. +- Make `Entry` hold a `Cow` and add an `entry_lazy` method for creating `Entry`s that don't own their keys, as + (suggested)[https://internals.rust-lang.org/t/head-desking-on-entry-api-4-0/2156] by @Gankro. While this might + work, it runs into issues of changing the signature of `Entry`, introducing a `Clone` bound on the non-lazy + case even though the key will never be cloned. Additionally, it doesn't support some more exotic use cases in + which the keys cannot be duplicated at all. +- Add an `entry_or_clone` method as suggested in rust-lang/rfcs#1203 that clones its key if the result is a + `VacantEntry`. This is an improvement, but doesn't handle all use cases and still is needlessly inefficient, as + the user might still not wish to do an insertion after finding a vacant slot. + +# Unresolved questions +[unresolved]: #unresolved-questions + +None