From 09cbcdc2c31325ec67047c5b9ce87dee03af62dc Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:34:47 +0100 Subject: [PATCH 1/7] Add BTreeMap::try_insert and btree_map::OccupiedError. --- library/alloc/src/collections/btree/map.rs | 36 ++++++++++++++++++- .../alloc/src/collections/btree/map/entry.rs | 21 +++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 783f88f026b8f..12a7322d8e7ee 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -14,7 +14,7 @@ use super::node::{self, marker, ForceResult::*, Handle, NodeRef, Root}; use super::search::SearchResult::*; mod entry; -pub use entry::{Entry, OccupiedEntry, VacantEntry}; +pub use entry::{Entry, OccupiedEntry, OccupiedError, VacantEntry}; use Entry::*; /// Minimum number of elements in nodes that are not a root. @@ -836,6 +836,40 @@ impl BTreeMap { } } + /// Tries to insert a key-value pair into the map, and returns + /// a mutable reference to the value in the entry. + /// + /// If the map already had this key present, nothing is updated, and + /// an error containing the occupied entry and the value is returned. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// #![feature(map_try_insert)] + /// + /// use std::collections::BTreeMap; + /// + /// let mut map = BTreeMap::new(); + /// assert_eq!(map.try_insert(37, "a").unwrap(), &"a"); + /// + /// let err = map.try_insert(37, "b").unwrap_err(); + /// assert_eq!(err.entry.key(), &37); + /// assert_eq!(err.entry.get(), &"a"); + /// assert_eq!(err.value, "b"); + /// ``` + #[unstable(feature = "map_try_insert", issue = "none")] + pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> + where + K: Ord, + { + match self.entry(key) { + Occupied(entry) => Err(OccupiedError { entry, value }), + Vacant(entry) => Ok(entry.insert(value)), + } + } + /// Removes a key from the map, returning the value at the key if the key /// was previously in the map. /// diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index 941f82a8070a0..bd7114f8a82b7 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -71,6 +71,27 @@ impl Debug for OccupiedEntry<'_, K, V> { } } +/// The error returned by [`try_insert`](BTreeMap::try_insert) when the key already exists. +/// +/// Contains the occupied entry, and the value that was not inserted. +#[unstable(feature = "map_try_insert", issue = "none")] +pub struct OccupiedError<'a, K: 'a, V: 'a> { + /// The entry in the map that was already occupied. + pub entry: OccupiedEntry<'a, K, V>, + /// The value which was not inserted, because the entry was already occupied. + pub value: V, +} + +#[unstable(feature = "map_try_insert", issue = "none")] +impl Debug for OccupiedError<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("OccupiedError") + .field("entry", &self.entry) + .field("value", &self.value) + .finish() + } +} + impl<'a, K: Ord, V> Entry<'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. From f6fe24aab36814ee31ad9dd46fbefe1017670ece Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:34:47 +0100 Subject: [PATCH 2/7] Add HashMap::try_insert and hash_map::OccupiedError. --- library/std/src/collections/hash/map.rs | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 27f7191831d41..4e6aee98647ea 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -842,6 +842,40 @@ where self.base.insert(k, v) } + /// Tries to insert a key-value pair into the map, and returns + /// a mutable reference to the value in the entry. + /// + /// If the map already had this key present, nothing is updated, and + /// an error containing the occupied entry and the value is returned. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// #![feature(map_try_insert)] + /// + /// use std::collections::HashMap; + /// + /// let mut map = HashMap::new(); + /// assert_eq!(map.try_insert(37, "a").unwrap(), &"a"); + /// + /// let err = map.try_insert(37, "b").unwrap_err(); + /// assert_eq!(err.entry.key(), &37); + /// assert_eq!(err.entry.get(), &"a"); + /// assert_eq!(err.value, "b"); + /// ``` + #[unstable(feature = "map_try_insert", issue = "none")] + pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> + where + K: Ord, + { + match self.entry(key) { + Occupied(entry) => Err(OccupiedError { entry, value }), + Vacant(entry) => Ok(entry.insert(value)), + } + } + /// Removes a key from the map, returning the value at the key if the key /// was previously in the map. /// @@ -1851,6 +1885,18 @@ impl Debug for VacantEntry<'_, K, V> { } } +/// The error returned by [`try_insert`](HashMap::try_insert) when the key already exists. +/// +/// Contains the occupied entry, and the value that was not inserted. +#[unstable(feature = "map_try_insert", issue = "none")] +#[derive(Debug)] +pub struct OccupiedError<'a, K: 'a, V: 'a> { + /// The entry in the map that was already occupied. + pub entry: OccupiedEntry<'a, K, V>, + /// The value which was not inserted, because the entry was already occupied. + pub value: V, +} + #[stable(feature = "rust1", since = "1.0.0")] impl<'a, K, V, S> IntoIterator for &'a HashMap { type Item = (&'a K, &'a V); From 69d95e232af0fe81de85e1e4a1f8dc73d7b0f16c Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:56:38 +0100 Subject: [PATCH 3/7] Improve Debug implementations of OccupiedError. --- library/alloc/src/collections/btree/map/entry.rs | 5 +++-- library/std/src/collections/hash/map.rs | 12 +++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index bd7114f8a82b7..cc508f30a614a 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -86,8 +86,9 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { impl Debug for OccupiedError<'_, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OccupiedError") - .field("entry", &self.entry) - .field("value", &self.value) + .field("key", self.entry.key()) + .field("old_value", self.entry.get()) + .field("new_value", &self.value) .finish() } } diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 4e6aee98647ea..7a50b49007cb0 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -1889,7 +1889,6 @@ impl Debug for VacantEntry<'_, K, V> { /// /// Contains the occupied entry, and the value that was not inserted. #[unstable(feature = "map_try_insert", issue = "none")] -#[derive(Debug)] pub struct OccupiedError<'a, K: 'a, V: 'a> { /// The entry in the map that was already occupied. pub entry: OccupiedEntry<'a, K, V>, @@ -1897,6 +1896,17 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { pub value: V, } +#[unstable(feature = "map_try_insert", issue = "none")] +impl Debug for OccupiedError<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("OccupiedError") + .field("key", self.entry.key()) + .field("old_value", self.entry.get()) + .field("new_value", &self.value) + .finish() + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl<'a, K, V, S> IntoIterator for &'a HashMap { type Item = (&'a K, &'a V); From d85d82ab2234cf08e2e86575d0cdebefdb819831 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:57:26 +0100 Subject: [PATCH 4/7] Implement Error for OccupiedError. --- .../alloc/src/collections/btree/map/entry.rs | 13 +++++++++++++ library/std/src/collections/hash/map.rs | 13 +++++++++++++ library/std/src/error.rs | 18 ++++++++++++++++++ library/std/src/lib.rs | 1 + 4 files changed, 45 insertions(+) diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index cc508f30a614a..a5ece31d67b62 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -93,6 +93,19 @@ impl Debug for OccupiedError<'_, K, V> { } } +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug + Ord, V: Debug> fmt::Display for OccupiedError<'a, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "failed to insert {:?}, key {:?} already exists with value {:?}", + self.value, + self.entry.key(), + self.entry.get(), + ) + } +} + impl<'a, K: Ord, V> Entry<'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. diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 7a50b49007cb0..a4a7b2566547b 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -1907,6 +1907,19 @@ impl Debug for OccupiedError<'_, K, V> { } } +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug, V: Debug> fmt::Display for OccupiedError<'a, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "failed to insert {:?}, key {:?} already exists with value {:?}", + self.value, + self.entry.key(), + self.entry.get(), + ) + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl<'a, K, V, S> IntoIterator for &'a HashMap { type Item = (&'a K, &'a V); diff --git a/library/std/src/error.rs b/library/std/src/error.rs index 94338c7b04d06..a7f744ce51533 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -470,6 +470,24 @@ impl Error for char::DecodeUtf16Error { } } +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug + Ord, V: Debug> Error + for crate::collections::btree_map::OccupiedError<'a, K, V> +{ + #[allow(deprecated)] + fn description(&self) -> &str { + "key already exists" + } +} + +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug, V: Debug> Error for crate::collections::hash_map::OccupiedError<'a, K, V> { + #[allow(deprecated)] + fn description(&self) -> &str { + "key already exists" + } +} + #[stable(feature = "box_error", since = "1.8.0")] impl Error for Box { #[allow(deprecated, deprecated_in_future)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ba49dee38e642..7e86311bbb00c 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -281,6 +281,7 @@ #![feature(linkage)] #![feature(llvm_asm)] #![feature(log_syntax)] +#![feature(map_try_insert)] #![feature(maybe_uninit_extra)] #![feature(maybe_uninit_ref)] #![feature(maybe_uninit_slice)] From da01455813f8887d7c709f8c37bff9dcbbce34c3 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 16:25:24 +0100 Subject: [PATCH 5/7] Ignore file length tidy warning in hash/map.rs. --- library/std/src/collections/hash/map.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index a4a7b2566547b..24333fb5724cb 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -1,3 +1,5 @@ +// ignore-tidy-filelength + #[cfg(test)] mod tests; From 1aedb4c3a38b099a127c3fd3815400b3d0098475 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 16:46:41 +0100 Subject: [PATCH 6/7] Remove unnecessary bound from HashMap::try_insert. --- library/std/src/collections/hash/map.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 24333fb5724cb..cd6787cf58812 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -868,10 +868,7 @@ where /// assert_eq!(err.value, "b"); /// ``` #[unstable(feature = "map_try_insert", issue = "none")] - pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> - where - K: Ord, - { + pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> { match self.entry(key) { Occupied(entry) => Err(OccupiedError { entry, value }), Vacant(entry) => Ok(entry.insert(value)), From eddd4f05012ed9ebefa6e2e5ca91da4116d30996 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 16:54:28 +0100 Subject: [PATCH 7/7] Add tracking issue for map_try_insert. --- library/alloc/src/collections/btree/map.rs | 2 +- library/alloc/src/collections/btree/map/entry.rs | 6 +++--- library/std/src/collections/hash/map.rs | 8 ++++---- library/std/src/error.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 12a7322d8e7ee..622983996aa08 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -859,7 +859,7 @@ impl BTreeMap { /// assert_eq!(err.entry.get(), &"a"); /// assert_eq!(err.value, "b"); /// ``` - #[unstable(feature = "map_try_insert", issue = "none")] + #[unstable(feature = "map_try_insert", issue = "82766")] pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> where K: Ord, diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index a5ece31d67b62..6b30d95977395 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -74,7 +74,7 @@ impl Debug for OccupiedEntry<'_, K, V> { /// The error returned by [`try_insert`](BTreeMap::try_insert) when the key already exists. /// /// Contains the occupied entry, and the value that was not inserted. -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] pub struct OccupiedError<'a, K: 'a, V: 'a> { /// The entry in the map that was already occupied. pub entry: OccupiedEntry<'a, K, V>, @@ -82,7 +82,7 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { pub value: V, } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl Debug for OccupiedError<'_, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OccupiedError") @@ -93,7 +93,7 @@ impl Debug for OccupiedError<'_, K, V> { } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug + Ord, V: Debug> fmt::Display for OccupiedError<'a, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index cd6787cf58812..233afa9238999 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -867,7 +867,7 @@ where /// assert_eq!(err.entry.get(), &"a"); /// assert_eq!(err.value, "b"); /// ``` - #[unstable(feature = "map_try_insert", issue = "none")] + #[unstable(feature = "map_try_insert", issue = "82766")] pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> { match self.entry(key) { Occupied(entry) => Err(OccupiedError { entry, value }), @@ -1887,7 +1887,7 @@ impl Debug for VacantEntry<'_, K, V> { /// The error returned by [`try_insert`](HashMap::try_insert) when the key already exists. /// /// Contains the occupied entry, and the value that was not inserted. -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] pub struct OccupiedError<'a, K: 'a, V: 'a> { /// The entry in the map that was already occupied. pub entry: OccupiedEntry<'a, K, V>, @@ -1895,7 +1895,7 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { pub value: V, } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl Debug for OccupiedError<'_, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OccupiedError") @@ -1906,7 +1906,7 @@ impl Debug for OccupiedError<'_, K, V> { } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug, V: Debug> fmt::Display for OccupiedError<'a, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( diff --git a/library/std/src/error.rs b/library/std/src/error.rs index a7f744ce51533..80c35307d52ac 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -470,7 +470,7 @@ impl Error for char::DecodeUtf16Error { } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug + Ord, V: Debug> Error for crate::collections::btree_map::OccupiedError<'a, K, V> { @@ -480,7 +480,7 @@ impl<'a, K: Debug + Ord, V: Debug> Error } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug, V: Debug> Error for crate::collections::hash_map::OccupiedError<'a, K, V> { #[allow(deprecated)] fn description(&self) -> &str {