From 3b7bb5cb611d939144343fcc18b2c37f35ea7868 Mon Sep 17 00:00:00 2001 From: snowapril Date: Tue, 26 Apr 2022 18:46:37 +0900 Subject: [PATCH 1/3] Relocate generic_setattr into object protocol As generic_setattr is included in object protocol, it seems more appropriate to relocate. Signed-off-by: snowapril --- vm/src/builtins/object.rs | 48 +++------------------------------------ vm/src/protocol/object.rs | 41 +++++++++++++++++++++++++++++++++ vm/src/types/slot.rs | 2 +- vm/src/vm/mod.rs | 3 +-- 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index 996468ab45..aa4315b7ec 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -162,13 +162,13 @@ impl PyBaseObject { value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - generic_setattr(&obj, name, Some(value), vm) + obj.generic_setattr(name, Some(value), vm) } /// Implement delattr(self, name). #[pymethod] fn __delattr__(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { - generic_setattr(&obj, name, None, vm) + obj.generic_setattr(name, None, vm) } #[pyslot] @@ -178,7 +178,7 @@ impl PyBaseObject { value: Option, vm: &VirtualMachine, ) -> PyResult<()> { - generic_setattr(&*obj, attr_name, value, vm) + obj.generic_setattr(attr_name, value, vm) } /// Return str(self). @@ -334,48 +334,6 @@ pub fn generic_getattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachin vm.generic_getattribute(obj, attr_name) } -#[cfg_attr(feature = "flame-it", flame)] -pub fn generic_setattr( - obj: &PyObject, - attr_name: PyStrRef, - value: Option, - vm: &VirtualMachine, -) -> PyResult<()> { - vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value); - - if let Some(attr) = obj.get_class_attr(attr_name.as_str()) { - let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); - if let Some(descriptor) = descr_set { - return descriptor(attr, obj.to_owned(), value, vm); - } - } - - if let Some(dict) = obj.dict() { - if let Some(value) = value { - dict.set_item(attr_name, value, vm)?; - } else { - dict.del_item(attr_name.clone(), vm).map_err(|e| { - if e.fast_isinstance(&vm.ctx.exceptions.key_error) { - vm.new_attribute_error(format!( - "'{}' object has no attribute '{}'", - obj.class().name(), - attr_name, - )) - } else { - e - } - })?; - } - Ok(()) - } else { - Err(vm.new_attribute_error(format!( - "'{}' object has no attribute '{}'", - obj.class().name(), - attr_name, - ))) - } -} - pub fn init(ctx: &Context) { PyBaseObject::extend_class(ctx, &ctx.types.object_type); } diff --git a/vm/src/protocol/object.rs b/vm/src/protocol/object.rs index fc372f4996..120e01f6e3 100644 --- a/vm/src/protocol/object.rs +++ b/vm/src/protocol/object.rs @@ -131,6 +131,47 @@ impl PyObject { } // int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value) + #[cfg_attr(feature = "flame-it", flame)] + pub fn generic_setattr( + &self, + attr_name: PyStrRef, + value: Option, + vm: &VirtualMachine, + ) -> PyResult<()> { + vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value); + + if let Some(attr) = self.get_class_attr(attr_name.as_str()) { + let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load()); + if let Some(descriptor) = descr_set { + return descriptor(attr, self.to_owned(), value, vm); + } + } + + if let Some(dict) = self.dict() { + if let Some(value) = value { + dict.set_item(attr_name, value, vm)?; + } else { + dict.del_item(attr_name.clone(), vm).map_err(|e| { + if e.fast_isinstance(&vm.ctx.exceptions.key_error) { + vm.new_attribute_error(format!( + "'{}' object has no attribute '{}'", + self.class().name(), + attr_name, + )) + } else { + e + } + })?; + } + Ok(()) + } else { + Err(vm.new_attribute_error(format!( + "'{}' object has no attribute '{}'", + self.class().name(), + attr_name, + ))) + } + } pub fn del_attr(&self, attr_name: impl IntoPyStrRef, vm: &VirtualMachine) -> PyResult<()> { let attr_name = attr_name.into_pystr_ref(vm); diff --git a/vm/src/types/slot.rs b/vm/src/types/slot.rs index 6fa09cce43..6a86ba640c 100644 --- a/vm/src/types/slot.rs +++ b/vm/src/types/slot.rs @@ -1,4 +1,4 @@ -pub use crate::builtins::object::{generic_getattr, generic_setattr}; +pub use crate::builtins::object::generic_getattr; use crate::common::{hash::PyHash, lock::PyRwLock}; use crate::{ builtins::{PyInt, PyStrRef, PyType, PyTypeRef}, diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index 9b91cbac1a..0bfa582597 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -17,7 +17,6 @@ mod vm_ops; use crate::{ builtins::{ code::{self, PyCode}, - object, pystr::IntoPyStrRef, tuple::{PyTuple, PyTupleTyped}, PyBaseExceptionRef, PyDictRef, PyList, PyModule, PyStrRef, PyTypeRef, @@ -741,6 +740,6 @@ impl VirtualMachine { attr_value: impl Into, ) -> PyResult<()> { let val = attr_value.into(); - object::generic_setattr(module, attr_name.into_pystr_ref(self), Some(val), self) + module.generic_setattr(attr_name.into_pystr_ref(self), Some(val), self) } } From 09f7d92cb9f0ba9d00f516599f58b2f2fc69edf0 Mon Sep 17 00:00:00 2001 From: snowapril Date: Tue, 26 Apr 2022 19:20:14 +0900 Subject: [PATCH 2/3] Relocate generic_getattr/opt from vm to object protocol According to cpython docs, generic_getattr/opt is object protocol. It is more appropriate to locate in protocol/object.rs than vm Signed-off-by: snowapril --- vm/src/builtins/genericalias.rs | 2 +- vm/src/builtins/module.rs | 8 ++-- vm/src/builtins/object.rs | 4 +- vm/src/builtins/super.rs | 2 +- vm/src/builtins/union.rs | 2 +- vm/src/protocol/object.rs | 68 +++++++++++++++++++++++++++++++-- vm/src/stdlib/thread.rs | 3 +- vm/src/vm/mod.rs | 60 ----------------------------- 8 files changed, 76 insertions(+), 73 deletions(-) diff --git a/vm/src/builtins/genericalias.rs b/vm/src/builtins/genericalias.rs index 01c7c7ba16..4b463b24ba 100644 --- a/vm/src/builtins/genericalias.rs +++ b/vm/src/builtins/genericalias.rs @@ -369,7 +369,7 @@ impl GetAttr for PyGenericAlias { fn getattro(zelf: PyRef, attr: PyStrRef, vm: &VirtualMachine) -> PyResult { for exc in &ATTR_EXCEPTIONS { if *(*exc) == attr.to_string() { - return vm.generic_getattribute(zelf.as_object().to_owned(), attr); + return zelf.as_object().generic_getattr(attr, vm); } } zelf.origin().get_attr(attr, vm) diff --git a/vm/src/builtins/module.rs b/vm/src/builtins/module.rs index 85d5dd092f..f16fab71b6 100644 --- a/vm/src/builtins/module.rs +++ b/vm/src/builtins/module.rs @@ -49,8 +49,9 @@ impl PyModule { } fn getattr_inner(zelf: &Py, name: PyStrRef, vm: &VirtualMachine) -> PyResult { - if let Some(attr) = - vm.generic_getattribute_opt(zelf.to_owned().into(), name.clone(), None)? + if let Some(attr) = zelf + .as_object() + .generic_getattr_opt(name.clone(), None, vm)? { return Ok(attr); } @@ -66,7 +67,8 @@ impl PyModule { } fn name(zelf: PyRef, vm: &VirtualMachine) -> Option { - vm.generic_getattribute_opt(zelf.into(), PyStr::from("__name__").into_ref(vm), None) + zelf.as_object() + .generic_getattr_opt(PyStr::from("__name__").into_ref(vm), None, vm) .unwrap_or(None) .and_then(|obj| obj.downcast::().ok()) } diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index aa4315b7ec..be8e267a15 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -288,7 +288,7 @@ impl PyBaseObject { #[pyslot] pub(crate) fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult { vm_trace!("object.__getattribute__({:?}, {:?})", obj, name); - vm.generic_getattribute(obj, name) + obj.as_object().generic_getattr(name, vm) } #[pymethod(magic)] @@ -331,7 +331,7 @@ pub fn object_set_dict(obj: PyObjectRef, dict: PyDictRef, vm: &VirtualMachine) - } pub fn generic_getattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult { - vm.generic_getattribute(obj, attr_name) + obj.as_object().generic_getattr(attr_name, vm) } pub fn init(ctx: &Context) { diff --git a/vm/src/builtins/super.rs b/vm/src/builtins/super.rs index ec94702c49..1b83c3b87a 100644 --- a/vm/src/builtins/super.rs +++ b/vm/src/builtins/super.rs @@ -126,7 +126,7 @@ impl PySuper { impl GetAttr for PySuper { fn getattro(zelf: PyRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult { - let skip = |zelf: PyRef, name| vm.generic_getattribute(zelf.into(), name); + let skip = |zelf: PyRef, name| zelf.as_object().generic_getattr(name, vm); let (obj, start_type): (PyObjectRef, PyTypeRef) = match zelf.obj.clone() { Some(o) => o, None => return skip(zelf, name), diff --git a/vm/src/builtins/union.rs b/vm/src/builtins/union.rs index 2e594b48e7..49bb3b9ef6 100644 --- a/vm/src/builtins/union.rs +++ b/vm/src/builtins/union.rs @@ -266,7 +266,7 @@ impl GetAttr for PyUnion { fn getattro(zelf: PyRef, attr: PyStrRef, vm: &VirtualMachine) -> PyResult { for &exc in CLS_ATTRS { if *exc == attr.to_string() { - return vm.generic_getattribute(zelf.as_object().to_owned(), attr); + return zelf.as_object().generic_getattr(attr, vm); } } zelf.as_object().to_pyobject(vm).get_attr(attr, vm) diff --git a/vm/src/protocol/object.rs b/vm/src/protocol/object.rs index 120e01f6e3..4750207e3f 100644 --- a/vm/src/protocol/object.rs +++ b/vm/src/protocol/object.rs @@ -3,8 +3,8 @@ use crate::{ builtins::{ - pystr::IntoPyStrRef, PyBytes, PyDict, PyGenericAlias, PyInt, PyStrRef, PyTupleRef, - PyTypeRef, + pystr::IntoPyStrRef, PyBytes, PyDict, PyDictRef, PyGenericAlias, PyInt, PyStrRef, + PyTupleRef, PyTypeRef, }, bytesinner::ByteInnerNewOptions, common::{hash::PyHash, str::to_ascii}, @@ -118,8 +118,6 @@ impl PyObject { setattro(self, attr_name, attr_value, vm) } - // PyObject *PyObject_GenericGetAttr(PyObject *o, PyObject *name) - pub fn set_attr( &self, attr_name: impl IntoPyStrRef, @@ -173,6 +171,68 @@ impl PyObject { } } + pub fn generic_getattr(&self, name: PyStrRef, vm: &VirtualMachine) -> PyResult { + self.generic_getattr_opt(name.clone(), None, vm)? + .ok_or_else(|| vm.new_attribute_error(format!("{} has no attribute '{}'", self, name))) + } + + /// CPython _PyObject_GenericGetAttrWithDict + pub fn generic_getattr_opt( + &self, + name_str: PyStrRef, + dict: Option, + vm: &VirtualMachine, + ) -> PyResult> { + let name = name_str.as_str(); + let obj_cls = self.class(); + let cls_attr = match obj_cls.get_attr(name) { + Some(descr) => { + let descr_cls = descr.class(); + let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load()); + if let Some(descr_get) = descr_get { + if descr_cls + .mro_find_map(|cls| cls.slots.descr_set.load()) + .is_some() + { + drop(descr_cls); + let cls = obj_cls.into_owned().into(); + return descr_get(descr, Some(self.to_pyobject(vm)), Some(cls), vm) + .map(Some); + } + } + drop(descr_cls); + Some((descr, descr_get)) + } + None => None, + }; + + let dict = dict.or_else(|| self.dict()); + + let attr = if let Some(dict) = dict { + dict.get_item_opt(name, vm)? + } else { + None + }; + + if let Some(obj_attr) = attr { + Ok(Some(obj_attr)) + } else if let Some((attr, descr_get)) = cls_attr { + match descr_get { + Some(descr_get) => { + let cls = obj_cls.into_owned().into(); + descr_get(attr, Some(self.to_pyobject(vm)), Some(cls), vm).map(Some) + } + None => Ok(Some(attr)), + } + } else if let Some(getter) = obj_cls.get_attr("__getattr__") { + drop(obj_cls); + vm.invoke(&getter, (self.to_pyobject(vm), name_str)) + .map(Some) + } else { + Ok(None) + } + } + pub fn del_attr(&self, attr_name: impl IntoPyStrRef, vm: &VirtualMachine) -> PyResult<()> { let attr_name = attr_name.into_pystr_ref(vm); self.call_set_attr(vm, attr_name, None) diff --git a/vm/src/stdlib/thread.rs b/vm/src/stdlib/thread.rs index 1f84b54c43..5bfa0773a9 100644 --- a/vm/src/stdlib/thread.rs +++ b/vm/src/stdlib/thread.rs @@ -326,7 +326,8 @@ pub(crate) mod _thread { if attr.as_str() == "__dict__" { Ok(ldict.into()) } else { - vm.generic_getattribute_opt(zelf.clone().into(), attr.clone(), Some(ldict))? + zelf.as_object() + .generic_getattr_opt(attr.clone(), Some(ldict), vm)? .ok_or_else(|| { vm.new_attribute_error(format!( "{} has no attribute '{}'", diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index 0bfa582597..7ace9251ef 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -593,66 +593,6 @@ impl VirtualMachine { Some(self.call_if_get_descriptor(method, obj)) } - pub fn generic_getattribute(&self, obj: PyObjectRef, name: PyStrRef) -> PyResult { - self.generic_getattribute_opt(obj.clone(), name.clone(), None)? - .ok_or_else(|| self.new_attribute_error(format!("{} has no attribute '{}'", obj, name))) - } - - /// CPython _PyObject_GenericGetAttrWithDict - pub fn generic_getattribute_opt( - &self, - obj: PyObjectRef, - name_str: PyStrRef, - dict: Option, - ) -> PyResult> { - let name = name_str.as_str(); - let obj_cls = obj.class(); - let cls_attr = match obj_cls.get_attr(name) { - Some(descr) => { - let descr_cls = descr.class(); - let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load()); - if let Some(descr_get) = descr_get { - if descr_cls - .mro_find_map(|cls| cls.slots.descr_set.load()) - .is_some() - { - drop(descr_cls); - let cls = obj_cls.into_owned().into(); - return descr_get(descr, Some(obj), Some(cls), self).map(Some); - } - } - drop(descr_cls); - Some((descr, descr_get)) - } - None => None, - }; - - let dict = dict.or_else(|| obj.dict()); - - let attr = if let Some(dict) = dict { - dict.get_item_opt(name, self)? - } else { - None - }; - - if let Some(obj_attr) = attr { - Ok(Some(obj_attr)) - } else if let Some((attr, descr_get)) = cls_attr { - match descr_get { - Some(descr_get) => { - let cls = obj_cls.into_owned().into(); - descr_get(attr, Some(obj), Some(cls), self).map(Some) - } - None => Ok(Some(attr)), - } - } else if let Some(getter) = obj_cls.get_attr("__getattr__") { - drop(obj_cls); - self.invoke(&getter, (obj, name_str)).map(Some) - } else { - Ok(None) - } - } - pub fn is_callable(&self, obj: &PyObject) -> bool { obj.class() .mro_find_map(|cls| cls.slots.call.load()) From df666dd21760e6460dac32805ab40d147e9d13cb Mon Sep 17 00:00:00 2001 From: snowapril Date: Tue, 26 Apr 2022 19:57:38 +0900 Subject: [PATCH 3/3] Remove duplicate generic_getattr in builtins::object Signed-off-by: snowapril --- vm/src/builtins/object.rs | 4 ---- vm/src/types/slot.rs | 1 - 2 files changed, 5 deletions(-) diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs index be8e267a15..d660d3d779 100644 --- a/vm/src/builtins/object.rs +++ b/vm/src/builtins/object.rs @@ -330,10 +330,6 @@ pub fn object_set_dict(obj: PyObjectRef, dict: PyDictRef, vm: &VirtualMachine) - .map_err(|_| vm.new_attribute_error("This object has no __dict__".to_owned())) } -pub fn generic_getattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult { - obj.as_object().generic_getattr(attr_name, vm) -} - pub fn init(ctx: &Context) { PyBaseObject::extend_class(ctx, &ctx.types.object_type); } diff --git a/vm/src/types/slot.rs b/vm/src/types/slot.rs index 6a86ba640c..e4dfafd01f 100644 --- a/vm/src/types/slot.rs +++ b/vm/src/types/slot.rs @@ -1,4 +1,3 @@ -pub use crate::builtins::object::generic_getattr; use crate::common::{hash::PyHash, lock::PyRwLock}; use crate::{ builtins::{PyInt, PyStrRef, PyType, PyTypeRef},