diff --git a/Lib/test/test_class.py b/Lib/test/test_class.py index 7420f289b16..1aee6fb73d2 100644 --- a/Lib/test/test_class.py +++ b/Lib/test/test_class.py @@ -978,7 +978,6 @@ def __init__(self): obj.foo = None # Aborted here self.assertEqual(obj.__dict__, {"foo":None}) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_store_attr_deleted_dict(self): class Foo: pass @@ -988,7 +987,6 @@ class Foo: f.a = 3 self.assertEqual(f.a, 3) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_rematerialize_object_dict(self): # gh-121860: rematerializing an object's managed dictionary after it # had been deleted caused a crash. diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 782daf0b9a6..55137c0de36 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -2,8 +2,8 @@ mod jit; use super::{ - PyAsyncGen, PyCode, PyCoroutine, PyDictRef, PyGenerator, PyModule, PyStr, PyStrRef, PyTuple, - PyTupleRef, PyType, + PyAsyncGen, PyCode, PyCoroutine, PyDict, PyDictRef, PyGenerator, PyModule, PyStr, PyStrRef, + PyTuple, PyTupleRef, PyType, }; use crate::common::hash::PyHash; use crate::common::lock::PyMutex; @@ -954,6 +954,33 @@ impl PyFunction { Ok(()) } + #[pygetset] + fn __dict__(zelf: &Py, vm: &VirtualMachine) -> PyDictRef { + zelf.instance_dict() + .map(|d| d.get_or_insert(vm)) + .unwrap_or_else(|| vm.ctx.new_dict()) + } + + #[pygetset(setter)] + fn set___dict__(zelf: &Py, value: PySetterValue, vm: &VirtualMachine) -> PyResult<()> { + match value { + PySetterValue::Assign(obj) => { + let dict = obj.downcast::().map_err(|_| { + vm.new_type_error(format!( + "__dict__ must be set to a dictionary, not a '{}'", + obj.class().name() + )) + })?; + zelf.set_dict(Some(dict)).map_err(|_| { + vm.new_attribute_error("function object has no __dict__") + }) + } + PySetterValue::Delete => Err(vm.new_type_error("cannot delete __dict__")), + } + } + + + #[pygetset] fn __annotate__(&self, vm: &VirtualMachine) -> PyObjectRef { self.annotate diff --git a/crates/vm/src/builtins/object.rs b/crates/vm/src/builtins/object.rs index 002b05d38f1..5599435706c 100644 --- a/crates/vm/src/builtins/object.rs +++ b/crates/vm/src/builtins/object.rs @@ -554,10 +554,24 @@ impl PyBaseObject { } pub fn object_get_dict(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { - obj.dict() - .ok_or_else(|| vm.new_attribute_error("This object has no __dict__")) + if let Some(dict) = obj.dict() { + Ok(dict) + } else { + match obj.instance_dict() { + Some(d) => Ok(d.get_or_insert(vm)), + None => Err(vm.new_attribute_error("This object has no __dict__")), + } + } } -pub fn object_set_dict(obj: PyObjectRef, dict: PyDictRef, vm: &VirtualMachine) -> PyResult<()> { +pub fn object_set_dict( + obj: PyObjectRef, + value: PySetterValue, + vm: &VirtualMachine, +) -> PyResult<()> { + let dict = match value { + PySetterValue::Assign(dict) => Some(dict), + PySetterValue::Delete => None, + }; obj.set_dict(dict) .map_err(|_| vm.new_attribute_error("This object has no __dict__")) } diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 64801a1de5c..75731a4e72f 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -2622,7 +2622,7 @@ fn subtype_get_dict(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { } // = subtype_setdict -fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { +fn subtype_set_dict(obj: PyObjectRef, value: PySetterValue, vm: &VirtualMachine) -> PyResult<()> { let base = get_builtin_base_with_dict(obj.class(), vm); if let Some(base_type) = base { @@ -2634,13 +2634,14 @@ fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) - .descr_set .load() .ok_or_else(|| raise_dict_descriptor_error(&obj, vm))?; - descr_set(&descr, obj, PySetterValue::Assign(value), vm) + descr_set(&descr, obj, value, vm) } else { Err(raise_dict_descriptor_error(&obj, vm)) } } else { // PyObject_GenericSetDict - object::object_set_dict(obj, value.try_into_value(vm)?, vm)?; + let value = value.map(|v| v.try_into_value(vm)).transpose()?; + object::object_set_dict(obj, value, vm)?; Ok(()) } } diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index a8d7c09da89..bee32f543a2 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -307,9 +307,13 @@ pub(super) struct ObjExt { } impl ObjExt { - fn new(dict: Option, member_count: usize) -> Self { + fn new(dict: Option, member_count: usize, has_dict: bool) -> Self { Self { - dict: dict.map(InstanceDict::new), + dict: if has_dict { + Some(InstanceDict::from_opt(dict)) + } else { + None + }, slots: core::iter::repeat_with(|| PyRwLock::new(None)) .take(member_count) .collect_vec() @@ -909,8 +913,8 @@ impl Py { } #[derive(Debug)] -pub(super) struct InstanceDict { - pub(super) d: PyRwLock, +pub(crate) struct InstanceDict { + pub(crate) d: PyRwLock>, } impl From for InstanceDict { @@ -923,30 +927,45 @@ impl From for InstanceDict { impl InstanceDict { #[inline] pub const fn new(d: PyDictRef) -> Self { + Self { + d: PyRwLock::new(Some(d)), + } + } + + #[inline] + pub const fn from_opt(d: Option) -> Self { Self { d: PyRwLock::new(d), } } #[inline] - pub fn get(&self) -> PyDictRef { + pub fn get(&self) -> Option { self.d.read().clone() } #[inline] - pub fn set(&self, d: PyDictRef) { + pub fn set(&self, d: Option) { self.replace(d); } #[inline] - pub fn replace(&self, d: PyDictRef) -> PyDictRef { + pub fn replace(&self, d: Option) -> Option { core::mem::replace(&mut self.d.write(), d) } - /// Consume the InstanceDict and return the inner PyDictRef. - #[inline] - pub fn into_inner(self) -> PyDictRef { - self.d.into_inner() + pub(crate) fn get_or_insert(&self, vm: &VirtualMachine) -> PyDictRef { + if let Some(existing) = self.d.read().as_ref() { + return existing.clone(); + } + let dict = vm.ctx.new_dict(); + let mut d = self.d.write(); + if let Some(existing) = d.as_ref() { + existing.clone() + } else { + *d = Some(dict.clone()); + dict + } } } @@ -1060,7 +1079,11 @@ impl PyInner { unsafe { if let Some(offset) = ext_start { let ext_ptr = alloc_ptr.add(offset) as *mut ObjExt; - ext_ptr.write(ObjExt::new(dict, member_count)); + let has_dict = typ + .slots + .flags + .has_feature(crate::types::PyTypeFlags::HAS_DICT); + ext_ptr.write(ObjExt::new(dict, member_count, has_dict)); } if let Some(offset) = weakref_start { @@ -1451,18 +1474,18 @@ impl PyObject { } #[inline(always)] - fn instance_dict(&self) -> Option<&InstanceDict> { + pub(crate) fn instance_dict(&self) -> Option<&InstanceDict> { self.0.ext_ref().and_then(|ext| ext.dict.as_ref()) } #[inline(always)] pub fn dict(&self) -> Option { - self.instance_dict().map(|d| d.get()) + self.instance_dict().and_then(|d| d.get()) } /// Set the dict field. Returns `Err(dict)` if this object does not have a dict field /// in the first place. - pub fn set_dict(&self, dict: PyDictRef) -> Result<(), PyDictRef> { + pub fn set_dict(&self, dict: Option) -> Result<(), Option> { match self.instance_dict() { Some(d) => { d.set(dict); @@ -1774,9 +1797,7 @@ impl PyObject { let ext_ptr = core::ptr::with_exposed_provenance_mut::(self_addr.wrapping_sub(offset)); let ext = unsafe { &mut *ext_ptr }; - if let Some(old_dict) = ext.dict.take() { - // Get the dict ref before dropping InstanceDict - let dict_ref = old_dict.into_inner(); + if let Some(dict_ref) = ext.dict.as_ref().and_then(|d| d.replace(None)) { result.push(dict_ref.into()); } for slot in ext.slots.iter() { @@ -2444,7 +2465,7 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef, PyTypeRef) { unsafe { let ext_ptr = alloc_ptr as *mut ObjExt; - ext_ptr.write(ObjExt::new(None, 0)); + ext_ptr.write(ObjExt::new(None, 0, true)); let weakref_ptr = alloc_ptr.add(weakref_offset) as *mut WeakRefList; weakref_ptr.write(WeakRefList::new()); diff --git a/crates/vm/src/protocol/object.rs b/crates/vm/src/protocol/object.rs index e59a1f15a6f..79458d50a54 100644 --- a/crates/vm/src/protocol/object.rs +++ b/crates/vm/src/protocol/object.rs @@ -199,10 +199,12 @@ impl PyObject { } } - if let Some(dict) = self.dict() { + if let Some(instance_dict) = self.instance_dict() { if let PySetterValue::Assign(value) = value { - dict.set_item(attr_name, value, vm)?; - } else { + instance_dict + .get_or_insert(vm) + .set_item(attr_name, value, vm)?; + } else if let Some(dict) = instance_dict.get() { dict.del_item(attr_name, vm).map_err(|e| { if e.fast_isinstance(vm.ctx.exceptions.key_error) { vm.new_no_attribute_error(self.to_owned(), attr_name.to_owned()) @@ -210,6 +212,8 @@ impl PyObject { e } })?; + } else { + return Err(vm.new_no_attribute_error(self.to_owned(), attr_name.to_owned())); } Ok(()) } else { diff --git a/crates/vm/src/stdlib/_functools.rs b/crates/vm/src/stdlib/_functools.rs index 494f0e7fd83..d6837489e1a 100644 --- a/crates/vm/src/stdlib/_functools.rs +++ b/crates/vm/src/stdlib/_functools.rs @@ -4,9 +4,9 @@ pub(crate) use _functools::module_def; mod _functools { use crate::{ Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, - builtins::{PyBoundMethod, PyDict, PyGenericAlias, PyTuple, PyType, PyTypeRef}, + builtins::{PyBoundMethod, PyDict, PyDictRef, PyGenericAlias, PyTuple, PyType, PyTypeRef}, common::lock::PyRwLock, - function::{FuncArgs, KwArgs, OptionalOption}, + function::{FuncArgs, KwArgs, OptionalOption, PySetterValue}, object::AsObject, protocol::PyIter, pyclass, @@ -158,6 +158,32 @@ mod _functools { self.inner.read().keywords.clone() } + #[pygetset] + fn __dict__(zelf: &Py, vm: &VirtualMachine) -> PyDictRef { + zelf.instance_dict() + .map(|d| d.get_or_insert(vm)) + .unwrap_or_else(|| vm.ctx.new_dict()) + } + + #[pygetset(setter)] + fn set___dict__(zelf: &Py, value: PySetterValue, vm: &VirtualMachine) -> PyResult<()> { + match value { + PySetterValue::Assign(obj) => { + let dict = obj.downcast::().map_err(|_| { + vm.new_type_error(format!( + "__dict__ must be set to a dictionary, not a '{}'", + obj.class().name() + )) + })?; + zelf.set_dict(Some(dict)).map_err(|_| { + vm.new_attribute_error("partial object has no __dict__") + }) + } + PySetterValue::Delete => Err(vm.new_type_error("cannot delete __dict__")), + } + } + + #[pymethod] fn __reduce__(zelf: &Py, vm: &VirtualMachine) -> PyResult { let inner = zelf.inner.read(); diff --git a/extra_tests/snippets/test_del_dict.py b/extra_tests/snippets/test_del_dict.py new file mode 100644 index 00000000000..965699b6054 --- /dev/null +++ b/extra_tests/snippets/test_del_dict.py @@ -0,0 +1,41 @@ +# Test that del obj.__dict__ works and lazy creation happens +class C: + pass + + +obj = C() +obj.x = 42 + +# Delete __dict__ +del obj.__dict__ + +# After deletion, accessing __dict__ should return a new empty dict +d = obj.__dict__ +assert isinstance(d, dict) +assert len(d) == 0 + +# Old attribute should be gone +try: + obj.x + assert False, "AttributeError expected" +except AttributeError: + pass + +# Function __dict__ deletion should fail +def f(): pass +try: + del f.__dict__ + assert False, "TypeError expected for function dict deletion" +except TypeError: + pass + +# functools.partial __dict__ deletion should fail +import functools +p = functools.partial(f) +try: + del p.__dict__ + assert False, "TypeError expected for partial dict deletion" +except TypeError: + pass + +print("OK")