Merge pull request RustPython#3708 from youknowone/dictkey

Refactor dictkey

Author: youknowone

stdlib/src/bisect.rs

@@ -74,7 +74,7 @@ mod _bisect {
         while lo < hi {
             // Handles issue 13496.
             let mid = (lo + hi) / 2;
-            if a.get_item(mid, vm)?
+            if a.get_item(&mid, vm)?
                 .rich_compare_bool(&x, PyComparisonOp::Lt, vm)?
             {
                 lo = mid + 1;
@@ -104,7 +104,7 @@ mod _bisect {
         while lo < hi {
             // Handles issue 13496.
             let mid = (lo + hi) / 2;
-            if x.rich_compare_bool(&*a.get_item(mid, vm)?, PyComparisonOp::Lt, vm)? {
+            if x.rich_compare_bool(&*a.get_item(&mid, vm)?, PyComparisonOp::Lt, vm)? {
                 hi = mid;
             } else {
                 lo = mid + 1;

stdlib/src/dis.rs

@@ -33,8 +33,12 @@ mod decl {
     fn compiler_flag_names(vm: &VirtualMachine) -> PyDictRef {
         let dict = vm.ctx.new_dict();
         for (name, flag) in CodeFlags::NAME_MAPPING {
-            dict.set_item(vm.new_pyobj(flag.bits()), vm.ctx.new_str(*name).into(), vm)
-                .unwrap();
+            dict.set_item(
+                &*vm.new_pyobj(flag.bits()),
+                vm.ctx.new_str(*name).into(),
+                vm,
+            )
+            .unwrap();
         }
         dict
     }

vm/src/builtins/dict.rs

@@ -86,8 +86,8 @@ impl PyDict {
         if let Some(keys) = vm.get_method(other.clone(), "keys") {
             let keys = vm.invoke(&keys?, ())?.get_iter(vm)?;
             while let PyIterReturn::Return(key) = keys.next(vm)? {
-                let val = other.get_item(key.clone(), vm)?;
-                dict.insert(vm, key, val)?;
+                let val = other.get_item(&*key, vm)?;
+                dict.insert(vm, &*key, val)?;
             }
         } else {
             let iter = other.get_iter(vm)?;
@@ -105,7 +105,7 @@ impl PyDict {
                 if matches!(elem_iter.next(vm)?, PyIterReturn::Return(_)) {
                     return Err(err(vm));
                 }
-                dict.insert(vm, key, value)?;
+                dict.insert(vm, &*key, value)?;
             }
         }
         Ok(())
@@ -118,7 +118,7 @@ impl PyDict {
     ) -> PyResult<()> {
         let dict_size = &dict_other.size();
         for (key, value) in &dict_other {
-            dict.insert(vm, key, value)?;
+            dict.insert(vm, &*key, value)?;
         }
         if dict_other.entries.has_changed_size(dict_size) {
             return Err(vm.new_runtime_error("dict mutated during update".to_owned()));
@@ -146,7 +146,7 @@ impl PyDict {
             (zelf, other)
         };
         for (k, v1) in subset {
-            match superset.get_item_opt(k, vm)? {
+            match superset.get_item_opt(&*k, vm)? {
                 Some(v2) => {
                     if v1.is(&v2) {
                         continue;
@@ -169,18 +169,18 @@ impl PyDict {
 
     /// Set item variant which can be called with multiple
     /// key types, such as str to name a notable one.
-    pub(crate) fn inner_setitem<K: DictKey + ToPyObject>(
+    pub(crate) fn inner_setitem<K: DictKey + ?Sized>(
         &self,
-        key: K,
+        key: &K,
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         self.entries.insert(vm, key, value)
     }
 
-    pub(crate) fn inner_delitem<K: DictKey + ToPyObject>(
+    pub(crate) fn inner_delitem<K: DictKey + ?Sized>(
         &self,
-        key: K,
+        key: &K,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         self.entries.delete(vm, key)
@@ -192,22 +192,21 @@ impl PyDict {
         key: PyObjectRef,
         default: impl FnOnce() -> PyObjectRef,
     ) -> PyResult {
-        self.entries.setdefault(vm, key, default)
+        self.entries.setdefault(vm, &*key, default)
     }
 
     pub fn from_attributes(attrs: PyAttributes, vm: &VirtualMachine) -> PyResult<Self> {
         let dict = DictContentType::default();
 
         for (key, value) in attrs {
-            dict.insert(vm, vm.new_pyobj(key), value)?;
+            dict.insert(vm, key.as_str(), value)?;
         }
 
         Ok(PyDict { entries: dict })
     }
 
-    pub fn contains_key<K: ToPyObject>(&self, key: K, vm: &VirtualMachine) -> bool {
-        let key = key.to_pyobject(vm);
-        self.entries.contains(vm, &key).unwrap()
+    pub fn contains_key<K: DictKey + ?Sized>(&self, key: &K, vm: &VirtualMachine) -> bool {
+        self.entries.contains(vm, key).unwrap()
     }
 
     pub fn size(&self) -> dictdatatype::DictSize {
@@ -263,7 +262,7 @@ impl PyDict {
             }
             Err(pyobj) => {
                 for key in iterable.iter(vm)? {
-                    pyobj.set_item(key?, value.clone(), vm)?;
+                    pyobj.set_item(&*key?, value.clone(), vm)?;
                 }
                 Ok(pyobj)
             }
@@ -304,12 +303,12 @@ impl PyDict {
 
     #[pymethod(magic)]
     fn contains(&self, key: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
-        self.entries.contains(vm, &key)
+        self.entries.contains(vm, &*key)
     }
 
     #[pymethod(magic)]
     fn delitem(&self, key: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.inner_delitem(key, vm)
+        self.inner_delitem(&*key, vm)
     }
 
     #[pymethod]
@@ -334,13 +333,13 @@ impl PyDict {
 
     #[pymethod(magic)]
     fn setitem(&self, key: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.inner_setitem(key, value, vm)
+        self.inner_setitem(&*key, value, vm)
     }
 
     #[pymethod(magic)]
     #[cfg_attr(feature = "flame-it", flame("PyDictRef"))]
     fn getitem(zelf: PyRef<Self>, key: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        zelf.inner_getitem(key, vm)
+        zelf.inner_getitem(&*key, vm)
     }
 
     #[pymethod]
@@ -350,7 +349,7 @@ impl PyDict {
         default: OptionalArg<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
-        match self.entries.get(vm, &key)? {
+        match self.entries.get(vm, &*key)? {
             Some(value) => Ok(value),
             None => Ok(default.unwrap_or_none(vm)),
         }
@@ -364,7 +363,7 @@ impl PyDict {
         vm: &VirtualMachine,
     ) -> PyResult {
         self.entries
-            .setdefault(vm, key, || default.unwrap_or_none(vm))
+            .setdefault(vm, &*key, || default.unwrap_or_none(vm))
     }
 
     #[pymethod]
@@ -385,7 +384,7 @@ impl PyDict {
             Self::merge_object(&self.entries, dict_obj, vm)?;
         }
         for (key, value) in kwargs.into_iter() {
-            self.entries.insert(vm, vm.new_pyobj(key), value)?;
+            self.entries.insert(vm, &key, value)?;
         }
         Ok(())
     }
@@ -425,7 +424,7 @@ impl PyDict {
         default: OptionalArg<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
-        match self.entries.pop(vm, &key)? {
+        match self.entries.pop(vm, &*key)? {
             Some(value) => Ok(value),
             None => default.ok_or_else(|| vm.new_key_error(key)),
         }
@@ -523,25 +522,25 @@ impl Py<PyDict> {
         self.class().is(&vm.ctx.types.dict_type)
     }
 
-    fn missing_opt<K: DictKey + ToPyObject>(
+    fn missing_opt<K: DictKey + ?Sized>(
         &self,
-        key: K,
+        key: &K,
         vm: &VirtualMachine,
     ) -> PyResult<Option<PyObjectRef>> {
         vm.get_method(self.to_owned().into(), "__missing__")
-            .map(|methods| vm.invoke(&methods?, (key,)))
+            .map(|methods| vm.invoke(&methods?, (key.to_pyobject(vm),)))
             .transpose()
     }
 
     #[inline]
-    fn inner_getitem<K: DictKey + ToPyObject + Clone>(
+    fn inner_getitem<K: DictKey + ?Sized>(
         &self,
-        key: K,
+        key: &K,
         vm: &VirtualMachine,
     ) -> PyResult<PyObjectRef> {
-        if let Some(value) = self.entries.get(vm, &key)? {
+        if let Some(value) = self.entries.get(vm, key)? {
             Ok(value)
-        } else if let Some(value) = self.missing_opt(key.clone(), vm)? {
+        } else if let Some(value) = self.missing_opt(key, vm)? {
             Ok(value)
         } else {
             Err(vm.new_key_error(key.to_pyobject(vm)))
@@ -558,16 +557,16 @@ impl Py<PyDict> {
         attrs
     }
 
-    pub fn get_item_opt<K: DictKey + ToPyObject + Clone>(
+    pub fn get_item_opt<K: DictKey + ?Sized>(
         &self,
-        key: K,
+        key: &K,
         vm: &VirtualMachine,
     ) -> PyResult<Option<PyObjectRef>> {
         if self.exact_dict(vm) {
-            self.entries.get(vm, &key)
+            self.entries.get(vm, key)
             // FIXME: check __missing__?
         } else {
-            match self.as_object().get_item(key.clone(), vm) {
+            match self.as_object().get_item(key, vm) {
                 Ok(value) => Ok(Some(value)),
                 Err(e) if e.fast_isinstance(&vm.ctx.exceptions.key_error) => {
                     self.missing_opt(key, vm)
@@ -577,21 +576,17 @@ impl Py<PyDict> {
         }
     }
 
-    pub fn get_item<K: DictKey + ToPyObject + Clone>(
-        &self,
-        key: K,
-        vm: &VirtualMachine,
-    ) -> PyResult {
+    pub fn get_item<K: DictKey + ?Sized>(&self, key: &K, vm: &VirtualMachine) -> PyResult {
         if self.exact_dict(vm) {
             self.inner_getitem(key, vm)
         } else {
             self.as_object().get_item(key, vm)
         }
     }
 
-    pub fn set_item<K: DictKey + ToPyObject>(
+    pub fn set_item<K: DictKey + ?Sized>(
         &self,
-        key: K,
+        key: &K,
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
@@ -602,25 +597,25 @@ impl Py<PyDict> {
         }
     }
 
-    pub fn del_item<K: DictKey + ToPyObject>(&self, key: K, vm: &VirtualMachine) -> PyResult<()> {
+    pub fn del_item<K: DictKey + ?Sized>(&self, key: &K, vm: &VirtualMachine) -> PyResult<()> {
         if self.exact_dict(vm) {
             self.inner_delitem(key, vm)
         } else {
             self.as_object().del_item(key, vm)
         }
     }
 
-    pub fn get_chain<K: ToPyObject + DictKey + Clone>(
+    pub fn get_chain<K: DictKey + ?Sized>(
         &self,
         other: &Self,
-        key: K,
+        key: &K,
         vm: &VirtualMachine,
     ) -> PyResult<Option<PyObjectRef>> {
         let self_exact = self.exact_dict(vm);
         let other_exact = other.exact_dict(vm);
         if self_exact && other_exact {
-            self.entries.get_chain(&other.entries, vm, &key)
-        } else if let Some(value) = self.get_item_opt(key.clone(), vm)? {
+            self.entries.get_chain(&other.entries, vm, key)
+        } else if let Some(value) = self.get_item_opt(key, vm)? {
             Ok(Some(value))
         } else {
             other.get_item_opt(key, vm)

vm/src/builtins/enumerate.rs

@@ -128,7 +128,7 @@ impl IterNext for PyReverseSequenceIterator {
     fn next(zelf: &crate::Py<Self>, vm: &VirtualMachine) -> PyResult<PyIterReturn> {
         zelf.internal
             .lock()
-            .rev_next(|obj, pos| PyIterReturn::from_getitem_result(obj.get_item(pos, vm), vm))
+            .rev_next(|obj, pos| PyIterReturn::from_getitem_result(obj.get_item(&pos, vm), vm))
     }
 }
 

vm/src/builtins/function.rs

@@ -137,7 +137,7 @@ impl PyFunction {
                 }
                 *slot = Some(value);
             } else if let Some(kwargs) = kwargs.as_ref() {
-                kwargs.set_item(name, value, vm)?;
+                kwargs.set_item(&name, value, vm)?;
             } else if argpos(0..code.posonlyarg_count, &name).is_some() {
                 posonly_passed_as_kwarg.push(name);
             } else {
@@ -242,7 +242,7 @@ impl PyFunction {
                 .filter(|(slot, _)| slot.is_none())
             {
                 if let Some(defaults) = &get_defaults!().1 {
-                    if let Some(default) = defaults.get_item_opt(kwarg.clone(), vm)? {
+                    if let Some(default) = defaults.get_item_opt(&**kwarg, vm)? {
                         *slot = Some(default);
                         continue;
                     }

vm/src/builtins/function/jitfunc.rs

@@ -188,7 +188,7 @@ pub(crate) fn get_jit_args<'a>(
             let arg_idx = i + func.code.arg_count;
             if !jit_args.is_set(arg_idx) {
                 let default = kw_only_defaults
-                    .get_item(name.clone(), vm)
+                    .get_item(&**name, vm)
                     .map_err(|_| ArgsError::NotAllArgsPassed)
                     .and_then(|obj| get_jit_value(vm, &obj))?;
                 jit_args.set(arg_idx, default)?;

vm/src/builtins/genericalias.rs

@@ -249,7 +249,7 @@ fn subs_tvars(
                             })
                             .collect::<Vec<_>>();
                         let sub_args: PyObjectRef = PyTuple::new_ref(sub_args, &vm.ctx).into();
-                        Some(obj.get_item(sub_args, vm))
+                        Some(obj.get_item(&*sub_args, vm))
                     } else {
                         None
                     }

vm/src/builtins/mappingproxy.rs

@@ -67,7 +67,7 @@ impl PyMappingProxy {
                 let key = PyStrRef::try_from_object(vm, key)?;
                 class.attributes.read().get(key.as_str()).cloned()
             }
-            MappingProxyInner::Dict(obj) => obj.get_item(key, vm).ok(),
+            MappingProxyInner::Dict(obj) => obj.get_item(&*key, vm).ok(),
         };
         Ok(opt)
     }

vm/src/builtins/set.rs

@@ -174,7 +174,7 @@ impl PySetInner {
     ) -> PyResult<PySetInner> {
         let set = self.copy();
         for item in other.iter(vm)? {
-            set.content.delete_if_exists(vm, &item?)?;
+            set.content.delete_if_exists(vm, &*item?)?;
         }
         Ok(set)
     }
@@ -231,13 +231,11 @@ impl PySetInner {
     }
 
     fn add(&self, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.content.insert(vm, item, ())
+        self.content.insert(vm, &*item, ())
     }
 
     fn remove(&self, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.retry_op_with_frozenset(&item, vm, |item, vm| {
-            self.content.delete(vm, item.to_owned())
-        })
+        self.retry_op_with_frozenset(&item, vm, |item, vm| self.content.delete(vm, item))
     }
 
     fn discard(&self, item: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
@@ -293,7 +291,7 @@ impl PySetInner {
     fn difference_update(&self, others: PosArgs<ArgIterable>, vm: &VirtualMachine) -> PyResult<()> {
         for iterable in others {
             for item in iterable.iter(vm)? {
-                self.content.delete_if_exists(vm, &item?)?;
+                self.content.delete_if_exists(vm, &*item?)?;
             }
         }
         Ok(())