implement entry API for HashMap #17378
Conversation
| v as *mut _ | ||
| }; | ||
| robin_hood(bucket, ib, self.hash, self.key, value); | ||
| unsafe { &mut *val_ptr } |
There was a problem hiding this comment.
Unfortunately, this would become the only instance of unsafe in this module, even though it's correct. I think robin_hood already returns the value we need.
There was a problem hiding this comment.
Oh wow, I completely missed that. Perfect!
Gankra
force-pushed
the
hashmap-entry
branch
3 times, most recently
from
5eca516
to
6c255a8
Compare
|
This should be good-to-go now. |
|
f? @pczarn Can you do an initial review of this PR? Given your recent revamp of |
| idx_push(&mut *table.table.borrow_mut(), Mark(m, ctxt)); | ||
|
|
||
| *table.mark_memo.borrow_mut().find_or_insert_with(key, new_ctxt) | ||
| * match table.mark_memo.borrow_mut().entry(key) { |
There was a problem hiding this comment.
Yeah... I started going for literal translations of the old code, and dereffing matches was one of those weird consequences. You would make a temporary variable to deref, then?
|
I have already done a review. I went through the impl once more. The naming is not ideal, in particular |
|
@aturon Does that new commit resolve your issues? If so, I'll rebase. |
|
@gankro Yes, that's excellent! Please squash/rebase, and then I'll r+ |
|
Done. |
Gankra
force-pushed
the
hashmap-entry
branch
from
1f63542
to
c2e147c
Compare
Gankra
force-pushed
the
hashmap-entry
branch
from
c2e147c
to
2f723a8
Compare
|
@gankro weird about the deprecation failure. Were you doing |
|
Ah, I ran make check-collections, but hashmap is in std. |
Deprecates the `find_or_*` family of "internal mutation" methods on `HashMap` in favour of the "external mutation" Entry API as part of RFC 60. Part of rust-lang#17320, although this still needs to be done on the rest of the maps, they don't have any internal mutation methods defined, so they can be done without deprecating or breaking anything. Work on `BTree`'s is part of the complete rewrite in rust-lang#17334. The implemented API deviates from the API described in the RFC in two key places: * `VacantEntry.set` yields a mutable reference to the inserted element to avoid code duplication where complex logic needs to be done *regardless* of whether the entry was vacant or not. * `OccupiedEntry.into_mut` was added so that it is possible to return a reference into the map beyond the lifetime of the Entry itself, providing functional parity to `VacantEntry.set`. This allows the full find_or_insert functionality to be implemented using this API. A PR will be submitted to the RFC to amend this. [breaking-change]
Gankra
force-pushed
the
hashmap-entry
branch
from
2f723a8
to
fe8a413
Compare
|
sigh TIL that check-std doesn't check the doccomment code examples... and that deprecation is checked there too. @aturon This time FOR REALS THOUGH. (please bors, please) |
Deprecates the `find_or_*` family of "internal mutation" methods on `HashMap` in favour of the "external mutation" Entry API as part of RFC 60. Part of #17320, but this still needs to be done on the rest of the maps. However they don't have any internal mutation methods defined, so they can be done without deprecating or breaking anything. Work on `BTree` is part of the complete rewrite in #17334. The implemented API deviates from the API described in the RFC in two key places: * `VacantEntry.set` yields a mutable reference to the inserted element to avoid code duplication where complex logic needs to be done *regardless* of whether the entry was vacant or not. * `OccupiedEntry.into_mut` was added so that it is possible to return a reference into the map beyond the lifetime of the Entry itself, providing functional parity to `VacantEntry.set`. This allows the full find_or_insert functionality to be implemented using this API. A PR will be submitted to the RFC to amend this. [breaking-change]
fix: ensure that the parent of a SourceRoot cannot be itself fix rust-lang#17378. In `FileSetConfig.map`, different roots might be mapped to the same `root_id` due to deduplication in `ProjectFolders::new`: ```rust // Example from rustup /Users/roife/code/rustup/target/debug/build/rustup-863a063426b56c51/out /Users/roife/code/rustup ``` In `source_root_parent_map`, r-a might encounter paths where their SourceRootId (i.e. `root_id`) is identical, yet one the them is the parent of the another. This situation can cause the `root_id` to be its own parent, potentially leading to an infinite loop. This PR resolves such cases by adding a check.
Deprecates the
find_or_*family of "internal mutation" methods onHashMapinfavour of the "external mutation" Entry API as part of RFC 60. Part of #17320,
but this still needs to be done on the rest of the maps. However they don't have
any internal mutation methods defined, so they can be done without deprecating
or breaking anything. Work on
BTreeis part of the complete rewrite in #17334.The implemented API deviates from the API described in the RFC in two key places:
VacantEntry.setyields a mutable reference to the inserted element to avoid codeduplication where complex logic needs to be done regardless of whether the entry
was vacant or not.
OccupiedEntry.into_mutwas added so that it is possible to return a referenceinto the map beyond the lifetime of the Entry itself, providing functional parity
to
VacantEntry.set.This allows the full find_or_insert functionality to be implemented using this API.
A PR will be submitted to the RFC to amend this.
[breaking-change]