PySetterValue for descr_set and setattro

youknowone · youknowone · commit a363b053402a · 2022-08-08T04:31:14.000+09:00
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
@@ -586,8 +586,6 @@ def testInvalidTraceback(self):
         else:
             self.fail("No exception raised")
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def testInvalidAttrs(self):
         self.assertRaises(TypeError, setattr, Exception(), '__cause__', 1)
         self.assertRaises(TypeError, delattr, Exception(), '__cause__')
diff --git a/vm/Cargo.toml b/vm/Cargo.toml
@@ -52,7 +52,7 @@ libc = "0.2.126"
 nix = "0.23.1"
 paste = "1.0.7"
 is-macro = "0.2.0"
-result-like ="0.4.4"
+result-like = "0.4.5"
 num_enum = "0.5.7"
 bstr = "0.2.17"
 crossbeam-utils = "0.8.8"
diff --git a/vm/src/builtins/frame.rs b/vm/src/builtins/frame.rs
@@ -2,10 +2,11 @@
 
 */
 
-use super::{PyCode, PyDictRef, PyStrRef};
+use super::{PyCode, PyDictRef};
 use crate::{
     class::PyClassImpl,
     frame::{Frame, FrameRef},
+    function::PySetterValue,
     types::{Constructor, Unconstructible},
     AsObject, Context, PyObjectRef, PyRef, PyResult, VirtualMachine,
 };
@@ -25,15 +26,6 @@ impl FrameRef {
         "<frame object at .. >".to_owned()
     }
 
-    #[pymethod(magic)]
-    fn delattr(self, value: PyStrRef, vm: &VirtualMachine) {
-        // CPython' Frame.f_trace is set to None when deleted.
-        // The strange behavior is mimicked here make bdb.py happy about it.
-        if value.to_string() == "f_trace" {
-            self.set_f_trace(vm.ctx.none());
-        };
-    }
-
     #[pymethod]
     fn clear(self) {
         // TODO
@@ -86,8 +78,8 @@ impl FrameRef {
     }
 
     #[pyproperty(setter)]
-    fn set_f_trace(self, value: PyObjectRef) {
+    fn set_f_trace(self, value: PySetterValue, vm: &VirtualMachine) {
         let mut storage = self.trace.lock();
-        *storage = value;
+        *storage = value.unwrap_or_none(vm);
     }
 }
diff --git a/vm/src/builtins/getset.rs b/vm/src/builtins/getset.rs
@@ -4,7 +4,7 @@
 use super::PyType;
 use crate::{
     class::PyClassImpl,
-    function::{IntoPyGetterFunc, IntoPySetterFunc, PyGetterFunc, PySetterFunc},
+    function::{IntoPyGetterFunc, IntoPySetterFunc, PyGetterFunc, PySetterFunc, PySetterValue},
     types::{Constructor, GetDescriptor, Unconstructible},
     AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
 };
@@ -102,12 +102,12 @@ impl PyGetSet {
     fn descr_set(
         zelf: PyObjectRef,
         obj: PyObjectRef,
-        value: Option<PyObjectRef>,
+        value: PySetterValue<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         let zelf = PyRef::<Self>::try_from_object(vm, zelf)?;
         if let Some(ref f) = zelf.setter {
-            f(vm, obj, value.into())
+            f(vm, obj, value)
         } else {
             Err(vm.new_attribute_error(format!(
                 "attribute '{}' of '{}' objects is not writable",
@@ -123,11 +123,11 @@ impl PyGetSet {
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        Self::descr_set(zelf, obj, Some(value), vm)
+        Self::descr_set(zelf, obj, PySetterValue::Assign(value), vm)
     }
     #[pymethod]
     fn __delete__(zelf: PyObjectRef, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        Self::descr_set(zelf, obj, None, vm)
+        Self::descr_set(zelf, obj, PySetterValue::Delete, vm)
     }
 
     #[pyproperty(magic)]
diff --git a/vm/src/builtins/object.rs b/vm/src/builtins/object.rs
@@ -2,8 +2,7 @@ use super::{PyDict, PyDictRef, PyList, PyStr, PyStrRef, PyType, PyTypeRef};
 use crate::common::hash::PyHash;
 use crate::{
     class::PyClassImpl,
-    function::Either,
-    function::{FuncArgs, PyArithmeticValue, PyComparisonValue},
+    function::{Either, FuncArgs, PyArithmeticValue, PyComparisonValue, PySetterValue},
     types::PyComparisonOp,
     AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine,
 };
@@ -162,20 +161,20 @@ impl PyBaseObject {
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        obj.generic_setattr(name, Some(value), vm)
+        obj.generic_setattr(name, PySetterValue::Assign(value), vm)
     }
 
     /// Implement delattr(self, name).
     #[pymethod]
     fn __delattr__(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
-        obj.generic_setattr(name, None, vm)
+        obj.generic_setattr(name, PySetterValue::Delete, vm)
     }
 
     #[pyslot]
     fn slot_setattro(
         obj: &PyObject,
         attr_name: PyStrRef,
-        value: Option<PyObjectRef>,
+        value: PySetterValue,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         obj.generic_setattr(attr_name, value, vm)
diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs
@@ -5,7 +5,7 @@ use super::{PyType, PyTypeRef};
 use crate::common::lock::PyRwLock;
 use crate::{
     class::PyClassImpl,
-    function::FuncArgs,
+    function::{FuncArgs, PySetterValue},
     types::{Constructor, GetDescriptor, Initializer},
     AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
 };
@@ -95,19 +95,19 @@ impl PyProperty {
     fn descr_set(
         zelf: PyObjectRef,
         obj: PyObjectRef,
-        value: Option<PyObjectRef>,
+        value: PySetterValue,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         let zelf = PyRef::<Self>::try_from_object(vm, zelf)?;
         match value {
-            Some(value) => {
+            PySetterValue::Assign(value) => {
                 if let Some(setter) = zelf.setter.read().as_ref() {
                     vm.invoke(setter, (obj, value)).map(drop)
                 } else {
                     Err(vm.new_attribute_error("can't set attribute".to_owned()))
                 }
             }
-            None => {
+            PySetterValue::Delete => {
                 if let Some(deleter) = zelf.deleter.read().as_ref() {
                     vm.invoke(deleter, (obj,)).map(drop)
                 } else {
@@ -123,11 +123,11 @@ impl PyProperty {
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        Self::descr_set(zelf, obj, Some(value), vm)
+        Self::descr_set(zelf, obj, PySetterValue::Assign(value), vm)
     }
     #[pymethod]
     fn __delete__(zelf: PyObjectRef, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        Self::descr_set(zelf, obj, None, vm)
+        Self::descr_set(zelf, obj, PySetterValue::Delete, vm)
     }
 
     // Access functions
diff --git a/vm/src/builtins/type.rs b/vm/src/builtins/type.rs
@@ -10,7 +10,7 @@ use crate::common::{
 use crate::{
     builtins::PyBaseExceptionRef,
     class::{PyClassImpl, StaticType},
-    function::{FuncArgs, KwArgs, OptionalArg},
+    function::{FuncArgs, KwArgs, OptionalArg, PySetterValue},
     identifier,
     protocol::PyNumberMethods,
     types::{Callable, GetAttr, PyTypeFlags, PyTypeSlots, SetAttr},
@@ -757,7 +757,7 @@ impl SetAttr for PyType {
     fn setattro(
         zelf: &crate::Py<Self>,
         attr_name: PyStrRef,
-        value: Option<PyObjectRef>,
+        value: PySetterValue,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         // TODO: pass PyRefExact instead of &str
@@ -768,10 +768,10 @@ impl SetAttr for PyType {
                 return descriptor(attr, zelf.to_owned().into(), value, vm);
             }
         }
-        let assign = value.is_some();
+        let assign = value.is_assign();
 
         let mut attributes = zelf.attributes.write();
-        if let Some(value) = value {
+        if let PySetterValue::Assign(value) = value {
             attributes.insert(attr_name, value);
         } else {
             let prev_value = attributes.remove(attr_name);
@@ -848,7 +848,7 @@ fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -
                         cls.name()
                     ))
                 })?;
-            descr_set(descr, obj, Some(value), vm)
+            descr_set(descr, obj, PySetterValue::Assign(value), vm)
         }
         None => {
             object::object_set_dict(obj, value.try_into_value(vm)?, vm)?;
diff --git a/vm/src/builtins/weakproxy.rs b/vm/src/builtins/weakproxy.rs
@@ -1,7 +1,7 @@
 use super::{PyStrRef, PyType, PyTypeRef, PyWeak};
 use crate::{
     class::PyClassImpl,
-    function::{OptionalArg, PyComparisonValue},
+    function::{OptionalArg, PyComparisonValue, PySetterValue},
     protocol::{PyMappingMethods, PySequence, PySequenceMethods},
     types::{AsMapping, AsSequence, Comparable, Constructor, GetAttr, PyComparisonOp, SetAttr},
     Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
@@ -134,7 +134,7 @@ impl SetAttr for PyWeakProxy {
     fn setattro(
         zelf: &crate::Py<Self>,
         attr_name: PyStrRef,
-        value: Option<PyObjectRef>,
+        value: PySetterValue,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         let obj = zelf.try_upgrade(vm)?;
diff --git a/vm/src/function/getset.rs b/vm/src/function/getset.rs
@@ -8,12 +8,21 @@ use crate::{
     PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
 };
 
-#[derive(result_like::OptionLike)]
+#[derive(result_like::OptionLike, is_macro::Is)]
 pub enum PySetterValue<T = PyObjectRef> {
     Assign(T),
     Delete,
 }
 
+impl PySetterValue {
+    pub fn unwrap_or_none(self, vm: &VirtualMachine) -> PyObjectRef {
+        match self {
+            Self::Assign(value) => value,
+            Self::Delete => vm.ctx.none(),
+        }
+    }
+}
+
 trait FromPySetterValue
 where
     Self: Sized,
@@ -25,6 +34,7 @@ impl<T> FromPySetterValue for T
 where
     T: Sized + TryFromObject,
 {
+    #[inline]
     fn from_setter_value(vm: &VirtualMachine, obj: PySetterValue) -> PyResult<Self> {
         let obj = obj.ok_or_else(|| vm.new_type_error("can't delete attribute".to_owned()))?;
         T::try_from_object(vm, obj)
@@ -35,8 +45,9 @@ impl<T> FromPySetterValue for PySetterValue<T>
 where
     T: Sized + TryFromObject,
 {
+    #[inline]
     fn from_setter_value(vm: &VirtualMachine, obj: PySetterValue) -> PyResult<Self> {
-        Ok(obj.map(|obj| T::try_from_object(vm, obj)).transpose()?)
+        obj.map(|obj| T::try_from_object(vm, obj)).transpose()
     }
 }
 
diff --git a/vm/src/protocol/object.rs b/vm/src/protocol/object.rs
@@ -10,8 +10,7 @@ use crate::{
     common::{hash::PyHash, str::to_ascii},
     convert::{ToPyObject, ToPyResult},
     dictdatatype::DictKey,
-    function::Either,
-    function::{OptionalArg, PyArithmeticValue},
+    function::{Either, OptionalArg, PyArithmeticValue, PySetterValue},
     protocol::{PyIter, PyMapping, PySequence},
     types::{Constructor, PyComparisonOp},
     AsObject, PyObject, PyObjectRef, PyResult, TryFromObject, VirtualMachine,
@@ -104,19 +103,22 @@ impl PyObject {
         &self,
         vm: &VirtualMachine,
         attr_name: PyStrRef,
-        attr_value: Option<PyObjectRef>,
+        attr_value: PySetterValue,
     ) -> PyResult<()> {
         let setattro = {
             let cls = self.class();
             cls.mro_find_map(|cls| cls.slots.setattro.load())
                 .ok_or_else(|| {
-                    let assign = attr_value.is_some();
                     let has_getattr = cls.mro_find_map(|cls| cls.slots.getattro.load()).is_some();
                     vm.new_type_error(format!(
                         "'{}' object has {} attributes ({} {})",
                         cls.name(),
                         if has_getattr { "only read-only" } else { "no" },
-                        if assign { "assign to" } else { "del" },
+                        if attr_value.is_assign() {
+                            "assign to"
+                        } else {
+                            "del"
+                        },
                         attr_name
                     ))
                 })?
@@ -131,15 +133,15 @@ impl PyObject {
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         let attr_name = attr_name.into_pystr_ref(vm);
-        self.call_set_attr(vm, attr_name, Some(attr_value.into()))
+        self.call_set_attr(vm, attr_name, PySetterValue::Assign(attr_value.into()))
     }
 
     // int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value)
     #[cfg_attr(feature = "flame-it", flame)]
     pub fn generic_setattr(
         &self,
         attr_name: PyStrRef, // TODO: Py<PyStr>
-        value: Option<PyObjectRef>,
+        value: PySetterValue,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);
@@ -155,7 +157,7 @@ impl PyObject {
         }
 
         if let Some(dict) = self.dict() {
-            if let Some(value) = value {
+            if let PySetterValue::Assign(value) = value {
                 dict.set_item(&*attr_name, value, vm)?;
             } else {
                 dict.del_item(&*attr_name, vm).map_err(|e| {
@@ -240,7 +242,7 @@ impl PyObject {
 
     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)
+        self.call_set_attr(vm, attr_name, PySetterValue::Delete)
     }
 
     // Perform a comparison, raising TypeError when the requested comparison
diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs
diff --git a/vm/src/stdlib/thread.rs b/vm/src/stdlib/thread.rs
diff --git a/vm/src/types/slot.rs b/vm/src/types/slot.rs
diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs
