Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework the Entry API #535

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Rework the Entry API #535

merged 2 commits into from
Aug 22, 2024

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Jun 22, 2024

The main change in this PR is that OccupiedEntry no longer holds the
key used in the initial entry call. As a result, OccupiedEntryRef is
no longer required since OccupiedEntry can be used in EntryRef
directly.

The following methods have been removed:

// hash_map

impl OccupiedEntry {
    fn replace_entry(self, value: V) -> (K, V);
    fn replace_key(self) -> K;
}

impl EntryRef {
    fn and_replace_entry_with<F>(self, f: F) -> Self;
}

impl VacantEntryRef {
    fn into_key(self) -> K;
}

// hash_set

impl Entry {
    fn replace(self) -> T;
}

The following methods have been added:

impl VacantEntry {
    fn insert_entry(self, value: V) -> OccupiedEntry<'a, K, V, S, A>;
}

The following methods have their signatures changed:

impl EntryRef {
    // Previously returned OccupiedEntryRef
    fn insert(self, value: V) -> OccupiedEntry<'a, K, V, S, A>;
}

impl VacantEntryRef {
    // Previously returned &Q
    fn key(&self) -> &'b Q;

    // Previously returned OccupiedEntryRef
    fn insert(self, value: V) -> OccupiedEntry<'a, K, V, S, A>;
}

@bors
Copy link
Contributor

bors commented Aug 22, 2024

☔ The latest upstream changes (presumably #534) made this pull request unmergeable. Please resolve the merge conflicts.

The main change in this PR is that `OccupiedEntry` no longer holds the
key used in the initial `entry` call. As a result, `OccupiedEntryRef` is
no longer required since `OccupiedEntry` can be used in `EntryRef`
directly.

The following methods have been removed:
```rust
// hash_map

impl OccupiedEntry {
    fn replace_entry(self, value: V) -> (K, V);
    fn replace_key(self) -> K;
}

impl EntryRef {
    fn and_replace_entry_with<F>(self, f: F) -> Self;
}

impl VacantEntryRef {
    fn into_key(self) -> K;
}

// hash_set

impl Entry {
    fn replace(self) -> T;
}
```

The following methods have been added:
```rust
impl VacantENtry {
    fn insert_entry(self, value: V) -> OccupiedEntry<'a, K, V, S, A>;
}
```

The following methods have their signatures changed:
```rust
impl EntryRef {
    // Previously returned OccupiedEntryRef
    fn insert(self, value: V) -> OccupiedEntry<'a, K, V, S, A>;
}

impl VacantEntryRef {
    // Previously returned &Q
    fn key(&self) -> &'b Q;

    // Previously returned OccupiedEntryRef
    fn insert(self, value: V) -> OccupiedEntry<'a, K, V, S, A>;
}
```
This mirrors the changes done in the main hashbrown API.
@Amanieu
Copy link
Member Author

Amanieu commented Aug 22, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Aug 22, 2024

📌 Commit 731dfcc has been approved by Amanieu

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 22, 2024

⌛ Testing commit 731dfcc with merge f1ba3b4...

@bors
Copy link
Contributor

bors commented Aug 22, 2024

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing f1ba3b4 to master...

@bors bors merged commit f1ba3b4 into rust-lang:master Aug 22, 2024
24 checks passed
@jwiesler
Copy link

jwiesler commented Oct 2, 2024

How would I keep the key after this change if the key already existed in the map? entry and try_insert both take the key by value but don't return it if the key was already in the map, so I used replace_key which does not exist any more.

@Amanieu
Copy link
Member Author

Amanieu commented Oct 2, 2024

Have you tried using the entry_ref API?

@jwiesler
Copy link

jwiesler commented Oct 2, 2024

Ah, interesting, didn't know about this one thanks. Is there anything I can read up on why it was done this way? The two APIs seem very similar to me.

@Amanieu
Copy link
Member Author

Amanieu commented Oct 2, 2024

It's a separate API for cases where you want to move the key into the map or make a copy of an existing key without moving it.

@clarfonthey
Copy link
Contributor

clarfonthey commented Oct 2, 2024

Currently working on updating hashbrown for the standard library, and does the removal of the replace_entry API mean that there was the decision to remove it on the Rust side as well? Or was it a mistake to remove it from the RustcOccupiedEntry?

EDIT: I see, the FCP did decide to close, so, I'll remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants