Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions crates/vm/src/builtins/dict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,14 @@ impl PyDict {
) -> PyResult<()> {
let dict = &self.entries;
let dict_size = &dict_other.size();
for (key, value) in dict_other {
if !override_existing && dict.contains(vm, &*key)? {
let mut position = 0;
while let Some((next, key, value, hash)) = dict_other.entries.next_entry_with_hash(position)
{
position = next;
if !override_existing && dict.contains_known_hash(vm, &*key, hash)? {
continue;
}
dict.insert(vm, &*key, value)?;
dict.insert_known_hash(vm, &*key, hash, value)?;
}
if dict_other.entries.has_changed_size(dict_size) {
return Err(vm.new_runtime_error("dict mutated during update"));
Expand Down
17 changes: 13 additions & 4 deletions crates/vm/src/dict_inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,9 @@ impl<T: Clone> Dict<T> {
/// Store a key whose hash the caller already knows.
///
/// `hash` must be computed for `key` or read from [`Self::keys_with_hashes`]
/// on a container holding this same key. A wrong hash lands the entry in a
/// bucket no lookup probes, silently losing the key.
/// or [`Self::next_entry_with_hash`] on a container holding this same key.
/// A wrong hash lands the entry in a bucket no lookup probes, silently
/// losing the key.
pub(crate) fn insert_known_hash<K>(
&self,
vm: &VirtualMachine,
Expand Down Expand Up @@ -1037,13 +1038,21 @@ impl<T: Clone> Dict<T> {
}
}

pub(crate) fn next_entry(&self, mut position: EntryIndex) -> Option<(usize, PyObjectRef, T)> {
pub(crate) fn next_entry(&self, position: EntryIndex) -> Option<(usize, PyObjectRef, T)> {
self.next_entry_with_hash(position)
.map(|(position, key, value, _)| (position, key, value))
}

pub(crate) fn next_entry_with_hash(
&self,
mut position: EntryIndex,
) -> Option<(usize, PyObjectRef, T, HashValue)> {
let inner = self.read();
loop {
let entry = inner.entries.get(position)?;
position += 1;
if let Some(entry) = entry {
break Some((position, entry.key.clone(), entry.value.clone()));
break Some((position, entry.key.clone(), entry.value.clone(), entry.hash));
}
}
}
Expand Down
72 changes: 72 additions & 0 deletions extra_tests/snippets/builtin_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,3 +615,75 @@ def __eq__(self, other):
lookup = LookupOrderSubclass("lookup", True)
assert {stored: 1}[lookup] == 1
assert LookupOrderKey.calls == ["lookup"]


class MergeHashKey:
hash_disabled = False

def __init__(self, value):
self.value = value

def __hash__(self):
assert not self.hash_disabled, "dictionary merge rehashed a stored key"
return 42

def __eq__(self, other):
if not isinstance(other, MergeHashKey):
return NotImplemented
return self.value == other.value


def merge_with_update(source):
result = {}
result.update(source)
return result


def merge_with_ior(source):
result = {}
result |= source
return result


# Exact dictionary merges reuse hashes, including colliding keys and holes.
merge_keys = [MergeHashKey(i) for i in range(3)]
merge_source = dict(zip(merge_keys, ("first", "removed", "last")))
del merge_source[merge_keys[1]]
existing_merge_key = MergeHashKey(2)
merge_target = {existing_merge_key: "old"}
MergeHashKey.hash_disabled = True
for merge in (
dict,
merge_with_update,
merge_with_ior,
lambda source: {} | source,
lambda source: dict.__ror__(source, {}),
lambda source: {**source},
):
assert list(merge(merge_source).items()) == list(merge_source.items())

# Overwriting a matching key keeps its identity and position.
merge_target.update(merge_source)
assert next(iter(merge_target)) is existing_merge_key
assert list(merge_target.values()) == ["last", "first"]
merge_source.update(merge_source)
merge_source |= merge_source
assert list(merge_source.values()) == ["first", "last"]
MergeHashKey.hash_disabled = False


class MergeMapping(dict):
def __iter__(self):
return iter(("virtual",))

def keys(self):
return ["virtual"]

def __getitem__(self, key):
assert key == "virtual"
return 42


# Generic mappings retain their lookup hooks instead of exposing dict storage.
for merge in (dict, merge_with_update, merge_with_ior):
assert merge(MergeMapping(stored=0)) == {"virtual": 42}
Loading