Skip to content

Commit 1249fab

Browse files
youknowoneyouknowone
authored
Merge pull request #2035 from youknowone/dictkey
Refactor DictKey
2 parents d33579f + 67b5d61 commit 1249fab

18 files changed

Lines changed: 126 additions & 137 deletions

File tree

‎common/src/hash.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ pub fn hash_float(value: f64) -> PyHash {
6868
x as PyHash * value.signum() as PyHash
6969
}
7070

71-
pub fn hash_value<T: Hash>(data: &T) -> PyHash {
71+
pub fn hash_value<T: Hash + ?Sized>(data: &T) -> PyHash {
7272
let mut hasher = DefaultHasher::new();
7373
data.hash(&mut hasher);
7474
hasher.finish() as PyHash

‎examples/mini_repl.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ macro_rules! add_python_function {
4040

4141
// inserts the first function found in the module into the provided scope.
4242
$scope.globals.set_item(
43-
&def.obj_name,
43+
def.obj_name.as_str(),
4444
$vm.context().new_pyfunction(
4545
vm::obj::objcode::PyCode::new(*def.clone()).into_ref(&$vm),
4646
$scope.clone(),

‎vm/src/bytesinner.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -883,9 +883,9 @@ impl PyBytesInner {
883883
return self
884884
.elements
885885
.iter()
886-
.cloned()
886+
.copied()
887887
.filter(|x| *x != b'\t')
888-
.collect::<Vec<u8>>();
888+
.collect();
889889
}
890890

891891
for i in &self.elements {

‎vm/src/dictdatatype.rs‎

Lines changed: 35 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,16 @@ impl<T: Clone> Dict<T> {
130130
}
131131

132132
/// Store a key
133-
pub fn insert<K: DictKey + IntoPyObject + Copy>(
134-
&self,
135-
vm: &VirtualMachine,
136-
key: K,
137-
value: T,
138-
) -> PyResult<()> {
133+
pub fn insert<K>(&self, vm: &VirtualMachine, key: K, value: T) -> PyResult<()>
134+
where
135+
K: DictKey,
136+
{
139137
// This does not need to be accurate so we can take the lock mutiple times.
140138
if self.borrow_value().indices.len() > 2 * self.borrow_value().size {
141139
self.resize();
142140
}
143141
loop {
144-
match self.lookup(vm, key)? {
142+
match self.lookup(vm, &key)? {
145143
LookupResult::Existing(index) => {
146144
let mut inner = self.borrow_value_mut();
147145
// Update existing key
@@ -166,7 +164,7 @@ impl<T: Clone> Dict<T> {
166164
}
167165
}
168166

169-
pub fn contains<K: DictKey + Copy>(&self, vm: &VirtualMachine, key: K) -> PyResult<bool> {
167+
pub fn contains<K: DictKey>(&self, vm: &VirtualMachine, key: &K) -> PyResult<bool> {
170168
if let LookupResult::Existing(_) = self.lookup(vm, key)? {
171169
Ok(true)
172170
} else {
@@ -176,7 +174,7 @@ impl<T: Clone> Dict<T> {
176174

177175
/// Retrieve a key
178176
#[cfg_attr(feature = "flame-it", flame("Dict"))]
179-
pub fn get<K: DictKey + Copy>(&self, vm: &VirtualMachine, key: K) -> PyResult<Option<T>> {
177+
pub fn get<K: DictKey>(&self, vm: &VirtualMachine, key: &K) -> PyResult<Option<T>> {
180178
loop {
181179
if let LookupResult::Existing(index) = self.lookup(vm, key)? {
182180
if let Some(entry) = &self.borrow_value().entries[index] {
@@ -304,7 +302,7 @@ impl<T: Clone> Dict<T> {
304302

305303
/// Lookup the index for the given key.
306304
#[cfg_attr(feature = "flame-it", flame("Dict"))]
307-
fn lookup<K: DictKey + Copy>(&self, vm: &VirtualMachine, key: K) -> PyResult<LookupResult> {
305+
fn lookup<K: DictKey>(&self, vm: &VirtualMachine, key: &K) -> PyResult<LookupResult> {
308306
let hash_value = key.do_hash(vm)?;
309307
let perturb = hash_value;
310308
let mut hash_index: HashIndex = hash_value;
@@ -359,7 +357,7 @@ impl<T: Clone> Dict<T> {
359357
}
360358

361359
/// Retrieve and delete a key
362-
pub fn pop<K: DictKey + Copy>(&self, vm: &VirtualMachine, key: K) -> PyResult<Option<T>> {
360+
pub fn pop<K: DictKey>(&self, vm: &VirtualMachine, key: &K) -> PyResult<Option<T>> {
363361
loop {
364362
if let LookupResult::Existing(index) = self.lookup(vm, key)? {
365363
let mut inner = self.borrow_value_mut();
@@ -413,41 +411,41 @@ enum LookupResult {
413411
/// the dictionary. Typical usecases are:
414412
/// - PyObjectRef -> arbitrary python type used as key
415413
/// - str -> string reference used as key, this is often used internally
416-
pub trait DictKey {
417-
fn do_hash(self, vm: &VirtualMachine) -> PyResult<HashValue>;
418-
fn do_is(self, other: &PyObjectRef) -> bool;
419-
fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool>;
414+
pub trait DictKey: IntoPyObject {
415+
fn do_hash(&self, vm: &VirtualMachine) -> PyResult<HashValue>;
416+
fn do_is(&self, other: &PyObjectRef) -> bool;
417+
fn do_eq(&self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool>;
420418
}
421419

422420
/// Implement trait for PyObjectRef such that we can use python objects
423421
/// to index dictionaries.
424-
impl DictKey for &PyObjectRef {
425-
fn do_hash(self, vm: &VirtualMachine) -> PyResult<HashValue> {
422+
impl DictKey for PyObjectRef {
423+
fn do_hash(&self, vm: &VirtualMachine) -> PyResult<HashValue> {
426424
let raw_hash = vm._hash(self)?;
427425
let mut hasher = DefaultHasher::new();
428426
raw_hash.hash(&mut hasher);
429427
Ok(hasher.finish() as HashValue)
430428
}
431429

432-
fn do_is(self, other: &PyObjectRef) -> bool {
430+
fn do_is(&self, other: &PyObjectRef) -> bool {
433431
self.is(other)
434432
}
435433

436-
fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
434+
fn do_eq(&self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
437435
vm.identical_or_equal(self, other_key)
438436
}
439437
}
440438

441-
impl DictKey for &PyStringRef {
442-
fn do_hash(self, _vm: &VirtualMachine) -> PyResult<HashValue> {
439+
impl DictKey for PyStringRef {
440+
fn do_hash(&self, _vm: &VirtualMachine) -> PyResult<HashValue> {
443441
Ok(self.hash())
444442
}
445443

446-
fn do_is(self, other: &PyObjectRef) -> bool {
444+
fn do_is(&self, other: &PyObjectRef) -> bool {
447445
self.is(other)
448446
}
449447

450-
fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
448+
fn do_eq(&self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
451449
if self.is(other_key) {
452450
Ok(true)
453451
} else if let Some(py_str_value) = other_key.payload::<PyString>() {
@@ -458,49 +456,37 @@ impl DictKey for &PyStringRef {
458456
}
459457
}
460458

459+
// AsRef<str> fit this case but not possible in rust 1.46
460+
461461
/// Implement trait for the str type, so that we can use strings
462462
/// to index dictionaries.
463463
impl DictKey for &str {
464-
fn do_hash(self, _vm: &VirtualMachine) -> PyResult<HashValue> {
464+
fn do_hash(&self, _vm: &VirtualMachine) -> PyResult<HashValue> {
465465
// follow a similar route as the hashing of PyStringRef
466-
let raw_hash = hash::hash_value(&self.to_owned()).to_bigint().unwrap();
466+
let raw_hash = hash::hash_value(*self).to_bigint().unwrap();
467467
let raw_hash = hash::hash_bigint(&raw_hash);
468468
let mut hasher = DefaultHasher::new();
469469
raw_hash.hash(&mut hasher);
470470
Ok(hasher.finish() as HashValue)
471471
}
472472

473-
fn do_is(self, _other: &PyObjectRef) -> bool {
473+
fn do_is(&self, _other: &PyObjectRef) -> bool {
474474
// No matter who the other pyobject is, we are never the same thing, since
475475
// we are a str, not a pyobject.
476476
false
477477
}
478478

479-
fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
479+
fn do_eq(&self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
480480
if let Some(py_str_value) = other_key.payload::<PyString>() {
481-
Ok(py_str_value.as_str() == self)
481+
Ok(py_str_value.as_str() == *self)
482482
} else {
483483
// Fall back to PyString implementation.
484-
let s = vm.new_str(self.to_owned());
484+
let s = vm.ctx.new_str(*self);
485485
s.do_eq(vm, other_key)
486486
}
487487
}
488488
}
489489

490-
impl DictKey for &String {
491-
fn do_hash(self, vm: &VirtualMachine) -> PyResult<HashValue> {
492-
self.as_str().do_hash(vm)
493-
}
494-
495-
fn do_is(self, other: &PyObjectRef) -> bool {
496-
self.as_str().do_is(other)
497-
}
498-
499-
fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
500-
self.as_str().do_eq(vm, other_key)
501-
}
502-
}
503-
504490
#[cfg(test)]
505491
mod tests {
506492
use super::{Dict, DictKey, VirtualMachine};
@@ -513,27 +499,27 @@ mod tests {
513499

514500
let key1 = vm.new_bool(true);
515501
let value1 = vm.new_str("abc".to_owned());
516-
dict.insert(&vm, &key1, value1.clone()).unwrap();
502+
dict.insert(&vm, key1.clone(), value1.clone()).unwrap();
517503
assert_eq!(1, dict.len());
518504

519505
let key2 = vm.new_str("x".to_owned());
520506
let value2 = vm.new_str("def".to_owned());
521-
dict.insert(&vm, &key2, value2.clone()).unwrap();
507+
dict.insert(&vm, key2.clone(), value2.clone()).unwrap();
522508
assert_eq!(2, dict.len());
523509

524-
dict.insert(&vm, &key1, value2.clone()).unwrap();
510+
dict.insert(&vm, key1.clone(), value2.clone()).unwrap();
525511
assert_eq!(2, dict.len());
526512

527513
dict.delete(&vm, &key1).unwrap();
528514
assert_eq!(1, dict.len());
529515

530-
dict.insert(&vm, &key1, value2.clone()).unwrap();
516+
dict.insert(&vm, key1.clone(), value2.clone()).unwrap();
531517
assert_eq!(2, dict.len());
532518

533519
assert_eq!(true, dict.contains(&vm, &key1).unwrap());
534-
assert_eq!(true, dict.contains(&vm, "x").unwrap());
520+
assert_eq!(true, dict.contains(&vm, &"x").unwrap());
535521

536-
let val = dict.get(&vm, "x").unwrap().unwrap();
522+
let val = dict.get(&vm, &"x").unwrap().unwrap();
537523
vm.bool_eq(val, value2)
538524
.expect("retrieved value must be equal to inserted value.");
539525
}

‎vm/src/frame.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,7 @@ impl ExecutingFrame<'_> {
956956
let idx = self.pop_value();
957957
let obj = self.pop_value();
958958
let value = self.pop_value();
959-
obj.set_item(&idx, value, vm)?;
959+
obj.set_item(idx, value, vm)?;
960960
Ok(None)
961961
}
962962

@@ -996,12 +996,12 @@ impl ExecutingFrame<'_> {
996996
return Err(vm.new_type_error(msg));
997997
}
998998
}
999-
map_obj.set_item(&key, value, vm).unwrap();
999+
map_obj.set_item(key, value, vm).unwrap();
10001000
}
10011001
}
10021002
} else {
10031003
for (key, value) in self.pop_multiple(2 * size).into_iter().tuples() {
1004-
map_obj.set_item(&key, value, vm).unwrap();
1004+
map_obj.set_item(key, value, vm).unwrap();
10051005
}
10061006
}
10071007

0 commit comments

Comments
 (0)