Repository navigation
Give the key of Table::entry() its own lifetime under experimental-api-5 - #1639
Merged
Merged
Conversation
entry() tied the key to the lifetime of the table borrow, so the guard an entry returned could not outlive the key. In generic code, where K::SelfType<'k> is invariant, the table stayed mutably borrowed for the key's whole lifetime, and entry() could not be called twice with keys of one lifetime. Under experimental-api-5, Entry, OccupiedEntry and VacantEntry take a second lifetime for the key. Without the feature the 4.x API is unchanged: OccupiedEntry and VacantEntry are aliases with one lifetime, and Entry keeps its 4.x definition. Assisted-by: Claude Code Claude-Session: https://claude.ai/code/session_01ALhxet6NP69dCfAUv8TAhR
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Under
experimental-api-5,Table::entry()gives the key its own lifetime:Entry,OccupiedEntryandVacantEntryget the matching'kparameter.key(),into_key(),remove_entry()andor_insert_with_key()hand the key back with that lifetime.Why: today the key shares the table borrow's lifetime (
entry<'a>(&'a mut self, key: K::SelfType<'a>)), which causes two problems:table.entry(key.as_str())?.or_insert(..)?from a block where the keyStringlives fails with E0597.K::SelfType<'k>is invariant in'k, so the table must stay mutably borrowed for the key's entire lifetime. Callingentry()in a loop over keys of one lifetime fails, and so does calling it with a key whose lifetime isn't tied to the table borrow.The key stays by value rather than
impl Borrow:Entryhas to own it, and a concrete parameter type keeps coercions working, such asentry(&string)on a&strtable orentry(&vec)on a&[u8]table. A'k: 'abound can't replace the extra lifetime, because the compiler can't shortenK::SelfType<'k>toK::SelfType<'a>.Gating, so that the 4.x API is unchanged:
OccupiedEntryandVacantEntrytake'kin both modes. Without the feature,lib.rsexports one-lifetime aliasesOccupiedEntry<'a, K, V> = table::OccupiedEntry<'a, 'a, K, V>. That is fully compatible because their fields are private.use redb::Entry::{Occupied, Vacant}, so without the feature the oldEntryenum and its impl stay as a verbatim copy inmod entry_api4. Only two doc-link lines differ.Table::entryhas both signatures, each behind acfg.At flag removal, delete
entry_api4, the two aliases and the oldentry().semver-checks note:
cargo semver-checks --default-featuresreportsstruct_missingforOccupiedEntryandVacantEntry, because it doesn't treat a type alias as equivalent to a struct. I checked the uses that matter for structs with private fields: import by path, user trait impls,{ .. }patterns, type-relative calls such asOccupiedEntry::key(&e), and elided lifetimes. All of them compile through the alias, so I believe it's a false positive. If you'd rather have that check clean for a 4.x release, the alternative is to also keep verbatim copies of the two structs and their impls, about 110 more lines, inentry_api4. With all features on, it reports the intended api-5 changes: the new lifetime, andredb::iofrom #1638.Tests:
entry_guard_outlives_keyandentry_in_generic_code(api-5 only). I checked that both fail to compile against the current signature: one with E0597, the other with a lifetime error.Checked:
cargo fmt.--all --all-targets, with and without--all-features.cargo docin both modes. Without the feature,Entryis still an enum whose variants link to the aliases.cargo test --all-features, andcargo testwith default features.Generated by Claude Code