X Tutup
Skip to content

Resolve del f.__dict__ incompatibility #5508

Closed
key262yek wants to merge 7 commits intoRustPython:mainfrom
key262yek:impl_del_object_set_dict
Closed

Resolve del f.__dict__ incompatibility #5508
key262yek wants to merge 7 commits intoRustPython:mainfrom
key262yek:impl_del_object_set_dict

Conversation

@key262yek
Copy link
Contributor

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.

  1. 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?
  1. Make simple error type and return it.
  2. Return Option not Result
  3. Add something to delete_dict's parameter and return it.

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

key262yek and others added 7 commits January 10, 2025 11:48
- Update object_set_dict to use new PySetterValue variants
- Modify getset.rs to handle PySetterValue::Delete
- Add delete_dict method to PyObject
- Adjust type.rs to use new PySetterValue semantics
@key262yek key262yek force-pushed the impl_del_object_set_dict branch 2 times, most recently from 7ac39d8 to 20c053a Compare February 10, 2025 01:31
@key262yek key262yek closed this Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup