From 0f3569a1b1df15a90ed539541840597d0153122b Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Tue, 23 Nov 2021 20:22:01 +0200 Subject: [PATCH 1/9] refactor mapping protocol --- vm/src/builtins/bytearray.rs | 48 ++++----- vm/src/builtins/bytes.rs | 36 +++---- vm/src/builtins/dict.rs | 47 ++++----- vm/src/builtins/genericalias.rs | 119 ++++++++++------------- vm/src/builtins/list.rs | 47 ++++----- vm/src/builtins/mappingproxy.rs | 36 +++---- vm/src/builtins/memory.rs | 49 ++++------ vm/src/builtins/pystr.rs | 39 +++----- vm/src/builtins/range.rs | 40 +++----- vm/src/builtins/tuple.rs | 37 +++---- vm/src/protocol/mapping.rs | 167 +++++++++++++++++--------------- vm/src/types/slot.rs | 36 ------- 12 files changed, 277 insertions(+), 424 deletions(-) diff --git a/vm/src/builtins/bytearray.rs b/vm/src/builtins/bytearray.rs index c7eb74165c..13ba0882cd 100644 --- a/vm/src/builtins/bytearray.rs +++ b/vm/src/builtins/bytearray.rs @@ -677,6 +677,23 @@ impl PyByteArray { } } +impl PyByteArray { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + subscript: Some(|mapping, needle, vm| { + mapping.obj_as::().getitem(needle.to_owned(), vm) + }), + ass_subscript: Some(|mapping, needle, value, vm| { + let zelf = mapping.obj_as::(); + if let Some(value) = value { + Self::setitem(zelf.to_owned(), needle.to_owned(), value, vm) + } else { + zelf.delitem(needle.to_owned(), vm) + } + }), + }; +} + impl Comparable for PyByteArray { fn cmp( zelf: &crate::PyObjectView, @@ -739,36 +756,7 @@ impl<'a> BufferResizeGuard<'a> for PyByteArray { impl AsMapping for PyByteArray { fn as_mapping(_zelf: &crate::PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: Some(Self::ass_subscript), - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len()))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| zelf.getitem(needle, vm))? - } - - #[inline] - fn ass_subscript( - zelf: PyObjectRef, - needle: PyObjectRef, - value: Option, - vm: &VirtualMachine, - ) -> PyResult<()> { - match value { - Some(value) => { - Self::downcast(zelf, vm).map(|zelf| Self::setitem(zelf, needle, value, vm)) - } - None => Self::downcast_ref(&zelf, vm).map(|zelf| zelf.delitem(needle, vm)), - }? + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/bytes.rs b/vm/src/builtins/bytes.rs index 750a914255..39e1276052 100644 --- a/vm/src/builtins/bytes.rs +++ b/vm/src/builtins/bytes.rs @@ -523,6 +523,16 @@ impl PyBytes { } } +impl PyBytes { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + subscript: Some(|mapping, needle, vm| { + mapping.obj_as::().getitem(needle.to_owned(), vm) + }), + ass_subscript: None, + }; +} + static BUFFER_METHODS: BufferMethods = BufferMethods { obj_bytes: |buffer| buffer.obj_as::().as_bytes().into(), obj_bytes_mut: |_| panic!(), @@ -543,31 +553,7 @@ impl AsBuffer for PyBytes { impl AsMapping for PyBytes { fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: None, - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len()))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| zelf.getitem(needle, vm))? - } - - #[cold] - fn ass_subscript( - zelf: PyObjectRef, - _needle: PyObjectRef, - _value: Option, - _vm: &VirtualMachine, - ) -> PyResult<()> { - unreachable!("ass_subscript not implemented for {}", zelf.class()) + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index ec1e7e99f8..cd43a0a0fc 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -420,36 +420,27 @@ impl PyDict { } } +impl PyDict { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + subscript: Some(|mapping, needle, vm| { + let zelf = mapping.obj_as::(); + Self::getitem(zelf.to_owned(), needle.to_owned(), vm) + }), + ass_subscript: Some(|mapping, needle, value, vm| { + let zelf = mapping.obj_as::(); + if let Some(value) = value { + zelf.setitem(needle.to_owned(), value, vm) + } else { + zelf.delitem(needle.to_owned(), vm) + } + }), + }; +} + impl AsMapping for PyDict { fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: Some(Self::ass_subscript), - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len()))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast(zelf, vm).map(|zelf| Self::getitem(zelf, needle, vm))? - } - - #[inline] - fn ass_subscript( - zelf: PyObjectRef, - needle: PyObjectRef, - value: Option, - vm: &VirtualMachine, - ) -> PyResult<()> { - Self::downcast_ref(&zelf, vm).map(|zelf| match value { - Some(value) => zelf.setitem(needle, value, vm), - None => zelf.delitem(needle, vm), - })? + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/genericalias.rs b/vm/src/builtins/genericalias.rs index f9a062ea49..907206ca31 100644 --- a/vm/src/builtins/genericalias.rs +++ b/vm/src/builtins/genericalias.rs @@ -237,81 +237,68 @@ fn subs_tvars( .unwrap_or(Ok(obj)) } -impl AsMapping for PyGenericAlias { - fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: None, - subscript: Some(Self::subscript), - ass_subscript: None, +impl PyGenericAlias { + fn getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { + let num_params = self.parameters.len(); + if num_params == 0 { + return Err(vm.new_type_error(format!( + "There are no type variables left in {}", + self.repr(vm)? + ))); } - } - - #[cold] - fn length(zelf: PyObjectRef, _vm: &VirtualMachine) -> PyResult { - unreachable!("length not implemented for {}", zelf.class()) - } - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast(zelf, vm).map(|zelf| { - let num_params = zelf.parameters.len(); - if num_params == 0 { - return Err(vm.new_type_error(format!( - "There are no type variables left in {}", - zelf.repr(vm)? - ))); - } + let items = PyTupleRef::try_from_object(vm, needle.clone()); + let arg_items = match items { + Ok(ref tuple) => tuple.as_slice(), + Err(_) => std::slice::from_ref(&needle), + }; - let items = PyTupleRef::try_from_object(vm, needle.clone()); - let arg_items = match items { - Ok(ref tuple) => tuple.as_slice(), - Err(_) => std::slice::from_ref(&needle), + let num_items = arg_items.len(); + if num_params != num_items { + let plural = if num_items > num_params { + "many" + } else { + "few" }; + return Err(vm.new_type_error(format!( + "Too {} arguments for {}", + plural, + self.repr(vm)? + ))); + } - let num_items = arg_items.len(); - if num_params != num_items { - let plural = if num_items > num_params { - "many" + let new_args = self + .args + .as_slice() + .iter() + .map(|arg| { + if is_typevar(arg) { + let idx = tuple_index(&self.parameters, arg).unwrap(); + Ok(arg_items[idx].clone()) } else { - "few" - }; - return Err(vm.new_type_error(format!( - "Too {} arguments for {}", - plural, - zelf.repr(vm)? - ))); - } + subs_tvars(arg.clone(), &self.parameters, arg_items, vm) + } + }) + .collect::>>()?; - let new_args = zelf - .args - .as_slice() - .iter() - .map(|arg| { - if is_typevar(arg) { - let idx = tuple_index(&zelf.parameters, arg).unwrap(); - Ok(arg_items[idx].clone()) - } else { - subs_tvars(arg.clone(), &zelf.parameters, arg_items, vm) - } - }) - .collect::>>()?; - - Ok(PyGenericAlias::new( - zelf.origin.clone(), - PyTuple::new_ref(new_args, &vm.ctx).into(), - vm, - ) - .into_object(vm)) - })? + Ok(PyGenericAlias::new( + self.origin.clone(), + PyTuple::new_ref(new_args, &vm.ctx).into(), + vm, + ) + .into_object(vm)) } - #[cold] - fn ass_subscript( - zelf: PyObjectRef, - _needle: PyObjectRef, - _value: Option, - _vm: &VirtualMachine, - ) -> PyResult<()> { - unreachable!("ass_subscript not implemented for {}", zelf.class()) + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: None, + subscript: Some(|mapping, needle, vm| mapping.obj_as::().getitem(needle, vm)), + ass_subscript: None, + }; +} + +impl AsMapping for PyGenericAlias { + fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/list.rs b/vm/src/builtins/list.rs index 835d84acbb..8702668507 100644 --- a/vm/src/builtins/list.rs +++ b/vm/src/builtins/list.rs @@ -389,36 +389,27 @@ impl<'a> MutObjectSequenceOp<'a> for PyList { } } +impl PyList { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + subscript: Some(|mapping, needle, vm| { + let zelf = mapping.obj_as::(); + Self::getitem(zelf.to_owned(), needle.to_owned(), vm) + }), + ass_subscript: Some(|mapping, needle, value, vm| { + let zelf = mapping.obj_as::(); + if let Some(value) = value { + zelf.setitem(needle.to_owned(), value, vm) + } else { + zelf.delitem(needle.to_owned(), vm) + } + }), + }; +} + impl AsMapping for PyList { fn as_mapping(_zelf: &crate::PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: Some(Self::ass_subscript), - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len()))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast(zelf, vm).map(|zelf| Self::getitem(zelf, needle, vm))? - } - - #[inline] - fn ass_subscript( - zelf: PyObjectRef, - needle: PyObjectRef, - value: Option, - vm: &VirtualMachine, - ) -> PyResult<()> { - Self::downcast_ref(&zelf, vm).map(|zelf| match value { - Some(value) => zelf.setitem(needle, value, vm), - None => zelf.delitem(needle, vm), - })? + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/mappingproxy.rs b/vm/src/builtins/mappingproxy.rs index ab6a147fe6..4134f836bc 100644 --- a/vm/src/builtins/mappingproxy.rs +++ b/vm/src/builtins/mappingproxy.rs @@ -155,33 +155,19 @@ impl PyMappingProxy { } } +impl PyMappingProxy { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: None, + subscript: Some(|mapping, needle, vm| { + mapping.obj_as::().getitem(needle.to_owned(), vm) + }), + ass_subscript: None, + }; +} + impl AsMapping for PyMappingProxy { fn as_mapping(_zelf: &crate::PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: None, - subscript: Some(Self::subscript), - ass_subscript: None, - } - } - - #[inline] - fn length(zelf: PyObjectRef, _vm: &VirtualMachine) -> PyResult { - unreachable!("length not implemented for {}", zelf.class()) - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| zelf.getitem(needle, vm))? - } - - #[cold] - fn ass_subscript( - zelf: PyObjectRef, - _needle: PyObjectRef, - _value: Option, - _vm: &VirtualMachine, - ) -> PyResult<()> { - unreachable!("ass_subscript not implemented for {}", zelf.class()) + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/memory.rs b/vm/src/builtins/memory.rs index df52bd76af..7682b4c7f3 100644 --- a/vm/src/builtins/memory.rs +++ b/vm/src/builtins/memory.rs @@ -938,38 +938,27 @@ impl Drop for PyMemoryView { } } +impl PyMemoryView { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, vm| mapping.obj_as::().len(vm)), + subscript: Some(|mapping, needle, vm| { + let zelf = mapping.obj_as::(); + Self::getitem(zelf.to_owned(), needle.to_owned(), vm) + }), + ass_subscript: Some(|mapping, needle, value, vm| { + let zelf = mapping.obj_as::(); + if let Some(value) = value { + Self::setitem(zelf.to_owned(), needle.to_owned(), value, vm) + } else { + Err(vm.new_type_error("cannot delete memory".to_owned())) + } + }), + }; +} + impl AsMapping for PyMemoryView { fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: Some(Self::ass_subscript), - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| zelf.len(vm))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast(zelf, vm).map(|zelf| Self::getitem(zelf, needle, vm))? - } - - #[inline] - fn ass_subscript( - zelf: PyObjectRef, - needle: PyObjectRef, - value: Option, - vm: &VirtualMachine, - ) -> PyResult<()> { - match value { - Some(value) => { - Self::downcast(zelf, vm).map(|zelf| Self::setitem(zelf, needle, value, vm))? - } - None => Err(vm.new_type_error("cannot delete memory".to_owned())), - } + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/pystr.rs b/vm/src/builtins/pystr.rs index 080f4f9bdf..5efa26c532 100644 --- a/vm/src/builtins/pystr.rs +++ b/vm/src/builtins/pystr.rs @@ -1260,34 +1260,21 @@ impl Iterable for PyStr { impl AsMapping for PyStr { fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: None, - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len()))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm) - .map(|zelf| zelf.getitem(needle, vm))? - .map(|item| item.into_object(vm)) + Self::MAPPING_METHODS.clone() } +} - #[cold] - fn ass_subscript( - zelf: PyObjectRef, - _needle: PyObjectRef, - _value: Option, - _vm: &VirtualMachine, - ) -> PyResult<()> { - unreachable!("ass_subscript not implemented for {}", zelf.class()) - } +impl PyStr { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Self::SEQUENCE_METHDOS.length, + subscript: Some(|mapping, needle, vm| { + mapping + .obj_as::() + ._getitem(needle, vm) + .map(|x| x.into_ref(vm).into()) + }), + ass_subscript: None, + }; } #[derive(FromArgs)] diff --git a/vm/src/builtins/range.rs b/vm/src/builtins/range.rs index 8cea7fa31c..f8071fc917 100644 --- a/vm/src/builtins/range.rs +++ b/vm/src/builtins/range.rs @@ -377,33 +377,23 @@ impl PyRange { } } +impl PyRange { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, vm| { + mapping.obj_as::().len().to_usize().ok_or_else(|| { + vm.new_overflow_error("RustPython int too large to convert to C ssize_t".to_owned()) + }) + }), + subscript: Some(|mapping, needle, vm| { + mapping.obj_as::().getitem(needle.to_owned(), vm) + }), + ass_subscript: None, + }; +} + impl AsMapping for PyRange { fn as_mapping(_zelf: &crate::PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: None, - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len().to_usize().unwrap()))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| zelf.getitem(needle, vm))? - } - - #[inline] - fn ass_subscript( - zelf: PyObjectRef, - _needle: PyObjectRef, - _value: Option, - _vm: &VirtualMachine, - ) -> PyResult<()> { - unreachable!("ass_subscript not implemented for {}", zelf.class()) + Self::MAPPING_METHODS } } diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index b69b520a2a..abffd688b5 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -308,33 +308,20 @@ impl PyTuple { } } +impl PyTuple { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + subscript: Some(|mapping, needle, vm| { + let zelf = mapping.obj_as::(); + Self::getitem(zelf.to_owned(), needle.to_owned(), vm) + }), + ass_subscript: None, + }; +} + impl AsMapping for PyTuple { fn as_mapping(_zelf: &crate::PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: None, - } - } - - #[inline] - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len()))? - } - - #[inline] - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast(zelf, vm).map(|zelf| Self::getitem(zelf, needle, vm))? - } - - #[inline] - fn ass_subscript( - zelf: PyObjectRef, - _needle: PyObjectRef, - _value: Option, - _vm: &VirtualMachine, - ) -> PyResult<()> { - unreachable!("ass_subscript not implemented for {}", zelf.class()) + Self::MAPPING_METHODS } } diff --git a/vm/src/protocol/mapping.rs b/vm/src/protocol/mapping.rs index 3eee59a943..b0fc7d2194 100644 --- a/vm/src/protocol/mapping.rs +++ b/vm/src/protocol/mapping.rs @@ -1,82 +1,116 @@ use crate::{ builtins::{ - dict::{PyDictKeys, PyDictValues}, - PyDictRef, PyList, + dict::{PyDictItems, PyDictKeys, PyDictValues}, + PyDict, }, - function::IntoPyObject, - IdProtocol, PyObject, PyObjectRef, PyObjectWrap, PyResult, TryFromObject, TypeProtocol, + common::lock::OnceCell, + IdProtocol, PyObject, PyObjectPayload, PyObjectRef, PyObjectView, PyResult, TypeProtocol, VirtualMachine, }; -use std::borrow::Borrow; // Mapping protocol // https://docs.python.org/3/c-api/mapping.html #[allow(clippy::type_complexity)] -#[derive(Default)] +#[derive(Default, Copy, Clone)] pub struct PyMappingMethods { - pub length: Option PyResult>, - pub subscript: Option PyResult>, + pub length: Option PyResult>, + pub subscript: Option PyResult>, pub ass_subscript: - Option, &VirtualMachine) -> PyResult<()>>, + Option, &VirtualMachine) -> PyResult<()>>, } #[derive(Debug, Clone)] -#[repr(transparent)] -pub struct PyMapping(T) -where - T: Borrow; - -impl PyMapping { - pub fn check(obj: &PyObject, vm: &VirtualMachine) -> bool { - obj.class() - .mro_find_map(|x| x.slots.as_mapping.load()) - .map(|f| f(obj, vm).subscript.is_some()) - .unwrap_or(false) +pub struct PyMapping<'a> { + pub obj: &'a PyObject, + methods: OnceCell, +} + +impl From<&PyObject> for PyMapping<'_> { + fn from(obj: &PyObject) -> Self { + Self { + obj, + methods: OnceCell::new(), + } + } +} + +impl AsRef for PyMapping<'_> { + fn as_ref(&self) -> &PyObject { + self.obj + } +} + +impl PyMapping<'_> { + pub fn obj_as(&self) -> &PyObjectView { + unsafe { self.obj.downcast_unchecked_ref::() } } - pub fn methods(&self, vm: &VirtualMachine) -> PyMappingMethods { - let obj_cls = self.0.class(); - for cls in obj_cls.iter_mro() { - if let Some(f) = cls.slots.as_mapping.load() { - return f(&self.0, vm); + pub fn methods(&self, vm: &VirtualMachine) -> &PyMappingMethods { + self.methods.get_or_init(|| { + if let Some(f) = self + .obj + .class() + .mro_find_map(|cls| cls.slots.as_mapping.load()) + { + f(self.obj, vm) + } else { + PyMappingMethods::default() } + }) + } + + // PyMapping::Check + pub fn has_protocol(&self, vm: &VirtualMachine) -> bool { + self.methods(vm).subscript.is_some() + } + + pub fn try_protocol(&self, vm: &VirtualMachine) -> PyResult<()> { + if self.has_protocol(vm) { + Ok(()) + } else { + Err(vm.new_type_error(format!("{} is not a mapping object", self.obj.class()))) } - PyMappingMethods::default() } -} -impl PyMapping -where - T: Borrow, -{ - pub fn new(obj: T) -> Self { - Self(obj) + pub fn length_opt(&self, vm: &VirtualMachine) -> Option> { + self.methods(vm).length.map(|f| f(self, vm)) + } + + pub fn length(&self, vm: &VirtualMachine) -> PyResult { + self.length_opt(vm).ok_or_else(|| { + vm.new_type_error(format!( + "object of type '{}' has no len() or not a mapping", + self.0.class().name() + )) + })? } pub fn keys(&self, vm: &VirtualMachine) -> PyResult { - if self.0.borrow().is(&vm.ctx.types.dict_type) { - Ok( - PyDictKeys::new(PyDictRef::try_from_object(vm, self.0.borrow().to_owned())?) - .into_pyobject(vm), - ) + if let Some(dict) = self.obj.downcast_ref_if_exact::(vm) { + PyDictKeys::new(dict.to_owned()).into_pyresult(vm) } else { - Self::method_output_as_list(self.0.borrow(), "keys", vm) + self.method_output_as_list("keys", vm) } } pub fn values(&self, vm: &VirtualMachine) -> PyResult { - if self.0.borrow().is(&vm.ctx.types.dict_type) { - Ok( - PyDictValues::new(PyDictRef::try_from_object(vm, self.0.borrow().to_owned())?) - .into_pyobject(vm), - ) + if let Some(dict) = self.obj.downcast_ref_if_exact::(vm) { + PyDictValues::new(dict.to_owned()).into_pyresult(vm) } else { - Self::method_output_as_list(self.0.borrow(), "values", vm) + self.method_output_as_list("values", vm) } } - fn method_output_as_list(obj: &PyObject, method_name: &str, vm: &VirtualMachine) -> PyResult { - let meth_output = vm.call_method(obj, method_name, ())?; + pub fn items(&self, vm: &VirtualMachine) -> PyResult { + if let Some(dict) = self.obj.downcast_ref_if_exact::(vm) { + PyDictItems::new(dict.to_owned()).into_pyresult(vm) + } else { + self.method_output_as_list("items", vm) + } + } + + fn method_output_as_list(&self, method_name: &str, vm: &VirtualMachine) -> PyResult { + let meth_output = vm.call_method(self.obj, method_name, ())?; if meth_output.is(&vm.ctx.types.list_type) { return Ok(meth_output); } @@ -84,43 +118,16 @@ where let iter = meth_output.clone().get_iter(vm).map_err(|_| { vm.new_type_error(format!( "{}.{}() returned a non-iterable (type {})", - obj.class(), + self.obj.class(), method_name, meth_output.class() )) })?; - Ok(PyList::from(vm.extract_elements(&iter)?).into_pyobject(vm)) - } -} - -impl PyObjectWrap for PyMapping { - fn into_object(self) -> PyObjectRef { - self.0 - } -} - -impl AsRef for PyMapping -where - O: Borrow, -{ - fn as_ref(&self) -> &PyObject { - self.0.borrow() - } -} - -impl IntoPyObject for PyMapping { - fn into_pyobject(self, _vm: &VirtualMachine) -> PyObjectRef { - self.into() - } -} - -impl TryFromObject for PyMapping { - fn try_from_object(vm: &VirtualMachine, mapping: PyObjectRef) -> PyResult { - if Self::check(&mapping, vm) { - Ok(Self::new(mapping)) - } else { - Err(vm.new_type_error(format!("{} is not a mapping object", mapping.class()))) - } + // TODO + // PySequence::from(&iter).list(vm).map(|x| x.into()) + vm.ctx + .new_list(vm.extract_elements(&iter)?) + .into_pyresult(vm) } } diff --git a/vm/src/types/slot.rs b/vm/src/types/slot.rs index d5fb06800a..cabbc76970 100644 --- a/vm/src/types/slot.rs +++ b/vm/src/types/slot.rs @@ -776,43 +776,7 @@ pub trait AsMapping: PyValue { Self::as_mapping(zelf, vm) } - #[inline] - fn downcast(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult> { - zelf.downcast::().map_err(|obj| { - vm.new_type_error(format!( - "{} type is required, not {}", - Self::class(vm), - obj.class() - )) - }) - } - - #[inline] - fn downcast_ref<'a>( - zelf: &'a PyObject, - vm: &VirtualMachine, - ) -> PyResult<&'a PyObjectView> { - zelf.downcast_ref::().ok_or_else(|| { - vm.new_type_error(format!( - "{} type is required, not {}", - Self::class(vm), - zelf.class() - )) - }) - } - fn as_mapping(zelf: &PyObjectView, vm: &VirtualMachine) -> PyMappingMethods; - - fn length(zelf: PyObjectRef, _vm: &VirtualMachine) -> PyResult; - - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult; - - fn ass_subscript( - zelf: PyObjectRef, - needle: PyObjectRef, - value: Option, - vm: &VirtualMachine, - ) -> PyResult<()>; } #[pyimpl] From ca4e8f50e88c2d970a5c4183fe739cdd23782f83 Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Sat, 27 Nov 2021 13:00:37 +0200 Subject: [PATCH 2/9] refactor ItemProtocol -> Object Protocol --- src/lib.rs | 6 +- src/shell/helper.rs | 5 +- stdlib/src/array.rs | 45 +++----- stdlib/src/bisect.rs | 3 +- stdlib/src/dis.rs | 2 +- stdlib/src/pyexpat.rs | 3 +- vm/src/builtins/bytearray.rs | 2 +- vm/src/builtins/bytes.rs | 2 +- vm/src/builtins/dict.rs | 190 ++++++++++++++------------------ vm/src/builtins/enumerate.rs | 3 +- vm/src/builtins/frame.rs | 7 +- vm/src/builtins/function.rs | 14 +-- vm/src/builtins/genericalias.rs | 10 +- vm/src/builtins/iter.rs | 2 +- vm/src/builtins/list.rs | 2 +- vm/src/builtins/mappingproxy.rs | 6 +- vm/src/builtins/module.rs | 3 +- vm/src/builtins/object.rs | 6 +- vm/src/builtins/pystr.rs | 10 +- vm/src/builtins/tuple.rs | 2 +- vm/src/cformat.rs | 7 +- vm/src/dictdatatype.rs | 38 ++++++- vm/src/format.rs | 4 +- vm/src/frame.rs | 114 ++++++------------- vm/src/function.rs | 2 +- vm/src/function/argument.rs | 57 +++++++++- vm/src/import.rs | 2 +- vm/src/protocol/mapping.rs | 81 +++++++++++--- vm/src/protocol/object.rs | 98 +++++++++++++++- vm/src/py_serde.rs | 2 +- vm/src/pyobject.rs | 87 +-------------- vm/src/pyobjectrc.rs | 9 ++ vm/src/scope.rs | 13 +-- vm/src/stdlib/ast.rs | 4 +- vm/src/stdlib/builtins.rs | 25 +++-- vm/src/stdlib/errno.rs | 2 +- vm/src/stdlib/imp.rs | 2 +- vm/src/stdlib/operator.rs | 4 +- vm/src/stdlib/posix.rs | 10 +- vm/src/stdlib/sre.rs | 6 +- vm/src/stdlib/sys.rs | 6 +- vm/src/stdlib/sysconfigdata.rs | 3 +- vm/src/stdlib/thread.rs | 3 +- vm/src/types/slot.rs | 42 ++++--- vm/src/vm.rs | 13 ++- wasm/lib/src/vm_class.rs | 4 +- 46 files changed, 510 insertions(+), 451 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9aca42ee02..a1c2e9aa80 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,8 +46,8 @@ extern crate log; use clap::{App, AppSettings, Arg, ArgMatches}; use rustpython_vm::{ builtins::PyDictRef, builtins::PyInt, compile, match_class, scope::Scope, stdlib::sys, - InitParameter, Interpreter, ItemProtocol, PyObjectRef, PyResult, PySettings, TryFromObject, - TypeProtocol, VirtualMachine, + InitParameter, Interpreter, PyObjectRef, PyResult, PySettings, TryFromObject, TypeProtocol, + VirtualMachine, }; use std::env; @@ -645,7 +645,7 @@ fn run_module(vm: &VirtualMachine, module: &str) -> PyResult<()> { fn get_importer(path: &str, vm: &VirtualMachine) -> PyResult> { let path_importer_cache = vm.sys_module.clone().get_attr("path_importer_cache", vm)?; let path_importer_cache = PyDictRef::try_from_object(vm, path_importer_cache)?; - if let Some(importer) = path_importer_cache.get_item_option(path, vm)? { + if let Some(importer) = path_importer_cache.get_item_opt(path, vm)? { return Ok(Some(importer)); } let path = vm.ctx.new_str(path); diff --git a/src/shell/helper.rs b/src/shell/helper.rs index 360ae60571..03be107e9f 100644 --- a/src/shell/helper.rs +++ b/src/shell/helper.rs @@ -74,10 +74,7 @@ impl<'vm> ShellHelper<'vm> { // last: the last word, could be empty if it ends with a dot // parents: the words before the dot - let mut current = self - .globals - .get_item_option(first.as_str(), self.vm) - .ok()??; + let mut current = self.globals.get_item_opt(first.as_str(), self.vm).ok()??; for attr in parents { current = current.clone().get_attr(attr.as_str(), self.vm).ok()?; diff --git a/stdlib/src/array.rs b/stdlib/src/array.rs index 22189cfda9..71c63b147b 100644 --- a/stdlib/src/array.rs +++ b/stdlib/src/array.rs @@ -1239,35 +1239,26 @@ mod array { }, }; + impl PyArray { + const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + subscript: Some(|mapping, needle, vm| { + mapping.obj_as::().getitem(needle.to_owned(), vm) + }), + ass_subscript: Some(|mapping, needle, value, vm| { + let zelf = mapping.obj_as::(); + if let Some(value) = value { + Self::setitem(zelf.to_owned(), needle.to_owned(), value, vm) + } else { + Self::delitem(zelf.to_owned(), needle.to_owned(), vm) + } + }), + }; + } + impl AsMapping for PyArray { fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - PyMappingMethods { - length: Some(Self::length), - subscript: Some(Self::subscript), - ass_subscript: Some(Self::ass_subscript), - } - } - - fn length(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| Ok(zelf.len()))? - } - - fn subscript(zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Self::downcast_ref(&zelf, vm).map(|zelf| zelf.getitem(needle, vm))? - } - - fn ass_subscript( - zelf: PyObjectRef, - needle: PyObjectRef, - value: Option, - vm: &VirtualMachine, - ) -> PyResult<()> { - match value { - Some(value) => { - Self::downcast(zelf, vm).map(|zelf| Self::setitem(zelf, needle, value, vm))? - } - None => Self::downcast(zelf, vm).map(|zelf| Self::delitem(zelf, needle, vm))?, - } + Self::MAPPING_METHODS } } diff --git a/stdlib/src/bisect.rs b/stdlib/src/bisect.rs index 61dac0eaea..555fa029bd 100644 --- a/stdlib/src/bisect.rs +++ b/stdlib/src/bisect.rs @@ -3,8 +3,7 @@ pub(crate) use _bisect::make_module; #[pymodule] mod _bisect { use crate::vm::{ - function::OptionalArg, types::PyComparisonOp::Lt, ItemProtocol, PyObjectRef, PyResult, - VirtualMachine, + function::OptionalArg, types::PyComparisonOp::Lt, PyObjectRef, PyResult, VirtualMachine, }; #[derive(FromArgs)] diff --git a/stdlib/src/dis.rs b/stdlib/src/dis.rs index 56b7546b78..f69f05667c 100644 --- a/stdlib/src/dis.rs +++ b/stdlib/src/dis.rs @@ -5,7 +5,7 @@ mod decl { use crate::vm::{ builtins::{PyCode, PyDictRef, PyStrRef}, bytecode::CodeFlags, - compile, ItemProtocol, PyObjectRef, PyRef, PyResult, TryFromObject, VirtualMachine, + compile, PyObjectRef, PyRef, PyResult, TryFromObject, VirtualMachine, }; #[pyfunction] diff --git a/stdlib/src/pyexpat.rs b/stdlib/src/pyexpat.rs index f5b0bb8bd8..2aa7a70d58 100644 --- a/stdlib/src/pyexpat.rs +++ b/stdlib/src/pyexpat.rs @@ -35,8 +35,7 @@ mod _pyexpat { builtins::{PyStr, PyStrRef, PyTypeRef}, function::ArgBytesLike, function::{IntoFuncArgs, OptionalArg}, - ItemProtocol, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, - VirtualMachine, + PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, VirtualMachine, }; use rustpython_common::lock::PyRwLock; use std::io::Cursor; diff --git a/vm/src/builtins/bytearray.rs b/vm/src/builtins/bytearray.rs index 13ba0882cd..47d5ec7ec7 100644 --- a/vm/src/builtins/bytearray.rs +++ b/vm/src/builtins/bytearray.rs @@ -679,7 +679,7 @@ impl PyByteArray { impl PyByteArray { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), subscript: Some(|mapping, needle, vm| { mapping.obj_as::().getitem(needle.to_owned(), vm) }), diff --git a/vm/src/builtins/bytes.rs b/vm/src/builtins/bytes.rs index 39e1276052..f262b76abd 100644 --- a/vm/src/builtins/bytes.rs +++ b/vm/src/builtins/bytes.rs @@ -525,7 +525,7 @@ impl PyBytes { impl PyBytes { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), subscript: Some(|mapping, needle, vm| { mapping.obj_as::().getitem(needle.to_owned(), vm) }), diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index cd43a0a0fc..adc8695215 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -13,7 +13,7 @@ use crate::{ PyComparisonOp, Unconstructible, Unhashable, }, vm::{ReprGuard, VirtualMachine}, - IdProtocol, ItemProtocol, + IdProtocol, PyArithmeticValue::*, PyAttributes, PyClassDef, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyObjectView, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, @@ -178,7 +178,7 @@ impl PyDict { (zelf, other) }; for (k, v1) in subset { - match superset.get_item_option(k, vm)? { + match superset.get_item_opt(k, vm)? { Some(v2) => { if v1.is(&v2) { continue; @@ -232,7 +232,7 @@ impl PyDict { #[pymethod(magic)] fn delitem(&self, key: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - self.entries.delete(vm, key) + self.inner_delitem(key, vm) } #[pymethod] @@ -257,12 +257,12 @@ impl PyDict { #[pymethod(magic)] fn setitem(&self, key: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - self.inner_setitem_fast(key, value, vm) + self.inner_setitem(key, value, vm) } /// Set item variant which can be called with multiple /// key types, such as str to name a notable one. - fn inner_setitem_fast( + pub(crate) fn inner_setitem( &self, key: K, value: PyObjectRef, @@ -271,14 +271,18 @@ impl PyDict { self.entries.insert(vm, key, value) } + pub(crate) fn inner_delitem( + &self, + key: K, + vm: &VirtualMachine, + ) -> PyResult<()> { + self.entries.delete(vm, key) + } + #[pymethod(magic)] #[cfg_attr(feature = "flame-it", flame("PyDictRef"))] fn getitem(zelf: PyRef, key: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(value) = zelf.inner_getitem_option(key.clone(), zelf.exact_dict(vm), vm)? { - Ok(value) - } else { - Err(vm.new_key_error(key)) - } + zelf.inner_getitem(key, vm) } #[pymethod] @@ -421,18 +425,15 @@ impl PyDict { } impl PyDict { - const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), - subscript: Some(|mapping, needle, vm| { - let zelf = mapping.obj_as::(); - Self::getitem(zelf.to_owned(), needle.to_owned(), vm) - }), + pub(crate) const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { + length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + subscript: Some(|mapping, needle, vm| mapping.obj_as::().inner_getitem(needle, vm)), ass_subscript: Some(|mapping, needle, value, vm| { let zelf = mapping.obj_as::(); if let Some(value) = value { - zelf.setitem(needle.to_owned(), value, vm) + zelf.inner_setitem(needle, value, vm) } else { - zelf.delitem(needle.to_owned(), vm) + zelf.inner_delitem(needle, vm) } }), }; @@ -472,25 +473,31 @@ impl PyObjectView { self.class().is(&vm.ctx.types.dict_type) } - /// Return an optional inner item, or an error (can be key error as well) + fn missing_opt( + &self, + key: K, + vm: &VirtualMachine, + ) -> Option { + vm.get_method(self.to_owned().into(), "__missing__") + .map(|methods| vm.invoke(&methods?, (key,))) + } + #[inline] - fn inner_getitem_option( + fn inner_getitem( &self, key: K, - exact: bool, vm: &VirtualMachine, - ) -> PyResult> { + ) -> PyResult { if let Some(value) = self.entries.get(vm, &key)? { - return Ok(Some(value)); - } - - if !exact { - if let Some(method_or_err) = vm.get_method(self.to_owned().into(), "__missing__") { - let method = method_or_err?; - return vm.invoke(&method, (key,)).map(Some); - } + Ok(value) + } else if let Some(value) = self.missing_opt(key.clone(), vm) { + value + } else { + Err(vm.new_key_error(key.into_pyobject(vm))) } - Ok(None) + // self.entries + // .get(vm, &key)? + // .ok_or_else(|| vm.new_key_error(key.into_pyobject(vm))) } /// Take a python dictionary and convert it to attributes. @@ -503,50 +510,61 @@ impl PyObjectView { attrs } - /// This function can be used to get an item without raising the - /// KeyError, so we can simply check upon the result being Some - /// python value, or None. - /// Note that we can pass any type which implements the DictKey - /// trait. Notable examples are String and PyObjectRef. - pub fn get_item_option( + pub fn get_item_opt( &self, key: K, vm: &VirtualMachine, ) -> PyResult> { - // Test if this object is a true dict, or maybe a subclass? - // If it is a dict, we can directly invoke inner_get_item_option, - // and prevent the creation of the KeyError exception. - // Also note, that we prevent the creation of a full PyStr object - // if we lookup local names (which happens all of the time). - self._get_item_option_inner(key, self.exact_dict(vm), vm) + if self.exact_dict(vm) { + self.entries.get(vm, &key) + } else { + match self.as_object().get_item(key.clone(), vm) { + Ok(value) => Ok(Some(value)), + Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => { + self.missing_opt(key, vm).transpose() + } + Err(e) => Err(e), + } + } } - #[inline] - fn _get_item_option_inner( + pub fn get_item( &self, key: K, - exact: bool, vm: &VirtualMachine, - ) -> PyResult> { - if exact { - // We can take the short path here! - self.entries.get(vm, &key) + ) -> PyResult { + if self.exact_dict(vm) { + self.inner_getitem(key, vm) } else { - // Fall back to full get_item with KeyError checking - self._subcls_getitem_option(key.into_pyobject(vm), vm) + self.as_object().get_item(key, vm) + // match self.as_object().get_item(key.clone(), vm) { + // Ok(value) => Ok(value), + // Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => { + // self.missing_opt(key, vm).ok_or(e)? + // } + // Err(e) => Err(e), + // } } } - #[cold] - fn _subcls_getitem_option( + pub fn set_item( &self, - key: PyObjectRef, + key: K, + value: PyObjectRef, vm: &VirtualMachine, - ) -> PyResult> { - match self.get_item(key, vm) { - Ok(value) => Ok(Some(value)), - Err(exc) if exc.isinstance(&vm.ctx.exceptions.key_error) => Ok(None), - Err(exc) => Err(exc), + ) -> PyResult<()> { + if self.exact_dict(vm) { + self.inner_setitem(key, value, vm) + } else { + self.as_object().set_item(key, value, vm) + } + } + + pub fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult<()> { + if self.exact_dict(vm) { + self.inner_delitem(key, vm) + } else { + self.as_object().del_item(key, vm) } } @@ -556,60 +574,16 @@ impl PyObjectView { key: K, vm: &VirtualMachine, ) -> PyResult> { - let self_exact = self.class().is(&vm.ctx.types.dict_type); - let other_exact = self.class().is(&vm.ctx.types.dict_type); + 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_option_inner(key.clone(), self_exact, vm)? { + } else if let Some(value) = self.get_item_opt(key.clone(), vm)? { Ok(Some(value)) } else { - other._get_item_option_inner(key, other_exact, vm) - } - } -} - -impl ItemProtocol for PyObjectView -where - K: DictKey + IntoPyObject, -{ - fn get_item(&self, key: K, vm: &VirtualMachine) -> PyResult { - self.as_object().get_item(key, vm) - } - - fn set_item(&self, key: K, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if self.class().is(&vm.ctx.types.dict_type) { - self.inner_setitem_fast(key, value, vm) - } else { - // Fall back to slow path if we are in a dict subclass: - self.as_object().set_item(key, value, vm) + other.get_item_opt(key, vm) } } - - fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult<()> { - if self.class().is(&vm.ctx.types.dict_type) { - self.entries.delete(vm, key) - } else { - // Fall back to slow path if we are in a dict subclass: - self.as_object().del_item(key, vm) - } - } -} - -impl ItemProtocol for PyDictRef -where - K: DictKey + IntoPyObject, -{ - fn get_item(&self, key: K, vm: &VirtualMachine) -> PyResult { - (**self).get_item(key, vm) - } - - fn set_item(&self, key: K, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - (**self).set_item(key, value, vm) - } - - fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult<()> { - (**self).del_item(key, vm) - } } // Implement IntoIterator so that we can easily iterate dictionaries from rust code. diff --git a/vm/src/builtins/enumerate.rs b/vm/src/builtins/enumerate.rs index 699d2d3cbe..2272161800 100644 --- a/vm/src/builtins/enumerate.rs +++ b/vm/src/builtins/enumerate.rs @@ -4,8 +4,7 @@ use crate::{ function::{IntoPyObject, OptionalArg}, protocol::{PyIter, PyIterReturn}, types::{Constructor, IterNext, IterNextIterable}, - ItemProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, - VirtualMachine, + PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, VirtualMachine, }; use num_bigint::BigInt; use num_traits::Zero; diff --git a/vm/src/builtins/frame.rs b/vm/src/builtins/frame.rs index cd338fe4c3..c6d815ab29 100644 --- a/vm/src/builtins/frame.rs +++ b/vm/src/builtins/frame.rs @@ -5,9 +5,8 @@ use super::{PyCode, PyDictRef, PyStrRef}; use crate::{ frame::{Frame, FrameRef}, - protocol::PyMapping, types::{Constructor, Unconstructible}, - IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, VirtualMachine, + IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyObjectWrap, PyRef, PyResult, VirtualMachine, }; pub fn init(context: &PyContext) { @@ -45,8 +44,8 @@ impl FrameRef { } #[pyproperty] - fn f_locals(self, vm: &VirtualMachine) -> PyResult { - self.locals(vm) + fn f_locals(self, vm: &VirtualMachine) -> PyResult { + self.locals(vm).map(|x| x.into_object()) } #[pyproperty] diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs index e579b7f69e..01d655f64c 100644 --- a/vm/src/builtins/function.rs +++ b/vm/src/builtins/function.rs @@ -6,15 +6,15 @@ use super::{ PyTupleRef, PyTypeRef, }; use crate::common::lock::PyMutex; +use crate::function::ArgMapping; use crate::{ bytecode, frame::Frame, function::{FuncArgs, OptionalArg}, - protocol::PyMapping, scope::Scope, types::{Callable, Comparable, Constructor, GetAttr, GetDescriptor, PyComparisonOp}, - IdProtocol, ItemProtocol, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, - PyRef, PyResult, PyValue, TypeProtocol, VirtualMachine, + IdProtocol, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyRef, PyResult, + PyValue, TypeProtocol, VirtualMachine, }; #[cfg(feature = "jit")] use crate::{common::lock::OnceCell, function::IntoPyObject}; @@ -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_option(kwarg.clone(), vm)? { + if let Some(default) = defaults.get_item_opt(kwarg.clone(), vm)? { *slot = Some(default); continue; } @@ -268,7 +268,7 @@ impl PyFunction { pub fn invoke_with_locals( &self, func_args: FuncArgs, - locals: Option, + locals: Option, vm: &VirtualMachine, ) -> PyResult { #[cfg(feature = "jit")] @@ -288,11 +288,11 @@ impl PyFunction { let code = &self.code; let locals = if self.code.flags.contains(bytecode::CodeFlags::NEW_LOCALS) { - PyMapping::new(vm.ctx.new_dict().into()) + ArgMapping::from_dict_exact(vm.ctx.new_dict()) } else if let Some(locals) = locals { locals } else { - PyMapping::new(self.globals.clone().into()) + ArgMapping::from_dict_exact(self.globals.clone()) }; // Construct frame: diff --git a/vm/src/builtins/genericalias.rs b/vm/src/builtins/genericalias.rs index 907206ca31..9e71a04aa9 100644 --- a/vm/src/builtins/genericalias.rs +++ b/vm/src/builtins/genericalias.rs @@ -4,8 +4,8 @@ use crate::{ function::{FuncArgs, IntoPyObject}, protocol::PyMappingMethods, types::{AsMapping, Callable, Comparable, Constructor, GetAttr, Hashable, PyComparisonOp}, - IdProtocol, ItemProtocol, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, - PyObjectView, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, VirtualMachine, + IdProtocol, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyObjectView, + PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, VirtualMachine, }; use std::fmt; @@ -238,7 +238,7 @@ fn subs_tvars( } impl PyGenericAlias { - fn getitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { + fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { let num_params = self.parameters.len(); if num_params == 0 { return Err(vm.new_type_error(format!( @@ -291,7 +291,9 @@ impl PyGenericAlias { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { length: None, - subscript: Some(|mapping, needle, vm| mapping.obj_as::().getitem(needle, vm)), + subscript: Some(|mapping, needle, vm| { + mapping.obj_as::().getitem(needle.to_owned(), vm) + }), ass_subscript: None, }; } diff --git a/vm/src/builtins/iter.rs b/vm/src/builtins/iter.rs index 909ac3816d..7a08a194cb 100644 --- a/vm/src/builtins/iter.rs +++ b/vm/src/builtins/iter.rs @@ -7,7 +7,7 @@ use crate::{ function::ArgCallable, protocol::PyIterReturn, types::{IterNext, IterNextIterable}, - ItemProtocol, PyClassImpl, PyContext, PyObject, PyObjectRef, PyResult, PyValue, VirtualMachine, + PyClassImpl, PyContext, PyObject, PyObjectRef, PyResult, PyValue, VirtualMachine, }; use rustpython_common::{ lock::{PyMutex, PyRwLock, PyRwLockUpgradableReadGuard}, diff --git a/vm/src/builtins/list.rs b/vm/src/builtins/list.rs index 8702668507..21618f03f6 100644 --- a/vm/src/builtins/list.rs +++ b/vm/src/builtins/list.rs @@ -391,7 +391,7 @@ impl<'a> MutObjectSequenceOp<'a> for PyList { impl PyList { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), subscript: Some(|mapping, needle, vm| { let zelf = mapping.obj_as::(); Self::getitem(zelf.to_owned(), needle.to_owned(), vm) diff --git a/vm/src/builtins/mappingproxy.rs b/vm/src/builtins/mappingproxy.rs index 4134f836bc..6c8f89c1c5 100644 --- a/vm/src/builtins/mappingproxy.rs +++ b/vm/src/builtins/mappingproxy.rs @@ -3,8 +3,8 @@ use crate::{ function::{IntoPyObject, OptionalArg}, protocol::{PyMapping, PyMappingMethods}, types::{AsMapping, Constructor, Iterable}, - ItemProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, - TypeProtocol, VirtualMachine, + PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, + VirtualMachine, }; #[pyclass(module = false, name = "mappingproxy")] @@ -37,7 +37,7 @@ impl Constructor for PyMappingProxy { type Args = PyObjectRef; fn py_new(cls: PyTypeRef, mapping: Self::Args, vm: &VirtualMachine) -> PyResult { - if !PyMapping::check(&mapping, vm) + if !PyMapping::from(mapping.as_ref()).has_protocol(vm) || mapping.payload_if_subclass::(vm).is_some() || mapping.payload_if_subclass::(vm).is_some() { diff --git a/vm/src/builtins/module.rs b/vm/src/builtins/module.rs index 79a9399e3c..17c42aea5b 100644 --- a/vm/src/builtins/module.rs +++ b/vm/src/builtins/module.rs @@ -3,8 +3,7 @@ use super::{PyDictRef, PyStr, PyStrRef, PyTypeRef}; use crate::{ function::{FuncArgs, IntoPyObject}, types::GetAttr, - ItemProtocol, PyClassImpl, PyContext, PyObjectRef, PyObjectView, PyRef, PyResult, PyValue, - VirtualMachine, + PyClassImpl, PyContext, PyObjectRef, PyObjectView, PyRef, PyResult, PyValue, VirtualMachine, }; #[pyclass(module = false, name = "module")] diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index 14f8019865..a5d250cb12 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -1,9 +1,9 @@ use super::{PyDict, PyDictRef, PyList, PyStr, PyStrRef, PyType, PyTypeRef}; use crate::common::hash::PyHash; use crate::{ - function::FuncArgs, types::PyComparisonOp, utils::Either, IdProtocol, ItemProtocol, - PyArithmeticValue, PyAttributes, PyClassImpl, PyComparisonValue, PyContext, PyObject, - PyObjectRef, PyResult, PyValue, TypeProtocol, VirtualMachine, + function::FuncArgs, types::PyComparisonOp, utils::Either, IdProtocol, PyArithmeticValue, + PyAttributes, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, PyResult, + PyValue, TypeProtocol, VirtualMachine, }; /// object() diff --git a/vm/src/builtins/pystr.rs b/vm/src/builtins/pystr.rs index 5efa26c532..d743a55fff 100644 --- a/vm/src/builtins/pystr.rs +++ b/vm/src/builtins/pystr.rs @@ -15,8 +15,8 @@ use crate::{ PyComparisonOp, Unconstructible, }, utils::Either, - IdProtocol, ItemProtocol, PyClassDef, PyClassImpl, PyComparisonValue, PyContext, PyObject, - PyObjectRef, PyObjectView, PyRef, PyResult, PyValue, TypeProtocol, VirtualMachine, + IdProtocol, PyClassDef, PyClassImpl, PyComparisonValue, PyContext, PyObject, PyObjectRef, + PyObjectView, PyRef, PyResult, PyValue, TypeProtocol, VirtualMachine, }; use ascii::{AsciiStr, AsciiString}; use bstr::ByteSlice; @@ -1260,17 +1260,17 @@ impl Iterable for PyStr { impl AsMapping for PyStr { fn as_mapping(_zelf: &PyObjectView, _vm: &VirtualMachine) -> PyMappingMethods { - Self::MAPPING_METHODS.clone() + Self::MAPPING_METHODS } } impl PyStr { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Self::SEQUENCE_METHDOS.length, + length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), subscript: Some(|mapping, needle, vm| { mapping .obj_as::() - ._getitem(needle, vm) + .getitem(needle.to_owned(), vm) .map(|x| x.into_ref(vm).into()) }), ass_subscript: None, diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index abffd688b5..81b75d9804 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -310,7 +310,7 @@ impl PyTuple { impl PyTuple { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), subscript: Some(|mapping, needle, vm| { let zelf = mapping.obj_as::(); Self::getitem(zelf.to_owned(), needle.to_owned(), vm) diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index ddae2d5421..bcf2adb101 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -7,8 +7,7 @@ use crate::{ function::ArgIntoFloat, protocol::PyBuffer, stdlib::builtins, - ItemProtocol, PyObjectRef, PyResult, TryFromBorrowedObject, TryFromObject, TypeProtocol, - VirtualMachine, + PyObjectRef, PyResult, TryFromBorrowedObject, TryFromObject, TypeProtocol, VirtualMachine, }; use itertools::Itertools; use num_bigint::{BigInt, Sign}; @@ -725,7 +724,7 @@ impl CFormatBytes { CFormatPart::Literal(literal) => result.append(literal), CFormatPart::Spec(spec) => { let value = match &spec.mapping_key { - Some(key) => values_obj.get_item(key, vm)?, + Some(key) => values_obj.get_item(key.as_str(), vm)?, None => unreachable!(), }; let mut part_result = spec.bytes_format(vm, value)?; @@ -876,7 +875,7 @@ impl CFormatString { CFormatPart::Literal(literal) => result.push_str(literal), CFormatPart::Spec(spec) => { let value = match &spec.mapping_key { - Some(key) => values_obj.get_item(key, vm)?, + Some(key) => values_obj.get_item(key.as_str(), vm)?, None => unreachable!(), }; let part_result = spec.string_format(vm, value, idx)?; diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index 4fe19d0140..e29b95b1ae 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -1,16 +1,21 @@ -use crate::common::{ - hash, - lock::{PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard}, -}; +use num_traits::ToPrimitive; + /// Ordered dictionary implementation. /// Inspired by: https://morepypy.blogspot.com/2015/01/faster-more-memory-efficient-and-more.html /// And: https://www.youtube.com/watch?v=p33CVV29OG8 /// And: http://code.activestate.com/recipes/578375/ use crate::{ - builtins::{PyStr, PyStrRef}, + builtins::{PyInt, PyStr, PyStrRef}, function::IntoPyObject, IdProtocol, PyObject, PyObjectRef, PyRefExact, PyResult, TypeProtocol, VirtualMachine, }; +use crate::{ + common::{ + hash, + lock::{PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard}, + }, + PyObjectWrap, +}; use std::fmt; use std::mem::size_of; use std::ops::ControlFlow; @@ -783,6 +788,29 @@ impl DictKey for String { } } +impl DictKey for usize { + fn key_hash(&self, vm: &VirtualMachine) -> PyResult { + Ok(vm.state.hash_secret.hash_value(self)) + } + + fn key_is(&self, _other: &PyObject) -> bool { + false + } + + fn key_eq(&self, vm: &VirtualMachine, other_key: &PyObject) -> PyResult { + if let Some(int) = other_key.payload_if_exact::(vm) { + if let Some(i) = int.as_bigint().to_usize() { + Ok(i == *self) + } else { + Ok(false) + } + } else { + let int = vm.ctx.new_int(*self); + vm.bool_eq(&int.into_object(), other_key) + } + } +} + fn str_exact<'a>(obj: &'a PyObject, vm: &VirtualMachine) -> Option<&'a PyStr> { if obj.class().is(&vm.ctx.types.str_type) { obj.payload::() diff --git a/vm/src/format.rs b/vm/src/format.rs index d0f7ac1170..4351cd8ffe 100644 --- a/vm/src/format.rs +++ b/vm/src/format.rs @@ -3,7 +3,7 @@ use crate::{ common::float_ops, function::{FuncArgs, IntoPyException}, stdlib::builtins, - ItemProtocol, PyObject, PyObjectRef, PyResult, TypeProtocol, VirtualMachine, + PyObject, PyObjectRef, PyResult, TypeProtocol, VirtualMachine, }; use itertools::{Itertools, PeekingNext}; use num_bigint::{BigInt, Sign}; @@ -823,7 +823,7 @@ impl FormatString { argument = argument.get_item(index, vm)?; } FieldNamePart::StringIndex(index) => { - argument = argument.get_item(&index, vm)?; + argument = argument.get_item(index, vm)?; } } } diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 0ce21d5579..2de8f80080 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -10,13 +10,13 @@ use crate::{ bytecode, coroutine::Coro, exceptions::ExceptionCtor, - function::{FuncArgs, IntoPyResult}, - protocol::{PyIter, PyIterReturn, PyMapping}, + function::{ArgMapping, FuncArgs, IntoPyResult}, + protocol::{PyIter, PyIterReturn}, scope::Scope, stdlib::builtins, types::PyComparisonOp, - IdProtocol, ItemProtocol, PyMethod, PyObject, PyObjectRef, PyObjectWrap, PyRef, PyResult, - PyValue, TryFromObject, TypeProtocol, VirtualMachine, + IdProtocol, PyMethod, PyObject, PyObjectRef, PyObjectWrap, PyRef, PyResult, PyValue, + TryFromObject, TypeProtocol, VirtualMachine, }; use indexmap::IndexMap; use itertools::Itertools; @@ -99,7 +99,7 @@ pub struct Frame { pub fastlocals: PyMutex]>>, pub(crate) cells_frees: Box<[PyCellRef]>, - pub locals: PyMapping, + pub locals: ArgMapping, pub globals: PyDictRef, pub builtins: PyDictRef, @@ -179,7 +179,7 @@ impl FrameRef { f(exec) } - pub fn locals(&self, vm: &VirtualMachine) -> PyResult { + pub fn locals(&self, vm: &VirtualMachine) -> PyResult { let locals = &self.locals; let code = &**self.code; let map = &code.varnames; @@ -187,38 +187,20 @@ impl FrameRef { if !code.varnames.is_empty() { let fastlocals = self.fastlocals.lock(); for (k, v) in itertools::zip(&map[..j], &**fastlocals) { - if let Some(v) = v { - match locals.as_object().to_owned().downcast_exact::(vm) { - Ok(d) => d.set_item(k.clone(), v.clone(), vm)?, - Err(o) => o.set_item(k.clone(), v.clone(), vm)?, - }; - } else { - let res = match locals.as_object().to_owned().downcast_exact::(vm) { - Ok(d) => d.del_item(k.clone(), vm), - Err(o) => o.del_item(k.clone(), vm), - }; - match res { - Ok(()) => {} - Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => {} - Err(e) => return Err(e), - } + match locals.mapping().ass_subscript(k.as_object(), v.clone(), vm) { + Ok(()) => {} + Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => {} + Err(e) => return Err(e), } } } if !code.cellvars.is_empty() || !code.freevars.is_empty() { let map_to_dict = |keys: &[PyStrRef], values: &[PyCellRef]| { for (k, v) in itertools::zip(keys, values) { - if let Some(v) = v.get() { - match locals.as_object().to_owned().downcast_exact::(vm) { - Ok(d) => d.set_item(k.clone(), v, vm)?, - Err(o) => o.set_item(k.clone(), v, vm)?, - }; + if let Some(value) = v.get() { + locals.as_object().set_item(k.clone(), value, vm)?; } else { - let res = match locals.as_object().to_owned().downcast_exact::(vm) { - Ok(d) => d.del_item(k.clone(), vm), - Err(o) => o.del_item(k.clone(), vm), - }; - match res { + match locals.as_object().del_item(k.clone(), vm) { Ok(()) => {} Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => {} Err(e) => return Err(e), @@ -289,7 +271,7 @@ struct ExecutingFrame<'a> { code: &'a PyRef, fastlocals: &'a PyMutex]>>, cells_frees: &'a [PyCellRef], - locals: &'a PyMapping, + locals: &'a ArgMapping, globals: &'a PyDictRef, builtins: &'a PyDictRef, object: &'a FrameRef, @@ -514,17 +496,8 @@ impl ExecutingFrame<'_> { } bytecode::Instruction::LoadNameAny(idx) => { let name = &self.code.names[*idx as usize]; - // Try using locals as dict first, if not, fallback to generic method. - let x = match self - .locals - .clone() - .into_object() - .downcast_exact::(vm) - { - Ok(d) => d.get_item_option(name.clone(), vm)?, - Err(o) => o.get_item(name.clone(), vm).ok(), - }; - self.push_value(match x { + let value = self.locals.mapping().subscript(name.as_object(), vm).ok(); + self.push_value(match value { Some(x) => x, None => self.load_global_or_builtin(name, vm)?, }); @@ -547,16 +520,7 @@ impl ExecutingFrame<'_> { bytecode::Instruction::LoadClassDeref(i) => { let i = *i as usize; let name = self.code.freevars[i - self.code.cellvars.len()].clone(); - // Try using locals as dict first, if not, fallback to generic method. - let value = match self - .locals - .clone() - .into_object() - .downcast_exact::(vm) - { - Ok(d) => d.get_item_option(name, vm)?, - Err(o) => o.get_item(name, vm).ok(), - }; + let value = self.locals.mapping().subscript(name.as_object(), vm).ok(); self.push_value(match value { Some(v) => v, None => self.cells_frees[i] @@ -571,16 +535,11 @@ impl ExecutingFrame<'_> { Ok(None) } bytecode::Instruction::StoreLocal(idx) => { + let name = &self.code.names[*idx as usize]; let value = self.pop_value(); - match self - .locals - .clone() - .into_object() - .downcast_exact::(vm) - { - Ok(d) => d.set_item(self.code.names[*idx as usize].clone(), value, vm)?, - Err(o) => o.set_item(self.code.names[*idx as usize].clone(), value, vm)?, - }; + self.locals + .mapping() + .ass_subscript(name.as_object(), Some(value), vm)?; Ok(None) } bytecode::Instruction::StoreGlobal(idx) => { @@ -600,15 +559,10 @@ impl ExecutingFrame<'_> { } bytecode::Instruction::DeleteLocal(idx) => { let name = &self.code.names[*idx as usize]; - let res = match self + let res = self .locals - .clone() - .into_object() - .downcast_exact::(vm) - { - Ok(d) => d.del_item(name.clone(), vm), - Err(o) => o.del_item(name.clone(), vm), - }; + .mapping() + .ass_subscript(name.as_object(), None, vm); match res { Ok(()) => {} @@ -1204,18 +1158,12 @@ impl ExecutingFrame<'_> { } else { Box::new(|name| !name.starts_with('_')) }; - for (k, v) in &dict { + for (k, v) in dict { let k = PyStrRef::try_from_object(vm, k)?; if filter_pred(k.as_str()) { - match self - .locals - .clone() - .into_object() - .downcast_exact::(vm) - { - Ok(d) => d.set_item(k, v, vm)?, - Err(o) => o.set_item(k, v, vm)?, - }; + self.locals + .mapping() + .ass_subscript(k.as_object(), Some(v), vm)?; } } } @@ -1664,7 +1612,7 @@ impl ExecutingFrame<'_> { let name = qualified_name.as_str().split('.').next_back().unwrap(); func_obj.set_attr("__name__", vm.new_pyobj(name), vm)?; func_obj.set_attr("__qualname__", qualified_name, vm)?; - let module = vm.unwrap_or_none(self.globals.get_item_option("__name__", vm)?); + let module = vm.unwrap_or_none(self.globals.get_item_opt("__name__", vm)?); func_obj.set_attr("__module__", module, vm)?; func_obj.set_attr("__annotations__", annotations, vm)?; @@ -1910,7 +1858,9 @@ impl fmt::Debug for Frame { write!( f, "Frame Object {{ \n Stack:{}\n Blocks:{}\n Locals:{:?}\n}}", - stack_str, block_str, locals + stack_str, + block_str, + locals.into_object() ) } } diff --git a/vm/src/function.rs b/vm/src/function.rs index 10e8354360..9d1e1172b0 100644 --- a/vm/src/function.rs +++ b/vm/src/function.rs @@ -12,7 +12,7 @@ use itertools::Itertools; use std::marker::PhantomData; use std::ops::RangeInclusive; -pub use argument::{ArgCallable, ArgIterable}; +pub use argument::{ArgCallable, ArgIterable, ArgMapping}; pub use buffer::{ArgAsciiBuffer, ArgBytesLike, ArgMemoryBuffer, ArgStrOrBytesLike}; pub use number::{ArgIntoBool, ArgIntoComplex, ArgIntoFloat}; diff --git a/vm/src/function/argument.rs b/vm/src/function/argument.rs index 34a7a9cdd2..8baf535a5d 100644 --- a/vm/src/function/argument.rs +++ b/vm/src/function/argument.rs @@ -1,7 +1,10 @@ -use super::IntoFuncArgs; +use super::{IntoFuncArgs, IntoPyObject}; use crate::{ - builtins::iter::PySequenceIterator, protocol::PyIter, protocol::PyIterIter, PyObject, - PyObjectRef, PyObjectWrap, PyResult, PyValue, TryFromObject, TypeProtocol, VirtualMachine, + builtins::{iter::PySequenceIterator, PyDict, PyDictRef}, + protocol::PyIter, + protocol::{PyIterIter, PyMapping, PyMappingMethods}, + PyObject, PyObjectRef, PyObjectWrap, PyResult, PyValue, TryFromObject, TypeProtocol, + VirtualMachine, }; use std::marker::PhantomData; @@ -86,3 +89,51 @@ where }) } } + +#[derive(Clone)] +pub struct ArgMapping { + obj: PyObjectRef, + mapping_methods: PyMappingMethods, +} + +impl ArgMapping { + pub fn from_dict_exact(dict: PyDictRef) -> Self { + Self { + obj: dict.into(), + mapping_methods: PyDict::MAPPING_METHODS, + } + } + + pub fn mapping(&self) -> PyMapping { + PyMapping::with_methods(&self.obj, self.mapping_methods) + } +} + +impl AsRef for ArgMapping { + fn as_ref(&self) -> &PyObject { + &self.obj + } +} + +impl PyObjectWrap for ArgMapping { + fn into_object(self) -> PyObjectRef { + self.obj + } +} + +impl IntoPyObject for ArgMapping { + fn into_pyobject(self, _vm: &VirtualMachine) -> PyObjectRef { + self.obj + } +} + +impl TryFromObject for ArgMapping { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + let mapping = PyMapping::try_protocol(&obj, vm)?; + let mapping_methods = *mapping.methods(vm); + Ok(Self { + obj, + mapping_methods, + }) + } +} diff --git a/vm/src/import.rs b/vm/src/import.rs index 435a6d4548..215ab65cd7 100644 --- a/vm/src/import.rs +++ b/vm/src/import.rs @@ -8,7 +8,7 @@ use crate::{ scope::Scope, version::get_git_revision, vm::{InitParameter, VirtualMachine}, - ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, + PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; use rand::Rng; diff --git a/vm/src/protocol/mapping.rs b/vm/src/protocol/mapping.rs index b0fc7d2194..733bb5745e 100644 --- a/vm/src/protocol/mapping.rs +++ b/vm/src/protocol/mapping.rs @@ -4,6 +4,7 @@ use crate::{ PyDict, }, common::lock::OnceCell, + function::IntoPyResult, IdProtocol, PyObject, PyObjectPayload, PyObjectRef, PyObjectView, PyResult, TypeProtocol, VirtualMachine, }; @@ -19,14 +20,14 @@ pub struct PyMappingMethods { Option, &VirtualMachine) -> PyResult<()>>, } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct PyMapping<'a> { pub obj: &'a PyObject, methods: OnceCell, } -impl From<&PyObject> for PyMapping<'_> { - fn from(obj: &PyObject) -> Self { +impl<'a> From<&'a PyObject> for PyMapping<'a> { + fn from(obj: &'a PyObject) -> Self { Self { obj, methods: OnceCell::new(), @@ -40,12 +41,36 @@ impl AsRef for PyMapping<'_> { } } +impl<'a> PyMapping<'a> { + pub fn with_methods(obj: &'a PyObject, methods: PyMappingMethods) -> Self { + Self { + obj, + methods: OnceCell::from(methods), + } + } + + pub fn try_protocol(obj: &'a PyObject, vm: &VirtualMachine) -> PyResult { + let zelf = Self::from(obj); + if zelf.has_protocol(vm) { + Ok(zelf) + } else { + Err(vm.new_type_error(format!("{} is not a mapping object", zelf.obj.class()))) + } + } +} + impl PyMapping<'_> { + // PyMapping::Check + pub fn has_protocol(&self, vm: &VirtualMachine) -> bool { + self.methods(vm).subscript.is_some() + } + pub fn obj_as(&self) -> &PyObjectView { unsafe { self.obj.downcast_unchecked_ref::() } } pub fn methods(&self, vm: &VirtualMachine) -> &PyMappingMethods { + // let cls = self.obj.class(); self.methods.get_or_init(|| { if let Some(f) = self .obj @@ -57,19 +82,20 @@ impl PyMapping<'_> { PyMappingMethods::default() } }) - } - - // PyMapping::Check - pub fn has_protocol(&self, vm: &VirtualMachine) -> bool { - self.methods(vm).subscript.is_some() - } - pub fn try_protocol(&self, vm: &VirtualMachine) -> PyResult<()> { - if self.has_protocol(vm) { - Ok(()) - } else { - Err(vm.new_type_error(format!("{} is not a mapping object", self.obj.class()))) - } + // let get_methods = || { + // if let Some(f) = cls.mro_find_map(|cls| cls.slots.as_mapping.load()) { + // f(self.obj, vm) + // } else { + // PyMappingMethods::default() + // } + // }; + + // if cls.slots.flags.has_feature(PyTypeFlags::HEAPTYPE) { + // &get_methods() + // } else { + // self.methods.get_or_init(get_methods) + // } } pub fn length_opt(&self, vm: &VirtualMachine) -> Option> { @@ -80,11 +106,34 @@ impl PyMapping<'_> { self.length_opt(vm).ok_or_else(|| { vm.new_type_error(format!( "object of type '{}' has no len() or not a mapping", - self.0.class().name() + self.obj.class() )) })? } + pub fn subscript(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult { + let f = self + .methods(vm) + .subscript + .ok_or_else(|| vm.new_type_error(format!("{} is not a mapping", self.obj.class())))?; + f(self, needle, vm) + } + + pub fn ass_subscript( + &self, + needle: &PyObject, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + let f = self.methods(vm).ass_subscript.ok_or_else(|| { + vm.new_type_error(format!( + "'{}' object does not support item assignment", + self.obj.class() + )) + })?; + f(self, needle, value, vm) + } + pub fn keys(&self, vm: &VirtualMachine) -> PyResult { if let Some(dict) = self.obj.downcast_ref_if_exact::(vm) { PyDictKeys::new(dict.to_owned()).into_pyresult(vm) diff --git a/vm/src/protocol/object.rs b/vm/src/protocol/object.rs index 8dedb46192..b48f68b64d 100644 --- a/vm/src/protocol/object.rs +++ b/vm/src/protocol/object.rs @@ -2,11 +2,15 @@ //! https://docs.python.org/3/c-api/object.html use crate::{ - builtins::{pystr::IntoPyStrRef, PyBytes, PyInt, PyStrRef, PyTupleRef, PyTypeRef}, + builtins::{ + pystr::IntoPyStrRef, PyBytes, PyDict, PyGenericAlias, PyInt, PyStrRef, PyTupleRef, + PyTypeRef, + }, bytesinner::ByteInnerNewOptions, common::{hash::PyHash, str::to_ascii}, - function::{IntoPyObject, OptionalArg}, - protocol::PyIter, + dictdatatype::DictKey, + function::{IntoPyObject, IntoPyResult, OptionalArg}, + protocol::{PyIter, PyMapping}, pyref_type_error, types::{Constructor, PyComparisonOp}, utils::Either, @@ -411,4 +415,92 @@ impl PyObject { ))) }) } + + pub fn get_item( + &self, + needle: K, + vm: &VirtualMachine, + ) -> PyResult { + if let Some(dict) = self.downcast_ref_if_exact::(vm) { + return dict.get_item(needle, vm); + } + + let needle = needle.into_pyobject(vm); + + if let Ok(mapping) = PyMapping::try_protocol(self, vm) { + mapping.subscript(&needle, vm) + } else { + // TODO: sequence protocol + match vm.get_special_method(self.to_owned(), "__getitem__")? { + Ok(special_method) => return special_method.invoke((needle,), vm), + Err(obj) => { + if obj.class().issubclass(&vm.ctx.types.type_type) { + if obj.is(&vm.ctx.types.type_type) { + return PyGenericAlias::new(obj.clone_class(), needle, vm) + .into_pyresult(vm); + } + + if let Some(class_getitem) = + vm.get_attribute_opt(obj, "__class_getitem__")? + { + return vm.invoke(&class_getitem, (needle,)); + } + } + } + } + Err(vm.new_type_error(format!("'{}' object is not subscriptable", self.class()))) + } + } + + pub fn set_item( + &self, + needle: K, + value: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult<()> { + if let Some(dict) = self.downcast_ref_if_exact::(vm) { + return dict.set_item(needle, value, vm); + } + + let needle = needle.into_pyobject(vm); + + if let Ok(mapping) = PyMapping::try_protocol(self, vm) { + mapping.ass_subscript(&needle, Some(value), vm) + } else { + // TODO: sequence protocol + vm.get_special_method(self.to_owned(), "__setitem__")? + .map_err(|obj| { + vm.new_type_error(format!( + "'{}' does not support item assignment", + obj.class() + )) + })? + .invoke((needle, value), vm)?; + Ok(()) + } + } + + pub fn del_item( + &self, + needle: K, + vm: &VirtualMachine, + ) -> PyResult<()> { + if let Some(dict) = self.downcast_ref_if_exact::(vm) { + return dict.del_item(needle, vm); + } + + let needle = needle.into_pyobject(vm); + + if let Ok(mapping) = PyMapping::try_protocol(self, vm) { + mapping.ass_subscript(&needle, None, vm) + } else { + //TODO: sequence protocol + vm.get_special_method(self.to_owned(), "__delitem__")? + .map_err(|obj| { + vm.new_type_error(format!("'{}' does not support item deletion", obj.class())) + })? + .invoke((needle,), vm)?; + Ok(()) + } + } } diff --git a/vm/src/py_serde.rs b/vm/src/py_serde.rs index 34e59c0cc7..796d17316a 100644 --- a/vm/src/py_serde.rs +++ b/vm/src/py_serde.rs @@ -4,7 +4,7 @@ use serde::de::{DeserializeSeed, Visitor}; use serde::ser::{Serialize, SerializeMap, SerializeSeq}; use crate::builtins::{dict::PyDictRef, float, int, list::PyList, pybool, tuple::PyTuple, PyStr}; -use crate::{ItemProtocol, PyObject, PyObjectRef, TypeProtocol, VirtualMachine}; +use crate::{PyObject, PyObjectRef, TypeProtocol, VirtualMachine}; #[inline] pub fn serialize( diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index e6f0abd5e2..19dbc1495a 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -12,13 +12,12 @@ use crate::{ bytes, getset::{IntoPyGetterFunc, IntoPySetterFunc, PyGetSet}, object, pystr, PyBaseException, PyBaseExceptionRef, PyBoundMethod, PyDict, PyDictRef, - PyEllipsis, PyFloat, PyFrozenSet, PyGenericAlias, PyInt, PyIntRef, PyList, PyListRef, - PyNone, PyNotImplemented, PyStr, PyTuple, PyTupleRef, PyType, PyTypeRef, + PyEllipsis, PyFloat, PyFrozenSet, PyInt, PyIntRef, PyList, PyListRef, PyNone, + PyNotImplemented, PyStr, PyTuple, PyTupleRef, PyType, PyTypeRef, }, dictdatatype::Dict, exceptions, function::{IntoFuncArgs, IntoPyNativeFunc, IntoPyObject, IntoPyResult}, - protocol::PyMapping, types::TypeZoo, types::{PyTypeFlags, PyTypeSlots}, VirtualMachine, @@ -593,86 +592,6 @@ impl TypeProtocol for &'_ T { } } -/// The python item protocol. Mostly applies to dictionaries. -/// Allows getting, setting and deletion of keys-value pairs. -pub trait ItemProtocol -where - T: IntoPyObject + ?Sized, -{ - fn get_item(&self, key: T, vm: &VirtualMachine) -> PyResult; - fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()>; - fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()>; -} - -impl ItemProtocol for PyObject -where - T: IntoPyObject, -{ - fn get_item(&self, key: T, vm: &VirtualMachine) -> PyResult { - if let Ok(mapping) = PyMapping::try_from_object(vm, self.to_owned()) { - if let Some(getitem) = mapping.methods(vm).subscript { - return getitem(self.to_owned(), key.into_pyobject(vm), vm); - } - } - - match vm.get_special_method(self.to_owned(), "__getitem__")? { - Ok(special_method) => return special_method.invoke((key,), vm), - Err(obj) => { - if obj.class().issubclass(&vm.ctx.types.type_type) { - if obj.is(&vm.ctx.types.type_type) { - return PyGenericAlias::new(obj.clone_class(), key.into_pyobject(vm), vm) - .into_pyresult(vm); - } - - if let Some(class_getitem) = vm.get_attribute_opt(obj, "__class_getitem__")? { - return vm.invoke(&class_getitem, (key,)); - } - } - } - } - Err(vm.new_type_error(format!( - "'{}' object is not subscriptable", - self.class().name() - ))) - } - - fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if let Ok(mapping) = PyMapping::try_from_object(vm, self.to_owned()) { - if let Some(setitem) = mapping.methods(vm).ass_subscript { - return setitem(self.to_owned(), key.into_pyobject(vm), Some(value), vm); - } - } - - vm.get_special_method(self.to_owned(), "__setitem__")? - .map_err(|obj| { - vm.new_type_error(format!( - "'{}' does not support item assignment", - obj.class().name() - )) - })? - .invoke((key, value), vm)?; - Ok(()) - } - - fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()> { - if let Ok(mapping) = PyMapping::try_from_object(vm, self.to_owned()) { - if let Some(setitem) = mapping.methods(vm).ass_subscript { - return setitem(self.to_owned(), key.into_pyobject(vm), None, vm); - } - } - - vm.get_special_method(self.to_owned(), "__delitem__")? - .map_err(|obj| { - vm.new_type_error(format!( - "'{}' does not support item deletion", - obj.class().name() - )) - })? - .invoke((key,), vm)?; - Ok(()) - } -} - impl TryFromObject for PyObjectRef { #[inline] fn try_from_object(_vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { @@ -1165,7 +1084,7 @@ impl PyMethod { }; if let Some(dict) = obj.dict() { - if let Some(attr) = dict.get_item_option(name.clone(), vm)? { + if let Some(attr) = dict.get_item_opt(name.clone(), vm)? { return Ok(Self::Attribute(attr)); } } diff --git a/vm/src/pyobjectrc.rs b/vm/src/pyobjectrc.rs index d6ebc3ccda..4dd10ec047 100644 --- a/vm/src/pyobjectrc.rs +++ b/vm/src/pyobjectrc.rs @@ -675,6 +675,15 @@ impl PyObject { } } + pub fn downcast_ref_if_exact( + &self, + vm: &VirtualMachine, + ) -> Option<&PyObjectView> { + self.class() + .is(T::class(vm)) + .then(|| unsafe { self.downcast_unchecked_ref::() }) + } + /// # Safety /// T must be the exact payload type pub unsafe fn downcast_unchecked_ref(&self) -> &crate::PyObjectView { diff --git a/vm/src/scope.rs b/vm/src/scope.rs index 30d833e4a2..f9113ba36d 100644 --- a/vm/src/scope.rs +++ b/vm/src/scope.rs @@ -1,14 +1,13 @@ use crate::{ builtins::{pystr::IntoPyStrRef, PyDictRef, PyStrRef}, - function::IntoPyObject, - protocol::PyMapping, - ItemProtocol, VirtualMachine, + function::{ArgMapping, IntoPyObject}, + VirtualMachine, }; use std::fmt; #[derive(Clone)] pub struct Scope { - pub locals: PyMapping, + pub locals: ArgMapping, pub globals: PyDictRef, } @@ -21,13 +20,13 @@ impl fmt::Debug for Scope { impl Scope { #[inline] - pub fn new(locals: Option, globals: PyDictRef) -> Scope { - let locals = locals.unwrap_or_else(|| PyMapping::new(globals.clone().into())); + pub fn new(locals: Option, globals: PyDictRef) -> Scope { + let locals = locals.unwrap_or_else(|| ArgMapping::from_dict_exact(globals.clone())); Scope { locals, globals } } pub fn with_builtins( - locals: Option, + locals: Option, globals: PyDictRef, vm: &VirtualMachine, ) -> Scope { diff --git a/vm/src/stdlib/ast.rs b/vm/src/stdlib/ast.rs index 6d9aadb999..94a56e57ea 100644 --- a/vm/src/stdlib/ast.rs +++ b/vm/src/stdlib/ast.rs @@ -5,8 +5,8 @@ use crate::{ builtins::{self, PyStrRef, PyTypeRef}, - IdProtocol, ItemProtocol, PyClassImpl, PyContext, PyObject, PyObjectRef, PyResult, PyValue, - StaticType, TryFromObject, TypeProtocol, VirtualMachine, + IdProtocol, PyClassImpl, PyContext, PyObject, PyObjectRef, PyResult, PyValue, StaticType, + TryFromObject, TypeProtocol, VirtualMachine, }; use num_complex::Complex64; use num_traits::{ToPrimitive, Zero}; diff --git a/vm/src/stdlib/builtins.rs b/vm/src/stdlib/builtins.rs index 630aaa7729..ea5404a100 100644 --- a/vm/src/stdlib/builtins.rs +++ b/vm/src/stdlib/builtins.rs @@ -22,18 +22,18 @@ mod builtins { }, common::{hash::PyHash, str::to_ascii}, function::{ - ArgBytesLike, ArgCallable, ArgIntoBool, ArgIterable, FuncArgs, KwArgs, OptionalArg, - OptionalOption, PosArgs, + ArgBytesLike, ArgCallable, ArgIntoBool, ArgIterable, ArgMapping, FuncArgs, KwArgs, + OptionalArg, OptionalOption, PosArgs, }, - protocol::{PyIter, PyIterReturn, PyMapping}, + protocol::{PyIter, PyIterReturn}, py_io, readline::{Readline, ReadlineResult}, scope::Scope, stdlib::sys, types::PyComparisonOp, utils::Either, - IdProtocol, ItemProtocol, PyArithmeticValue, PyClassImpl, PyObject, PyObjectRef, - PyObjectWrap, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, VirtualMachine, + IdProtocol, PyArithmeticValue, PyClassImpl, PyObject, PyObjectRef, PyObjectWrap, PyRef, + PyResult, PyValue, TryFromObject, TypeProtocol, VirtualMachine, }; use num_traits::{Signed, ToPrimitive, Zero}; @@ -205,7 +205,7 @@ mod builtins { #[pyarg(any, default)] globals: Option, #[pyarg(any, default)] - locals: Option, + locals: Option, } #[cfg(feature = "rustpython-compiler")] @@ -219,8 +219,9 @@ mod builtins { } ( globals.clone(), - self.locals - .unwrap_or_else(|| PyMapping::new(globals.into())), + self.locals.unwrap_or_else(|| { + ArgMapping::try_from_object(vm, globals.into()).unwrap() + }), ) } None => ( @@ -428,8 +429,8 @@ mod builtins { } #[pyfunction] - fn locals(vm: &VirtualMachine) -> PyResult { - vm.current_locals() + fn locals(vm: &VirtualMachine) -> PyResult { + vm.current_locals().map(|x| x.into_object()) } fn min_or_max( @@ -882,11 +883,11 @@ mod builtins { })?; // Accept any PyMapping as namespace. - let namespace = PyMapping::try_from_object(vm, namespace.clone()).map_err(|_| { + let namespace = ArgMapping::try_from_object(vm, namespace.clone()).map_err(|_| { vm.new_type_error(format!( "{}.__prepare__() must return a mapping, not {}", meta_name, - namespace.class().name() + namespace.class() )) })?; diff --git a/vm/src/stdlib/errno.rs b/vm/src/stdlib/errno.rs index 73a20ef379..9070d68453 100644 --- a/vm/src/stdlib/errno.rs +++ b/vm/src/stdlib/errno.rs @@ -1,5 +1,5 @@ +use crate::PyObjectRef; use crate::VirtualMachine; -use crate::{ItemProtocol, PyObjectRef}; #[pymodule] mod errno {} diff --git a/vm/src/stdlib/imp.rs b/vm/src/stdlib/imp.rs index d1676421b3..f8deeb4363 100644 --- a/vm/src/stdlib/imp.rs +++ b/vm/src/stdlib/imp.rs @@ -52,7 +52,7 @@ mod lock { mod _imp { use crate::{ builtins::{PyBytesRef, PyCode, PyModule, PyStr, PyStrRef}, - import, ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, VirtualMachine, + import, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, VirtualMachine, }; #[pyfunction] diff --git a/vm/src/stdlib/operator.rs b/vm/src/stdlib/operator.rs index 5edf6f203a..429354fe85 100644 --- a/vm/src/stdlib/operator.rs +++ b/vm/src/stdlib/operator.rs @@ -20,8 +20,8 @@ mod _operator { }, utils::Either, vm::ReprGuard, - IdProtocol, ItemProtocol, PyObjectRef, PyObjectView, PyRef, PyResult, PyValue, - TypeProtocol, VirtualMachine, + IdProtocol, PyObjectRef, PyObjectView, PyRef, PyResult, PyValue, TypeProtocol, + VirtualMachine, }; /// Same as a < b. diff --git a/vm/src/stdlib/posix.rs b/vm/src/stdlib/posix.rs index 11205c4181..41a2502656 100644 --- a/vm/src/stdlib/posix.rs +++ b/vm/src/stdlib/posix.rs @@ -38,7 +38,7 @@ pub mod module { }, types::Constructor, utils::{Either, ToCString}, - ItemProtocol, PyObjectRef, PyResult, PyValue, TryFromObject, TypeProtocol, VirtualMachine, + PyObjectRef, PyResult, PyValue, TryFromObject, TypeProtocol, VirtualMachine, }; use bitflags::bitflags; use nix::fcntl; @@ -1210,11 +1210,11 @@ pub mod module { #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "macos"))] fn envp_from_dict( - env: crate::protocol::PyMapping, + env: crate::function::ArgMapping, vm: &VirtualMachine, ) -> PyResult> { - let keys = env.keys(vm)?; - let values = env.values(vm)?; + let keys = env.mapping().keys(vm)?; + let values = env.mapping().values(vm)?; let keys = PyListRef::try_from_object(vm, keys) .map_err(|_| vm.new_type_error("env.keys() is not a list".to_owned()))? @@ -1261,7 +1261,7 @@ pub mod module { #[pyarg(positional)] args: crate::function::ArgIterable, #[pyarg(positional)] - env: crate::protocol::PyMapping, + env: crate::function::ArgMapping, #[pyarg(named, default)] file_actions: Option>, #[pyarg(named, default)] diff --git a/vm/src/stdlib/sre.rs b/vm/src/stdlib/sre.rs index c9c8ba286e..ad1c811d50 100644 --- a/vm/src/stdlib/sre.rs +++ b/vm/src/stdlib/sre.rs @@ -12,8 +12,8 @@ mod _sre { protocol::PyBuffer, stdlib::sys, types::{Comparable, Hashable}, - ItemProtocol, PyComparisonValue, PyObject, PyObjectRef, PyRef, PyResult, PyValue, - TryFromBorrowedObject, TryFromObject, VirtualMachine, + PyComparisonValue, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TryFromBorrowedObject, + TryFromObject, VirtualMachine, }; use core::str; use crossbeam_utils::atomic::AtomicCell; @@ -752,7 +752,7 @@ mod _sre { } else { self.pattern .groupindex - .get_item_option(group, vm) + .get_item_opt(group, vm) .ok()?? .downcast::() .ok()? diff --git a/vm/src/stdlib/sys.rs b/vm/src/stdlib/sys.rs index 699829b764..10f98f0fe5 100644 --- a/vm/src/stdlib/sys.rs +++ b/vm/src/stdlib/sys.rs @@ -1,6 +1,4 @@ -use crate::{ - function::IntoPyObject, ItemProtocol, PyClassImpl, PyObject, PyResult, VirtualMachine, -}; +use crate::{function::IntoPyObject, PyClassImpl, PyObject, PyResult, VirtualMachine}; pub(crate) use sys::{MAXSIZE, MULTIARCH}; @@ -17,7 +15,7 @@ mod sys { stdlib::builtins, version, vm::{PySettings, VirtualMachine}, - ItemProtocol, PyObjectRef, PyRef, PyRefExact, PyResult, PyStructSequence, + PyObjectRef, PyRef, PyRefExact, PyResult, PyStructSequence, }; use num_traits::ToPrimitive; use std::{env, mem, path}; diff --git a/vm/src/stdlib/sysconfigdata.rs b/vm/src/stdlib/sysconfigdata.rs index 2a2d4e8f4b..c3fb39b16c 100644 --- a/vm/src/stdlib/sysconfigdata.rs +++ b/vm/src/stdlib/sysconfigdata.rs @@ -3,8 +3,7 @@ pub(crate) use _sysconfigdata::make_module; #[pymodule] pub(crate) mod _sysconfigdata { use crate::{ - builtins::PyDictRef, function::IntoPyObject, stdlib::sys::MULTIARCH, ItemProtocol, - VirtualMachine, + builtins::PyDictRef, function::IntoPyObject, stdlib::sys::MULTIARCH, VirtualMachine, }; #[pyattr] diff --git a/vm/src/stdlib/thread.rs b/vm/src/stdlib/thread.rs index fe796fc47f..d8a91dc270 100644 --- a/vm/src/stdlib/thread.rs +++ b/vm/src/stdlib/thread.rs @@ -10,8 +10,7 @@ pub(crate) mod _thread { py_io, types::{Constructor, GetAttr, SetAttr}, utils::Either, - IdProtocol, ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, - VirtualMachine, + IdProtocol, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, VirtualMachine, }; use parking_lot::{ lock_api::{RawMutex as RawMutexT, RawMutexTimed, RawReentrantMutex}, diff --git a/vm/src/types/slot.rs b/vm/src/types/slot.rs index cabbc76970..58950e5960 100644 --- a/vm/src/types/slot.rs +++ b/vm/src/types/slot.rs @@ -163,31 +163,37 @@ fn as_mapping_wrapper(zelf: &PyObject, _vm: &VirtualMachine) -> PyMappingMethods }; } PyMappingMethods { - length: then_some_closure!(zelf.has_class_attr("__len__"), |zelf, vm| { - vm.call_special_method(zelf, "__len__", ()).map(|obj| { - obj.payload_if_subclass::(vm) - .map(|length_obj| { - length_obj.as_bigint().to_usize().ok_or_else(|| { - vm.new_value_error("__len__() should return >= 0".to_owned()) + length: then_some_closure!(zelf.has_class_attr("__len__"), |mapping, vm| { + vm.call_special_method(mapping.obj.to_owned(), "__len__", ()) + .map(|obj| { + obj.payload_if_subclass::(vm) + .map(|length_obj| { + length_obj.as_bigint().to_usize().ok_or_else(|| { + vm.new_value_error("__len__() should return >= 0".to_owned()) + }) }) - }) - .unwrap() - })? + .unwrap() + })? + }), + subscript: then_some_closure!(zelf.has_class_attr("__getitem__"), |mapping, needle, vm| { + vm.call_special_method(mapping.obj.to_owned(), "__getitem__", (needle.to_owned(),)) }), - subscript: then_some_closure!( - zelf.has_class_attr("__getitem__"), - |zelf: PyObjectRef, needle: PyObjectRef, vm: &VirtualMachine| { - vm.call_special_method(zelf, "__getitem__", (needle,)) - } - ), ass_subscript: then_some_closure!( zelf.has_class_attr("__setitem__") | zelf.has_class_attr("__delitem__"), - |zelf, needle, value, vm| match value { + |mapping, needle, value, vm| match value { Some(value) => vm - .call_special_method(zelf, "__setitem__", (needle, value),) + .call_special_method( + mapping.obj.to_owned(), + "__setitem__", + (needle.to_owned(), value), + ) .map(|_| Ok(()))?, None => vm - .call_special_method(zelf, "__delitem__", (needle,)) + .call_special_method( + mapping.obj.to_owned(), + "__delitem__", + (needle.to_owned(),) + ) .map(|_| Ok(()))?, } ), diff --git a/vm/src/vm.rs b/vm/src/vm.rs index dbf7acf034..25a525bd4b 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -20,14 +20,14 @@ use crate::{ common::{ascii, hash::HashSecret, lock::PyMutex, rc::PyRc}, frame::{ExecutionResult, Frame, FrameRef}, frozen, - function::{FuncArgs, IntoFuncArgs, IntoPyObject}, + function::{ArgMapping, FuncArgs, IntoFuncArgs, IntoPyObject}, import, - protocol::{PyIterIter, PyIterReturn, PyMapping}, + protocol::{PyIterIter, PyIterReturn}, scope::Scope, signal, stdlib, types::PyComparisonOp, - IdProtocol, ItemProtocol, PyArithmeticValue, PyContext, PyLease, PyMethod, PyObject, - PyObjectRef, PyObjectWrap, PyRef, PyRefExact, PyResult, PyValue, TryFromObject, TypeProtocol, + IdProtocol, PyArithmeticValue, PyContext, PyLease, PyMethod, PyObject, PyObjectRef, + PyObjectWrap, PyRef, PyRefExact, PyResult, PyValue, TryFromObject, TypeProtocol, }; use crossbeam_utils::atomic::AtomicCell; use num_traits::{Signed, ToPrimitive}; @@ -574,7 +574,7 @@ impl VirtualMachine { } } - pub fn current_locals(&self) -> PyResult { + pub fn current_locals(&self) -> PyResult { self.current_frame() .expect("called current_locals but no frames on the stack") .locals(self) @@ -757,6 +757,7 @@ impl VirtualMachine { self.new_exception_msg(buffer_error, msg) } + // TODO: don't take ownership should make the success path faster pub fn new_key_error(&self, obj: PyObjectRef) -> PyBaseExceptionRef { let key_error = self.ctx.exceptions.key_error.clone(); self.new_exception(key_error, vec![obj]) @@ -1384,7 +1385,7 @@ impl VirtualMachine { let dict = dict.or_else(|| obj.dict()); let attr = if let Some(dict) = dict { - dict.get_item_option(name, self)? + dict.get_item_opt(name, self)? } else { None }; diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs index 67b0d75824..f531a8c8b1 100644 --- a/wasm/lib/src/vm_class.rs +++ b/wasm/lib/src/vm_class.rs @@ -8,7 +8,7 @@ use wasm_bindgen::prelude::*; use rustpython_vm::compile::{self, Mode}; use rustpython_vm::scope::Scope; use rustpython_vm::{InitParameter, Interpreter, PySettings, VirtualMachine}; -use rustpython_vm::{ItemProtocol, PyObjectRef, PyObjectWeak, PyResult, PyValue}; +use rustpython_vm::{PyObjectRef, PyObjectWeak, PyResult, PyValue}; use crate::browser_module::setup_browser_module; use crate::convert::{self, PyResultExt}; @@ -285,7 +285,7 @@ impl WASMVirtualMachine { } } - vm.run_code_obj(code, Scope::new(None, attrs.clone())) + vm.run_code_obj(code, Scope::new(None, attrs.clone(), vm)) .into_js(vm)?; let module = vm.new_module(&name, attrs, None); From 9a2eef6fabb87661014bb6f6d946b37e679f79d9 Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Sat, 27 Nov 2021 13:49:05 +0200 Subject: [PATCH 3/9] fix compile --- benches/microbenchmarks.rs | 2 +- examples/mini_repl.rs | 2 -- stdlib/src/scproxy.rs | 2 +- stdlib/src/ssl.rs | 2 +- vm/src/builtins/function/jitfunc.rs | 3 +-- vm/src/stdlib/nt.rs | 2 +- vm/src/stdlib/posix_compat.rs | 1 - vm/src/stdlib/winapi.rs | 2 +- wasm/lib/src/convert.rs | 3 +-- 9 files changed, 7 insertions(+), 12 deletions(-) diff --git a/benches/microbenchmarks.rs b/benches/microbenchmarks.rs index ddfdb57c25..bbed9a8907 100644 --- a/benches/microbenchmarks.rs +++ b/benches/microbenchmarks.rs @@ -4,7 +4,7 @@ use criterion::{ }; use rustpython_compiler::Mode; use rustpython_vm::{ - common::ascii, InitParameter, Interpreter, ItemProtocol, PyObjectWrap, PyResult, PySettings, + common::ascii, InitParameter, Interpreter, PyObjectWrap, PyResult, PySettings, }; use std::path::{Path, PathBuf}; use std::{ffi, fs, io}; diff --git a/examples/mini_repl.rs b/examples/mini_repl.rs index 87bf9b7c3e..7e4bf20987 100644 --- a/examples/mini_repl.rs +++ b/examples/mini_repl.rs @@ -6,8 +6,6 @@ use rustpython_vm as vm; // these are needed for special memory shenanigans to let us share a variable with Python and Rust use std::sync::atomic::{AtomicBool, Ordering}; -// this needs to be in scope in order to insert things into scope.globals -use vm::ItemProtocol; // This has to be a macro because it uses the py_compile macro, // which compiles python source to optimized bytecode at compile time, so that diff --git a/stdlib/src/scproxy.rs b/stdlib/src/scproxy.rs index baceb9167e..9e5c63c855 100644 --- a/stdlib/src/scproxy.rs +++ b/stdlib/src/scproxy.rs @@ -7,7 +7,7 @@ mod _scproxy { use crate::vm::{ builtins::{PyDictRef, PyStr}, function::IntoPyObject, - ItemProtocol, PyResult, VirtualMachine, + PyResult, VirtualMachine, }; use system_configuration::core_foundation::{ array::CFArray, diff --git a/stdlib/src/ssl.rs b/stdlib/src/ssl.rs index ad5578b1d6..7fe420c686 100644 --- a/stdlib/src/ssl.rs +++ b/stdlib/src/ssl.rs @@ -39,7 +39,7 @@ mod _ssl { stdlib::os::PyPathLike, types::Constructor, utils::{Either, ToCString}, - ItemProtocol, PyObjectRef, PyObjectWeak, PyRef, PyResult, PyValue, VirtualMachine, + PyObjectRef, PyObjectWeak, PyRef, PyResult, PyValue, VirtualMachine, }, }; use crossbeam_utils::atomic::AtomicCell; diff --git a/vm/src/builtins/function/jitfunc.rs b/vm/src/builtins/function/jitfunc.rs index 3b97f299ea..4d99fe3f8f 100644 --- a/vm/src/builtins/function/jitfunc.rs +++ b/vm/src/builtins/function/jitfunc.rs @@ -2,8 +2,7 @@ use crate::{ builtins::{float, int, pybool, PyBaseExceptionRef, PyDictRef, PyFunction, PyStrRef}, bytecode::CodeFlags, function::{FuncArgs, IntoPyObject}, - IdProtocol, ItemProtocol, PyObject, PyObjectRef, PyResult, TryFromObject, TypeProtocol, - VirtualMachine, + IdProtocol, PyObject, PyObjectRef, PyResult, TryFromObject, TypeProtocol, VirtualMachine, }; use num_traits::ToPrimitive; use rustpython_jit::{AbiValue, Args, CompiledCode, JitArgumentError, JitType}; diff --git a/vm/src/stdlib/nt.rs b/vm/src/stdlib/nt.rs index 277094e99c..5a8d12ac3f 100644 --- a/vm/src/stdlib/nt.rs +++ b/vm/src/stdlib/nt.rs @@ -25,9 +25,9 @@ pub(crate) mod module { use std::io; use std::{env, fs}; + use crate::builtins::PyDictRef; #[cfg(target_env = "msvc")] use crate::builtins::PyListRef; - use crate::{builtins::PyDictRef, ItemProtocol}; use winapi::{um, vc::vcruntime::intptr_t}; #[pyattr] diff --git a/vm/src/stdlib/posix_compat.rs b/vm/src/stdlib/posix_compat.rs index 3a958f6620..487f359ffc 100644 --- a/vm/src/stdlib/posix_compat.rs +++ b/vm/src/stdlib/posix_compat.rs @@ -45,7 +45,6 @@ pub(crate) mod module { #[cfg(target_os = "wasi")] #[pyattr] fn environ(vm: &VirtualMachine) -> crate::builtins::PyDictRef { - use crate::ItemProtocol; use ffi_ext::OsStringExt; let environ = vm.ctx.new_dict(); diff --git a/vm/src/stdlib/winapi.rs b/vm/src/stdlib/winapi.rs index 33b86cf8a9..2aea4cfe66 100644 --- a/vm/src/stdlib/winapi.rs +++ b/vm/src/stdlib/winapi.rs @@ -8,7 +8,7 @@ mod _winapi { function::{IntoPyException, OptionalArg}, protocol::PyMapping, stdlib::os::errno_err, - ItemProtocol, PyObjectRef, PyResult, PySequence, TryFromObject, VirtualMachine, + PyObjectRef, PyResult, PySequence, TryFromObject, VirtualMachine, }; use std::ptr::{null, null_mut}; use winapi::shared::winerror; diff --git a/wasm/lib/src/convert.rs b/wasm/lib/src/convert.rs index c71c4e1ec0..0e7972391b 100644 --- a/wasm/lib/src/convert.rs +++ b/wasm/lib/src/convert.rs @@ -7,8 +7,7 @@ use rustpython_vm::{ compile::{CompileError, CompileErrorType}, exceptions, function::{ArgBytesLike, FuncArgs}, - py_serde, ItemProtocol, PyObjectRef, PyResult, PyValue, TryFromBorrowedObject, TypeProtocol, - VirtualMachine, + py_serde, PyObjectRef, PyResult, PyValue, TryFromBorrowedObject, TypeProtocol, VirtualMachine, }; use wasm_bindgen::{closure::Closure, prelude::*, JsCast}; From fdd82fac78cd0bba8a73bdebce4eb202e2469b72 Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Sat, 27 Nov 2021 14:09:43 +0200 Subject: [PATCH 4/9] fixup --- vm/src/builtins/function/jitfunc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/builtins/function/jitfunc.rs b/vm/src/builtins/function/jitfunc.rs index 4d99fe3f8f..0c43244aad 100644 --- a/vm/src/builtins/function/jitfunc.rs +++ b/vm/src/builtins/function/jitfunc.rs @@ -41,7 +41,7 @@ pub fn new_jit_error(msg: String, vm: &VirtualMachine) -> PyBaseExceptionRef { } fn get_jit_arg_type(dict: &PyDictRef, name: &str, vm: &VirtualMachine) -> PyResult { - if let Some(value) = dict.get_item_option(name, vm)? { + if let Some(value) = dict.get_item_opt(name, vm)? { if value.is(&vm.ctx.types.int_type) { Ok(JitType::Int) } else if value.is(&vm.ctx.types.float_type) { From e93f98c52ea959d9fedf1d49d33d540e410d5f4a Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Sat, 27 Nov 2021 15:02:41 +0200 Subject: [PATCH 5/9] fix windows compile --- vm/src/stdlib/winapi.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/vm/src/stdlib/winapi.rs b/vm/src/stdlib/winapi.rs index 2aea4cfe66..dba7c69156 100644 --- a/vm/src/stdlib/winapi.rs +++ b/vm/src/stdlib/winapi.rs @@ -5,8 +5,7 @@ pub(crate) use _winapi::make_module; mod _winapi { use crate::{ builtins::{PyListRef, PyStrRef}, - function::{IntoPyException, OptionalArg}, - protocol::PyMapping, + function::{ArgMapping, IntoPyException, OptionalArg}, stdlib::os::errno_err, PyObjectRef, PyResult, PySequence, TryFromObject, VirtualMachine, }; @@ -145,7 +144,7 @@ mod _winapi { #[pyarg(positional)] creation_flags: u32, #[pyarg(positional)] - env_mapping: Option, + env_mapping: Option, #[pyarg(positional)] current_dir: Option, #[pyarg(positional)] @@ -245,9 +244,9 @@ mod _winapi { )) } - fn getenvironment(env: PyMapping, vm: &VirtualMachine) -> PyResult> { - let keys = env.keys(vm)?; - let values = env.values(vm)?; + fn getenvironment(env: ArgMapping, vm: &VirtualMachine) -> PyResult> { + let keys = env.mapping().keys(vm)?; + let values = env.mapping().values(vm)?; let keys = PyListRef::try_from_object(vm, keys)?.borrow_vec().to_vec(); let values = PyListRef::try_from_object(vm, values)? @@ -294,7 +293,7 @@ mod _winapi { } fn getattributelist(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult> { - >::try_from_object(vm, obj)? + >::try_from_object(vm, obj)? .map(|mapping| { let handlelist = mapping .as_ref() From e06afd2cc0ddc3eaffce0b4196d73a80c130512a Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Sun, 28 Nov 2021 10:42:22 +0200 Subject: [PATCH 6/9] fix windows fail, hope so --- vm/src/stdlib/winapi.rs | 6 ++---- wasm/lib/src/vm_class.rs | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/vm/src/stdlib/winapi.rs b/vm/src/stdlib/winapi.rs index dba7c69156..fede8d8db8 100644 --- a/vm/src/stdlib/winapi.rs +++ b/vm/src/stdlib/winapi.rs @@ -248,10 +248,8 @@ mod _winapi { let keys = env.mapping().keys(vm)?; let values = env.mapping().values(vm)?; - let keys = PyListRef::try_from_object(vm, keys)?.borrow_vec().to_vec(); - let values = PyListRef::try_from_object(vm, values)? - .borrow_vec() - .to_vec(); + let keys = PySequence::try_from_object(vm, keys)?.into_vec(); + let values = PySequence::try_from_object(vm, values)?.into_vec(); if keys.len() != values.len() { return Err( diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs index f531a8c8b1..b50d255d9f 100644 --- a/wasm/lib/src/vm_class.rs +++ b/wasm/lib/src/vm_class.rs @@ -285,7 +285,7 @@ impl WASMVirtualMachine { } } - vm.run_code_obj(code, Scope::new(None, attrs.clone(), vm)) + vm.run_code_obj(code, Scope::new(None, attrs.clone())) .into_js(vm)?; let module = vm.new_module(&name, attrs, None); From 06fbba6589521eb90941758969540e621eb8bfb3 Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Sun, 28 Nov 2021 12:48:19 +0200 Subject: [PATCH 7/9] clearup --- vm/src/builtins/dict.rs | 22 ++++++---------------- vm/src/frame.rs | 6 ++++-- vm/src/protocol/mapping.rs | 15 --------------- vm/src/stdlib/winapi.rs | 2 +- 4 files changed, 11 insertions(+), 34 deletions(-) diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index adc8695215..d75902cbbd 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -477,9 +477,10 @@ impl PyObjectView { &self, key: K, vm: &VirtualMachine, - ) -> Option { + ) -> PyResult> { vm.get_method(self.to_owned().into(), "__missing__") .map(|methods| vm.invoke(&methods?, (key,))) + .transpose() } #[inline] @@ -490,14 +491,11 @@ impl PyObjectView { ) -> PyResult { if let Some(value) = self.entries.get(vm, &key)? { Ok(value) - } else if let Some(value) = self.missing_opt(key.clone(), vm) { - value + } else if let Some(value) = self.missing_opt(key.clone(), vm)? { + Ok(value) } else { Err(vm.new_key_error(key.into_pyobject(vm))) } - // self.entries - // .get(vm, &key)? - // .ok_or_else(|| vm.new_key_error(key.into_pyobject(vm))) } /// Take a python dictionary and convert it to attributes. @@ -517,12 +515,11 @@ impl PyObjectView { ) -> PyResult> { if self.exact_dict(vm) { self.entries.get(vm, &key) + // FIXME: check __missing__? } else { match self.as_object().get_item(key.clone(), vm) { Ok(value) => Ok(Some(value)), - Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => { - self.missing_opt(key, vm).transpose() - } + Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => self.missing_opt(key, vm), Err(e) => Err(e), } } @@ -537,13 +534,6 @@ impl PyObjectView { self.inner_getitem(key, vm) } else { self.as_object().get_item(key, vm) - // match self.as_object().get_item(key.clone(), vm) { - // Ok(value) => Ok(value), - // Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => { - // self.missing_opt(key, vm).ok_or(e)? - // } - // Err(e) => Err(e), - // } } } diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 2de8f80080..9382573624 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -198,9 +198,11 @@ impl FrameRef { let map_to_dict = |keys: &[PyStrRef], values: &[PyCellRef]| { for (k, v) in itertools::zip(keys, values) { if let Some(value) = v.get() { - locals.as_object().set_item(k.clone(), value, vm)?; + locals + .mapping() + .ass_subscript(k.as_object(), Some(value), vm)?; } else { - match locals.as_object().del_item(k.clone(), vm) { + match locals.mapping().ass_subscript(k.as_object(), None, vm) { Ok(()) => {} Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => {} Err(e) => return Err(e), diff --git a/vm/src/protocol/mapping.rs b/vm/src/protocol/mapping.rs index 733bb5745e..ef7cc3bf25 100644 --- a/vm/src/protocol/mapping.rs +++ b/vm/src/protocol/mapping.rs @@ -70,7 +70,6 @@ impl PyMapping<'_> { } pub fn methods(&self, vm: &VirtualMachine) -> &PyMappingMethods { - // let cls = self.obj.class(); self.methods.get_or_init(|| { if let Some(f) = self .obj @@ -82,20 +81,6 @@ impl PyMapping<'_> { PyMappingMethods::default() } }) - - // let get_methods = || { - // if let Some(f) = cls.mro_find_map(|cls| cls.slots.as_mapping.load()) { - // f(self.obj, vm) - // } else { - // PyMappingMethods::default() - // } - // }; - - // if cls.slots.flags.has_feature(PyTypeFlags::HEAPTYPE) { - // &get_methods() - // } else { - // self.methods.get_or_init(get_methods) - // } } pub fn length_opt(&self, vm: &VirtualMachine) -> Option> { diff --git a/vm/src/stdlib/winapi.rs b/vm/src/stdlib/winapi.rs index fede8d8db8..eadfd7efbe 100644 --- a/vm/src/stdlib/winapi.rs +++ b/vm/src/stdlib/winapi.rs @@ -4,7 +4,7 @@ pub(crate) use _winapi::make_module; #[pymodule] mod _winapi { use crate::{ - builtins::{PyListRef, PyStrRef}, + builtins::PyStrRef, function::{ArgMapping, IntoPyException, OptionalArg}, stdlib::os::errno_err, PyObjectRef, PyResult, PySequence, TryFromObject, VirtualMachine, From 6a996b04c81926034c0aeb05c3076ff462f25a04 Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Wed, 1 Dec 2021 20:16:18 +0200 Subject: [PATCH 8/9] move obj downcast to AsMapping --- stdlib/src/array.rs | 6 +++--- vm/src/builtins/bytearray.rs | 6 +++--- vm/src/builtins/bytes.rs | 4 ++-- vm/src/builtins/dict.rs | 8 +++++--- vm/src/builtins/genericalias.rs | 2 +- vm/src/builtins/list.rs | 6 +++--- vm/src/builtins/mappingproxy.rs | 2 +- vm/src/builtins/memory.rs | 6 +++--- vm/src/builtins/pystr.rs | 5 ++--- vm/src/builtins/range.rs | 13 +++++++++---- vm/src/builtins/tuple.rs | 4 ++-- vm/src/protocol/mapping.rs | 7 +------ vm/src/types/slot.rs | 6 +++++- 13 files changed, 40 insertions(+), 35 deletions(-) diff --git a/stdlib/src/array.rs b/stdlib/src/array.rs index 71c63b147b..6c9a907449 100644 --- a/stdlib/src/array.rs +++ b/stdlib/src/array.rs @@ -1241,12 +1241,12 @@ mod array { impl PyArray { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(Self::mapping_downcast(mapping).len())), subscript: Some(|mapping, needle, vm| { - mapping.obj_as::().getitem(needle.to_owned(), vm) + Self::mapping_downcast(mapping).getitem(needle.to_owned(), vm) }), ass_subscript: Some(|mapping, needle, value, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); if let Some(value) = value { Self::setitem(zelf.to_owned(), needle.to_owned(), value, vm) } else { diff --git a/vm/src/builtins/bytearray.rs b/vm/src/builtins/bytearray.rs index 47d5ec7ec7..da55997a07 100644 --- a/vm/src/builtins/bytearray.rs +++ b/vm/src/builtins/bytearray.rs @@ -679,12 +679,12 @@ impl PyByteArray { impl PyByteArray { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(Self::mapping_downcast(mapping).len())), subscript: Some(|mapping, needle, vm| { - mapping.obj_as::().getitem(needle.to_owned(), vm) + Self::mapping_downcast(mapping).getitem(needle.to_owned(), vm) }), ass_subscript: Some(|mapping, needle, value, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); if let Some(value) = value { Self::setitem(zelf.to_owned(), needle.to_owned(), value, vm) } else { diff --git a/vm/src/builtins/bytes.rs b/vm/src/builtins/bytes.rs index f262b76abd..1e131b926e 100644 --- a/vm/src/builtins/bytes.rs +++ b/vm/src/builtins/bytes.rs @@ -525,9 +525,9 @@ impl PyBytes { impl PyBytes { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(Self::mapping_downcast(mapping).len())), subscript: Some(|mapping, needle, vm| { - mapping.obj_as::().getitem(needle.to_owned(), vm) + Self::mapping_downcast(mapping).getitem(needle.to_owned(), vm) }), ass_subscript: None, }; diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index d75902cbbd..2b0dd05285 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -426,10 +426,12 @@ impl PyDict { impl PyDict { pub(crate) const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), - subscript: Some(|mapping, needle, vm| mapping.obj_as::().inner_getitem(needle, vm)), + length: Some(|mapping, _vm| Ok(Self::mapping_downcast(mapping).len())), + subscript: Some(|mapping, needle, vm| { + Self::mapping_downcast(mapping).inner_getitem(needle, vm) + }), ass_subscript: Some(|mapping, needle, value, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); if let Some(value) = value { zelf.inner_setitem(needle, value, vm) } else { diff --git a/vm/src/builtins/genericalias.rs b/vm/src/builtins/genericalias.rs index 9e71a04aa9..6f3956eb2d 100644 --- a/vm/src/builtins/genericalias.rs +++ b/vm/src/builtins/genericalias.rs @@ -292,7 +292,7 @@ impl PyGenericAlias { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { length: None, subscript: Some(|mapping, needle, vm| { - mapping.obj_as::().getitem(needle.to_owned(), vm) + Self::mapping_downcast(mapping).getitem(needle.to_owned(), vm) }), ass_subscript: None, }; diff --git a/vm/src/builtins/list.rs b/vm/src/builtins/list.rs index 21618f03f6..2ba1a78bcb 100644 --- a/vm/src/builtins/list.rs +++ b/vm/src/builtins/list.rs @@ -391,13 +391,13 @@ impl<'a> MutObjectSequenceOp<'a> for PyList { impl PyList { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(Self::mapping_downcast(mapping).len())), subscript: Some(|mapping, needle, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); Self::getitem(zelf.to_owned(), needle.to_owned(), vm) }), ass_subscript: Some(|mapping, needle, value, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); if let Some(value) = value { zelf.setitem(needle.to_owned(), value, vm) } else { diff --git a/vm/src/builtins/mappingproxy.rs b/vm/src/builtins/mappingproxy.rs index 6c8f89c1c5..e5df6bac7c 100644 --- a/vm/src/builtins/mappingproxy.rs +++ b/vm/src/builtins/mappingproxy.rs @@ -159,7 +159,7 @@ impl PyMappingProxy { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { length: None, subscript: Some(|mapping, needle, vm| { - mapping.obj_as::().getitem(needle.to_owned(), vm) + Self::mapping_downcast(mapping).getitem(needle.to_owned(), vm) }), ass_subscript: None, }; diff --git a/vm/src/builtins/memory.rs b/vm/src/builtins/memory.rs index 7682b4c7f3..e36dadb29d 100644 --- a/vm/src/builtins/memory.rs +++ b/vm/src/builtins/memory.rs @@ -940,13 +940,13 @@ impl Drop for PyMemoryView { impl PyMemoryView { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, vm| mapping.obj_as::().len(vm)), + length: Some(|mapping, vm| Self::mapping_downcast(mapping).len(vm)), subscript: Some(|mapping, needle, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); Self::getitem(zelf.to_owned(), needle.to_owned(), vm) }), ass_subscript: Some(|mapping, needle, value, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); if let Some(value) = value { Self::setitem(zelf.to_owned(), needle.to_owned(), value, vm) } else { diff --git a/vm/src/builtins/pystr.rs b/vm/src/builtins/pystr.rs index d743a55fff..ac9c8511dc 100644 --- a/vm/src/builtins/pystr.rs +++ b/vm/src/builtins/pystr.rs @@ -1266,10 +1266,9 @@ impl AsMapping for PyStr { impl PyStr { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(Self::mapping_downcast(mapping).len())), subscript: Some(|mapping, needle, vm| { - mapping - .obj_as::() + Self::mapping_downcast(mapping) .getitem(needle.to_owned(), vm) .map(|x| x.into_ref(vm).into()) }), diff --git a/vm/src/builtins/range.rs b/vm/src/builtins/range.rs index f8071fc917..569d9724e1 100644 --- a/vm/src/builtins/range.rs +++ b/vm/src/builtins/range.rs @@ -380,12 +380,17 @@ impl PyRange { impl PyRange { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { length: Some(|mapping, vm| { - mapping.obj_as::().len().to_usize().ok_or_else(|| { - vm.new_overflow_error("RustPython int too large to convert to C ssize_t".to_owned()) - }) + Self::mapping_downcast(mapping) + .len() + .to_usize() + .ok_or_else(|| { + vm.new_overflow_error( + "RustPython int too large to convert to C ssize_t".to_owned(), + ) + }) }), subscript: Some(|mapping, needle, vm| { - mapping.obj_as::().getitem(needle.to_owned(), vm) + Self::mapping_downcast(mapping).getitem(needle.to_owned(), vm) }), ass_subscript: None, }; diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index 81b75d9804..8d5d9d27fa 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -310,9 +310,9 @@ impl PyTuple { impl PyTuple { const MAPPING_METHODS: PyMappingMethods = PyMappingMethods { - length: Some(|mapping, _vm| Ok(mapping.obj_as::().len())), + length: Some(|mapping, _vm| Ok(Self::mapping_downcast(mapping).len())), subscript: Some(|mapping, needle, vm| { - let zelf = mapping.obj_as::(); + let zelf = Self::mapping_downcast(mapping); Self::getitem(zelf.to_owned(), needle.to_owned(), vm) }), ass_subscript: None, diff --git a/vm/src/protocol/mapping.rs b/vm/src/protocol/mapping.rs index ef7cc3bf25..44e3e631a6 100644 --- a/vm/src/protocol/mapping.rs +++ b/vm/src/protocol/mapping.rs @@ -5,8 +5,7 @@ use crate::{ }, common::lock::OnceCell, function::IntoPyResult, - IdProtocol, PyObject, PyObjectPayload, PyObjectRef, PyObjectView, PyResult, TypeProtocol, - VirtualMachine, + IdProtocol, PyObject, PyObjectRef, PyResult, TypeProtocol, VirtualMachine, }; // Mapping protocol @@ -65,10 +64,6 @@ impl PyMapping<'_> { self.methods(vm).subscript.is_some() } - pub fn obj_as(&self) -> &PyObjectView { - unsafe { self.obj.downcast_unchecked_ref::() } - } - pub fn methods(&self, vm: &VirtualMachine) -> &PyMappingMethods { self.methods.get_or_init(|| { if let Some(f) = self diff --git a/vm/src/types/slot.rs b/vm/src/types/slot.rs index 58950e5960..9b738037ce 100644 --- a/vm/src/types/slot.rs +++ b/vm/src/types/slot.rs @@ -2,7 +2,7 @@ use crate::common::{hash::PyHash, lock::PyRwLock}; use crate::{ builtins::{PyInt, PyStrRef, PyType, PyTypeRef}, function::{FromArgs, FuncArgs, IntoPyResult, OptionalArg}, - protocol::{PyBuffer, PyIterReturn, PyMappingMethods}, + protocol::{PyBuffer, PyIterReturn, PyMapping, PyMappingMethods}, utils::Either, IdProtocol, PyComparisonValue, PyObject, PyObjectRef, PyObjectView, PyRef, PyResult, PyValue, TypeProtocol, VirtualMachine, @@ -783,6 +783,10 @@ pub trait AsMapping: PyValue { } fn as_mapping(zelf: &PyObjectView, vm: &VirtualMachine) -> PyMappingMethods; + + fn mapping_downcast<'a>(mapping: &'a PyMapping) -> &'a PyObjectView { + unsafe { mapping.obj.downcast_unchecked_ref() } + } } #[pyimpl] From ae538943c0784c4333112cd4333f2973d7075bcf Mon Sep 17 00:00:00 2001 From: Kangzhi Shi Date: Wed, 1 Dec 2021 20:40:04 +0200 Subject: [PATCH 9/9] fix ass_subscript check --- vm/src/builtins/mappingproxy.rs | 2 +- vm/src/protocol/mapping.rs | 4 ++-- vm/src/protocol/object.rs | 10 ++++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/vm/src/builtins/mappingproxy.rs b/vm/src/builtins/mappingproxy.rs index e5df6bac7c..a650fffd29 100644 --- a/vm/src/builtins/mappingproxy.rs +++ b/vm/src/builtins/mappingproxy.rs @@ -37,7 +37,7 @@ impl Constructor for PyMappingProxy { type Args = PyObjectRef; fn py_new(cls: PyTypeRef, mapping: Self::Args, vm: &VirtualMachine) -> PyResult { - if !PyMapping::from(mapping.as_ref()).has_protocol(vm) + if !PyMapping::from(mapping.as_ref()).check(vm) || mapping.payload_if_subclass::(vm).is_some() || mapping.payload_if_subclass::(vm).is_some() { diff --git a/vm/src/protocol/mapping.rs b/vm/src/protocol/mapping.rs index 44e3e631a6..da2fb020e8 100644 --- a/vm/src/protocol/mapping.rs +++ b/vm/src/protocol/mapping.rs @@ -50,7 +50,7 @@ impl<'a> PyMapping<'a> { pub fn try_protocol(obj: &'a PyObject, vm: &VirtualMachine) -> PyResult { let zelf = Self::from(obj); - if zelf.has_protocol(vm) { + if zelf.check(vm) { Ok(zelf) } else { Err(vm.new_type_error(format!("{} is not a mapping object", zelf.obj.class()))) @@ -60,7 +60,7 @@ impl<'a> PyMapping<'a> { impl PyMapping<'_> { // PyMapping::Check - pub fn has_protocol(&self, vm: &VirtualMachine) -> bool { + pub fn check(&self, vm: &VirtualMachine) -> bool { self.methods(vm).subscript.is_some() } diff --git a/vm/src/protocol/object.rs b/vm/src/protocol/object.rs index b48f68b64d..148abe09b2 100644 --- a/vm/src/protocol/object.rs +++ b/vm/src/protocol/object.rs @@ -464,8 +464,9 @@ impl PyObject { let needle = needle.into_pyobject(vm); - if let Ok(mapping) = PyMapping::try_protocol(self, vm) { - mapping.ass_subscript(&needle, Some(value), vm) + let mapping = PyMapping::from(self); + if let Some(f) = mapping.methods(vm).ass_subscript { + f(&mapping, &needle, Some(value), vm) } else { // TODO: sequence protocol vm.get_special_method(self.to_owned(), "__setitem__")? @@ -491,8 +492,9 @@ impl PyObject { let needle = needle.into_pyobject(vm); - if let Ok(mapping) = PyMapping::try_protocol(self, vm) { - mapping.ass_subscript(&needle, None, vm) + let mapping = PyMapping::from(self); + if let Some(f) = mapping.methods(vm).ass_subscript { + f(&mapping, &needle, None, vm) } else { //TODO: sequence protocol vm.get_special_method(self.to_owned(), "__delitem__")?