Implement `del object.__dict__` by key262yek · Pull Request #5509 · RustPython/RustPython

key262yek · 2025-02-10T01:35:15Z

I add object::delete_dict() which changes Object's dict to None, and add PySetterValue::Delete match case in object::object_set_dict.
All things fine I think, but minor format issue remains.

    pub fn delete_dict(&self) -> Result<(), ()> {
        match self.instance_dict() {
            Some(_) => {
                unsafe {
                    let ptr = self as *const _ as *mut PyObject;
                    (*ptr).0.dict = None;
                }
                Ok(())
            }
            None => Err(()),
        }
    }

In this code, since Err can be returned, we should define custom Error type.
I made several options for it.

Return Ok(()) when it does not have dict already

Can it satisfy normal convention? or I should raise error when the object to be deleted does not exist?

Make simple error type and return it.
Return Option not Result
Add something to delete_dict's parameter and return it.

I cannot decide which one is better. Can you suggest about it?

- Add support for deleting object dictionaries - Modify object_set_dict to handle dictionary deletion - Update setter value handling to support delete operations

youknowone

Thanks!

youknowone · 2025-02-10T07:23:01Z

vm/src/object/core.rs

        }
    }

+    pub fn delete_dict(&self) -> Result<(), ()> {


set_dict in core.rs is only used for object_set_dict. Rather than adding delete_dict, I prefer to add this logic into set_dict if other problems not found.

- Simplify object_set_dict method in object.rs - Update set_dict method in core.rs to handle both assignment and deletion - Consolidate dictionary modification logic

- Modify set_dict method to allow assigning dict to objects without existing dict - Update error handling in object_set_dict to provide more precise error message - Refactor type module to correctly handle dictionary assignment

key262yek

this problem is more complex than I thought at first.

key262yek · 2025-02-15T04:24:57Z

vm/src/function/getset.rs

+        match obj {
+            PySetterValue::Assign(obj) => T::try_from_object(vm, obj),
+            PySetterValue::Delete => T::try_from_object(vm, vm.ctx.none()),
+        }


Affect multiple attribute in getset.rs
By this code change, the test Lib/test/test_exceptions.py:620 testInvalidAttrs fails

def testInvalidAttrs(self): self.assertRaises(TypeError, setattr, Exception(), '__cause__', 1) self.assertRaises(TypeError, delattr, Exception(), '__cause__') self.assertRaises(TypeError, setattr, Exception(), '__context__', 1) self.assertRaises(TypeError, delattr, Exception(), '__context__')

This is because FromPySetterValue trait affect not only dict but also other attributes.

I found only one way to restrict deletion of attribute except __dict__ by editing following part

// vm/src/builtins/getset.rs: 97 #[pyslot] fn descr_set( zelf: &PyObject, obj: PyObjectRef, value: PySetterValue<PyObjectRef>, vm: &VirtualMachine, ) -> PyResult<()> { let zelf = zelf.try_to_ref::<Self>(vm)?; if let Some(ref f) = zelf.setter { match value { PySetterValue::Assign(v) => f(vm, obj, PySetterValue::Assign(v)), PySetterValue::Delete if zelf.name == "__dict__" => { f(vm, obj, PySetterValue::Delete) } _ => Err(vm.new_type_error("can't delete attribute".to_owned())), } } else { Err(vm.new_attribute_error(format!( "attribute '{}' of '{}' objects is not writable", zelf.name, obj.class().name() ))) } }

key262yek · 2025-02-15T05:41:52Z

vm/src/builtins/type.rs

        }
        None => {
-            object::object_set_dict(obj, value.try_into_value(vm)?, vm)?;
+            object::object_set_dict(obj, PySetterValue::Assign(value.try_into_value(vm)?), vm)?;


Cannot resolve Lib/test/test_descr.py:3399 test_set_dict
If I don't change code like this, object_set_dict fails to be compiled because now value must be the type PySetterValue<PyDictRef>.
There are two option for it, pass the value PySetterValue::Assign or PySetterValue::Delete

For the case of Assign I get

ERROR: test_set_dict (__main__.ClassPropertiesAndMethods.test_set_dict) ---------------------------------------------------------------------- Traceback (most recent call last): File "Lib/test/test_descr.py", line 3415, in test_set_dict del a.__dict__ # Deleting __dict__ is allowed TypeError: Expected type 'dict' but 'NoneType' found.

For Delete case,

ERROR: test_set_dict (__main__.ClassPropertiesAndMethods.test_set_dict) ---------------------------------------------------------------------- Traceback (most recent call last): File "Lib/test/test_descr.py", line 3404, in test_set_dict self.assertEqual(a.b, 1) AttributeError: 'C' object has no attribute 'b'

Test function

def test_set_dict(self): # Testing __dict__ assignment... class C(object): pass a = C() a.__dict__ = {'b': 1} self.assertEqual(a.b, 1) def cant(x, dict): try: x.__dict__ = dict except (AttributeError, TypeError): pass else: self.fail("shouldn't allow %r.__dict__ = %r" % (x, dict)) cant(a, None) cant(a, []) cant(a, 1) del a.__dict__ # Deleting __dict__ is allowed

Clarified problem is this.

If I try to delete __dict__, following methods are called.

(getset.rs:164) set - PySetterValue::Delete becomes None

impl<F, T, V, R> IntoPySetterFunc<(OwnedParam<T>, V, R, VirtualMachine)> for F where F: Fn(T, V, &VirtualMachine) -> R + 'static + Send + Sync, T: TryFromObject, V: FromPySetterValue, R: IntoPyNoResult, { fn set(&self, obj: PyObjectRef, value: PySetterValue, vm: &VirtualMachine) -> PyResult<()> { let obj = T::try_from_object(vm, obj)?; let value = V::from_setter_value(vm, value)?; // None = V::from_setter_value(vm, PySetterValue::Delete)?; (self)(obj, value, vm).into_noresult() // subtype_set_dict(obj, None, vm) } } impl<T> FromPySetterValue for T where T: Sized + TryFromObject, { #[inline] fn from_setter_value(vm: &VirtualMachine, obj: PySetterValue) -> PyResult<Self> { match obj { PySetterValue::Assign(obj) => T::try_from_object(vm, obj), PySetterValue::Delete => T::try_from_object(vm, vm.ctx.none()), } } }

(type.rs:1275) subtype_set_dict - value should be PySetterValue again.

fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let cls = obj.class(); match find_base_dict_descr(cls, vm) { Some(descr) => { let descr_set = descr .class() .mro_find_map(|cls| cls.slots.descr_set.load()) .ok_or_else(|| { vm.new_type_error(format!( "this __dict__ descriptor does not support '{}' objects", cls.name() )) })?; descr_set(&descr, obj, PySetterValue::Assign(value), vm) } None => { object::object_set_dict(obj, PySetterValue::Assign(value.try_into_value(vm)?), vm)?; Ok(()) } } }

Since the first V::from_setter_value(vm, value) give same result for value = PySetterValue::Assign(None) and value = PySetterValue::Delete to None, This kind of error occurs.

This PR is based on the work by key262yek. The original PR is here: RustPython#5509

Implement delete object dictionary

c40d24e

- Add support for deleting object dictionaries - Modify object_set_dict to handle dictionary deletion - Update setter value handling to support delete operations

youknowone reviewed Feb 10, 2025

View reviewed changes

key262yek added 2 commits February 15, 2025 00:12

Refactor object dictionary setter to handle delete and assign operations

dc0a75e

- Simplify object_set_dict method in object.rs - Update set_dict method in core.rs to handle both assignment and deletion - Consolidate dictionary modification logic

key262yek commented Feb 15, 2025

View reviewed changes

youknowone mentioned this pull request Mar 2, 2025

Properly handles del val.__dict__. #5576

Open

hbina added a commit to hbina/RustPython that referenced this pull request Mar 2, 2025

Implemented the ability to del __dict__

16113a4

This PR is based on the work by key262yek. The original PR is here: RustPython#5509

hbina added a commit to hbina/RustPython that referenced this pull request Mar 8, 2025

Implemented the ability to del __dict__

e25d2b0

This PR is based on the work by key262yek. The original PR is here: RustPython#5509

hbina added a commit to hbina/RustPython that referenced this pull request Mar 23, 2025

Implemented the ability to del __dict__

2010890

This PR is based on the work by key262yek. The original PR is here: RustPython#5509

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `del object.dict`#5509

Implement `del object.dict`#5509
key262yek wants to merge 3 commits intoRustPython:mainfrom
key262yek:impl_del_object_dict

key262yek commented Feb 10, 2025

Uh oh!

youknowone left a comment

Uh oh!

youknowone Feb 10, 2025

Uh oh!

key262yek left a comment

Uh oh!

key262yek Feb 15, 2025

Uh oh!

key262yek Feb 15, 2025

Uh oh!

key262yek Feb 15, 2025

Uh oh!

key262yek Feb 15, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

key262yek commented Feb 10, 2025

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

youknowone Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

key262yek left a comment

Choose a reason for hiding this comment

Uh oh!

key262yek Feb 15, 2025

Choose a reason for hiding this comment

Uh oh!

key262yek Feb 15, 2025

Choose a reason for hiding this comment

Uh oh!

key262yek Feb 15, 2025

Choose a reason for hiding this comment

Uh oh!

key262yek Feb 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

key262yek Feb 15, 2025 •

edited

Loading