From ce99057c8103e19dee486fca212668e8d07e5b61 Mon Sep 17 00:00:00 2001 From: eldpswp99 Date: Tue, 17 Aug 2021 20:41:37 +0900 Subject: [PATCH] make dict delete remains order --- Lib/test/test_dict.py | 6 ---- vm/src/builtins/dict.rs | 2 +- vm/src/builtins/set.rs | 2 +- vm/src/dictdatatype.rs | 78 +++++++++++++++++++++++++---------------- 4 files changed, 49 insertions(+), 39 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index ce449f7f92..77a5065e61 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -474,8 +474,6 @@ class DictTest(unittest.TestCase): for i in d: d[i+1] = 1 - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_mutating_iteration_delete(self): # change dict content during iteration d = {} @@ -485,8 +483,6 @@ class DictTest(unittest.TestCase): del d[0] d[0] = 0 - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_mutating_iteration_delete_over_values(self): # change dict content during iteration d = {} @@ -496,8 +492,6 @@ class DictTest(unittest.TestCase): del d[0] d[0] = 0 - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_mutating_iteration_delete_over_items(self): # change dict content during iteration d = {} diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index cfbdb53945..f25c376716 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -360,7 +360,7 @@ impl PyDict { #[pymethod] fn popitem(&self, vm: &VirtualMachine) -> PyResult { - if let Some((key, value)) = self.entries.pop_back() { + if let Some((key, value)) = self.entries.pop_back(vm) { Ok(vm.ctx.new_tuple(vec![key, value])) } else { let err_msg = vm.ctx.new_str("popitem(): dictionary is empty"); diff --git a/vm/src/builtins/set.rs b/vm/src/builtins/set.rs index 6c5cacba62..20f3972ea8 100644 --- a/vm/src/builtins/set.rs +++ b/vm/src/builtins/set.rs @@ -226,7 +226,7 @@ impl PySetInner { fn pop(&self, vm: &VirtualMachine) -> PyResult { // TODO: should be pop_front, but that requires rearranging every index - if let Some((key, _)) = self.content.pop_back() { + if let Some((key, _)) = self.content.pop_back(vm) { Ok(key) } else { let err_msg = vm.ctx.new_str("pop from an empty set"); diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index 23e9f6e3c8..2fe119df3e 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -221,8 +221,8 @@ impl Dict { /// Store a key pub fn insert(&self, vm: &VirtualMachine, key: K, value: T) -> PyResult<()> - where - K: DictKey, + where + K: DictKey, { let hash = key.key_hash(vm)?; let _removed = loop { @@ -320,8 +320,8 @@ impl Dict { /// Delete a key pub fn delete(&self, vm: &VirtualMachine, key: K) -> PyResult<()> - where - K: DictKey, + where + K: DictKey, { if self.delete_if_exists(vm, &key)? { Ok(()) @@ -331,8 +331,8 @@ impl Dict { } pub fn delete_if_exists(&self, vm: &VirtualMachine, key: &K) -> PyResult - where - K: DictKey, + where + K: DictKey, { let hash = key.key_hash(vm)?; let deleted = loop { @@ -376,9 +376,9 @@ impl Dict { } pub fn setdefault(&self, vm: &VirtualMachine, key: K, default: F) -> PyResult - where - K: DictKey, - F: FnOnce() -> T, + where + K: DictKey, + F: FnOnce() -> T, { let hash = key.key_hash(vm)?; let res = loop { @@ -419,9 +419,9 @@ impl Dict { key: K, default: F, ) -> PyResult<(PyObjectRef, T)> - where - K: DictKey, - F: FnOnce() -> T, + where + K: DictKey, + F: FnOnce() -> T, { let hash = key.key_hash(vm)?; let res = loop { @@ -489,9 +489,12 @@ impl Dict { } pub fn keys(&self) -> Vec { - self.read().entries.iter() + self.read() + .entries + .iter() .filter(|v| v.is_some()) - .map(|v| v.as_ref().unwrap().key.clone()).collect() + .map(|v| v.as_ref().unwrap().key.clone()) + .collect() } /// Lookup the index for the given key. @@ -562,7 +565,8 @@ impl Dict { return Ok(None); }; let mut inner = self.write(); - if matches!(inner.entries.get(entry_index), Some(entry) if entry.as_ref().unwrap().index == index_index) { + if matches!(inner.entries.get(entry_index), Some(entry) if entry.as_ref().unwrap().index == index_index) + { // all good } else { // The dict was changed since we did lookup. Let's try again. @@ -570,14 +574,9 @@ impl Dict { }; inner.indices[index_index] = IndexEntry::DUMMY; inner.used -= 1; - let removed = if entry_index == inner.used { - inner.entries.pop().unwrap() - } else { - let last_index = inner.entries.last().unwrap().as_ref().unwrap().index; - let removed = inner.entries.swap_remove(entry_index); - inner.indices[last_index] = entry_index as i64; - removed - }; + let removed = inner.entries[entry_index].clone(); + inner.entries[entry_index] = None; + Ok(Some(removed.unwrap())) } @@ -595,14 +594,31 @@ impl Dict { Ok(removed) } - pub fn pop_back(&self) -> Option<(PyObjectRef, T)> { - let mut inner = self.write(); - inner.entries.pop().map(|entry| { - let entry = entry.unwrap(); - inner.used -= 1; - inner.indices[entry.index] = IndexEntry::DUMMY; - (entry.key, entry.value) - }) + pub fn pop_back(&self, vm: &VirtualMachine) -> Option<(PyObjectRef, T)> { + let inner = self.read(); + let mut idx: i64 = inner.size().entries_size as i64 - 1; + + while idx >= 0 && inner.entries.get(idx as usize).unwrap().is_none() { + idx -= 1; + } + + if idx < 0 { + None + } else { + let idx = idx as usize; + let entry = inner.entries.get(idx).unwrap().as_ref().unwrap().clone(); + + let removed_key = entry.key; + let removed_item = entry.value; + if let Ok(lookup) = self.lookup(vm, &removed_key, entry.hash, Some(inner)) { + match self.pop_inner(lookup) { + Ok(_) => Some((removed_key, removed_item)), + Err(_) => None, + } + } else { + None + } + } } pub fn sizeof(&self) -> usize {