Add {BTreeMap,HashMap}::try_insert #82764
Conversation
m-ou-se
added
A-collections
rust-highfive
added
the
S-waiting-on-review
| /// 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, |
There was a problem hiding this comment.
I made these fields public instead of adding accessors. This is somewhat uncommon in std, but we do the same for std::ops::Range.
Accessor functions here would get somewhat messy, because taking ownership of one or both of the fields should be possible. The alternative is .entry(), .value(), .into_entry(), .into_value() and .into_entry_and_value() -> (OccupiedEntry, V). This can get confusing, as .entry().key() would work, but .entry().remove() would need .into_entry().remove() instead, which would also consume the value.
| @@ -470,6 +470,24 @@ impl Error for char::DecodeUtf16Error { | |||
| } | |||
| } | |||
|
|
|||
| #[unstable(feature = "map_try_insert", issue = "none")] | |||
| impl<'a, K: Debug + Ord, V: Debug> Error | |||
There was a problem hiding this comment.
I added Error (and Display) implementations for OccupiedError for completeness, since it's an error. However, since it has non-static lifetimes, the Error implementation is not very useful in most cases.
This comment has been minimized.
This comment has been minimized.
|
@bors r+ |
|
📌 Commit eddd4f0 has been approved by |
bors
added
S-waiting-on-bors
Add {BTreeMap,HashMap}::try_insert
`{BTreeMap,HashMap}::insert(key, new_val)` returns `Some(old_val)` if the key was already in the map. It's often useful to assert no duplicate values are inserted.
We experimented with `map.insert(key, val).unwrap_none()` (rust-lang#62633), but decided that that's not the kind of method we'd like to have on `Option`s.
`insert` always succeeds because it replaces the old value if it exists. One could argue that `insert()` is never the right method for panicking on duplicates, since already handles that case by replacing the value, only allowing you to panic after that already happened.
This PR adds a `try_insert` method that instead returns a `Result::Err` when the key already exists. This error contains both the `OccupiedEntry` and the value that was supposed to be inserted. This means that unwrapping that result gives more context:
```rust
map.insert(10, "world").unwrap_none();
// thread 'main' panicked at 'called `Option::unwrap_none()` on a `Some` value: "hello"', src/main.rs:8:29
```
```rust
map.try_insert(10, "world").unwrap();
// thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
// OccupiedError { key: 10, old_value: "hello", new_value: "world" }', src/main.rs:6:33
```
It also allows handling the failure in any other way, as you have full access to the `OccupiedEntry` and the value.
`try_insert` returns a reference to the value in case of success, making it an alternative to `.entry(key).or_insert(value)`.
r? `@Amanieu`
Fixes rust-lang/rfcs#3092
Add {BTreeMap,HashMap}::try_insert
`{BTreeMap,HashMap}::insert(key, new_val)` returns `Some(old_val)` if the key was already in the map. It's often useful to assert no duplicate values are inserted.
We experimented with `map.insert(key, val).unwrap_none()` (rust-lang#62633), but decided that that's not the kind of method we'd like to have on `Option`s.
`insert` always succeeds because it replaces the old value if it exists. One could argue that `insert()` is never the right method for panicking on duplicates, since already handles that case by replacing the value, only allowing you to panic after that already happened.
This PR adds a `try_insert` method that instead returns a `Result::Err` when the key already exists. This error contains both the `OccupiedEntry` and the value that was supposed to be inserted. This means that unwrapping that result gives more context:
```rust
map.insert(10, "world").unwrap_none();
// thread 'main' panicked at 'called `Option::unwrap_none()` on a `Some` value: "hello"', src/main.rs:8:29
```
```rust
map.try_insert(10, "world").unwrap();
// thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
// OccupiedError { key: 10, old_value: "hello", new_value: "world" }', src/main.rs:6:33
```
It also allows handling the failure in any other way, as you have full access to the `OccupiedEntry` and the value.
`try_insert` returns a reference to the value in case of success, making it an alternative to `.entry(key).or_insert(value)`.
r? ``@Amanieu``
Fixes rust-lang/rfcs#3092
Rollup of 10 pull requests Successful merges: - rust-lang#80723 (Implement NOOP_METHOD_CALL lint) - rust-lang#80763 (resolve: Reduce scope of `pub_use_of_private_extern_crate` deprecation lint) - rust-lang#81136 (Improved IO Bytes Size Hint) - rust-lang#81939 (Add suggestion `.collect()` for iterators in iterators) - rust-lang#82289 (Fix underflow in specialized ZipImpl::size_hint) - rust-lang#82728 (Avoid unnecessary Vec construction in BufReader) - rust-lang#82764 (Add {BTreeMap,HashMap}::try_insert) - rust-lang#82770 (Add assert_matches macro.) - rust-lang#82773 (Add diagnostic item to `Default` trait) - rust-lang#82787 (Remove unused code from main.js) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Add try_insert Ported from rust-lang/rust#82764.
Add try_insert Ported from rust-lang/rust#82764.
|
I think we could also have a |
|
I know this is easier but I was hoping to see the |
The motivation for this PR appears to be the ergonomics of error handling when you're assuming there will never be duplicate insertions, i.e. a duplicate insertion indicates there's a bug and you need to panic, log an error message or similar. If you're not sure whether the key is in the map and want to avoid the cost of generating the value unless it's actually needed, you should probably use the entry API directly, e.g:
|
{BTreeMap,HashMap}::insert(key, new_val)returnsSome(old_val)if the key was already in the map. It's often useful to assert no duplicate values are inserted.We experimented with
map.insert(key, val).unwrap_none()(#62633), but decided that that's not the kind of method we'd like to have onOptions.insertalways succeeds because it replaces the old value if it exists. One could argue thatinsert()is never the right method for panicking on duplicates, since already handles that case by replacing the value, only allowing you to panic after that already happened.This PR adds a
try_insertmethod that instead returns aResult::Errwhen the key already exists. This error contains both theOccupiedEntryand the value that was supposed to be inserted. This means that unwrapping that result gives more context:It also allows handling the failure in any other way, as you have full access to the
OccupiedEntryand the value.try_insertreturns a reference to the value in case of success, making it an alternative to.entry(key).or_insert(value).r? @Amanieu
Fixes rust-lang/rfcs#3092