Skip to content

Commit ade5549

Browse files
authored
Merge pull request #4010 from youknowone/fix-setter-interface
Fix getset setter to take None for deleter
2 parents 9087c2f + a363b05 commit ade5549

17 files changed

Lines changed: 275 additions & 220 deletions

File tree

Cargo.lock

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Lib/test/test_exceptions.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,6 @@ def testInvalidTraceback(self):
586586
else:
587587
self.fail("No exception raised")
588588

589-
# TODO: RUSTPYTHON
590-
@unittest.expectedFailure
591589
def testInvalidAttrs(self):
592590
self.assertRaises(TypeError, setattr, Exception(), '__cause__', 1)
593591
self.assertRaises(TypeError, delattr, Exception(), '__cause__')

vm/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ libc = "0.2.126"
5252
nix = "0.23.1"
5353
paste = "1.0.7"
5454
is-macro = "0.2.0"
55-
result-like = "0.4.2"
55+
result-like = "0.4.5"
5656
num_enum = "0.5.7"
5757
bstr = "0.2.17"
5858
crossbeam-utils = "0.8.8"

vm/src/builtins/frame.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
33
*/
44

5-
use super::{PyCode, PyDictRef, PyStrRef};
5+
use super::{PyCode, PyDictRef};
66
use crate::{
77
class::PyClassImpl,
88
frame::{Frame, FrameRef},
9+
function::PySetterValue,
910
types::{Constructor, Unconstructible},
1011
AsObject, Context, PyObjectRef, PyRef, PyResult, VirtualMachine,
1112
};
@@ -25,15 +26,6 @@ impl FrameRef {
2526
"<frame object at .. >".to_owned()
2627
}
2728

28-
#[pymethod(magic)]
29-
fn delattr(self, value: PyStrRef, vm: &VirtualMachine) {
30-
// CPython' Frame.f_trace is set to None when deleted.
31-
// The strange behavior is mimicked here make bdb.py happy about it.
32-
if value.to_string() == "f_trace" {
33-
self.set_f_trace(vm.ctx.none());
34-
};
35-
}
36-
3729
#[pymethod]
3830
fn clear(self) {
3931
// TODO
@@ -86,8 +78,8 @@ impl FrameRef {
8678
}
8779

8880
#[pyproperty(setter)]
89-
fn set_f_trace(self, value: PyObjectRef) {
81+
fn set_f_trace(self, value: PySetterValue, vm: &VirtualMachine) {
9082
let mut storage = self.trace.lock();
91-
*storage = value;
83+
*storage = value.unwrap_or_none(vm);
9284
}
9385
}

vm/src/builtins/getset.rs

Lines changed: 5 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -4,153 +4,11 @@
44
use super::PyType;
55
use crate::{
66
class::PyClassImpl,
7-
convert::ToPyResult,
8-
function::{OwnedParam, RefParam},
9-
object::PyThreadingConstraint,
7+
function::{IntoPyGetterFunc, IntoPySetterFunc, PyGetterFunc, PySetterFunc, PySetterValue},
108
types::{Constructor, GetDescriptor, Unconstructible},
119
AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
1210
};
1311

14-
pub type PyGetterFunc = Box<py_dyn_fn!(dyn Fn(&VirtualMachine, PyObjectRef) -> PyResult)>;
15-
pub type PySetterFunc =
16-
Box<py_dyn_fn!(dyn Fn(&VirtualMachine, PyObjectRef, PyObjectRef) -> PyResult<()>)>;
17-
18-
pub trait IntoPyGetterFunc<T>: PyThreadingConstraint + Sized + 'static {
19-
fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult;
20-
fn into_getter(self) -> PyGetterFunc {
21-
Box::new(move |vm, obj| self.get(obj, vm))
22-
}
23-
}
24-
25-
impl<F, T, R> IntoPyGetterFunc<(OwnedParam<T>, R, VirtualMachine)> for F
26-
where
27-
F: Fn(T, &VirtualMachine) -> R + 'static + Send + Sync,
28-
T: TryFromObject,
29-
R: ToPyResult,
30-
{
31-
fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
32-
let obj = T::try_from_object(vm, obj)?;
33-
(self)(obj, vm).to_pyresult(vm)
34-
}
35-
}
36-
37-
impl<F, S, R> IntoPyGetterFunc<(RefParam<S>, R, VirtualMachine)> for F
38-
where
39-
F: Fn(&S, &VirtualMachine) -> R + 'static + Send + Sync,
40-
S: PyPayload,
41-
R: ToPyResult,
42-
{
43-
fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
44-
let zelf = PyRef::<S>::try_from_object(vm, obj)?;
45-
(self)(&zelf, vm).to_pyresult(vm)
46-
}
47-
}
48-
49-
impl<F, T, R> IntoPyGetterFunc<(OwnedParam<T>, R)> for F
50-
where
51-
F: Fn(T) -> R + 'static + Send + Sync,
52-
T: TryFromObject,
53-
R: ToPyResult,
54-
{
55-
fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
56-
let obj = T::try_from_object(vm, obj)?;
57-
(self)(obj).to_pyresult(vm)
58-
}
59-
}
60-
61-
impl<F, S, R> IntoPyGetterFunc<(RefParam<S>, R)> for F
62-
where
63-
F: Fn(&S) -> R + 'static + Send + Sync,
64-
S: PyPayload,
65-
R: ToPyResult,
66-
{
67-
fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
68-
let zelf = PyRef::<S>::try_from_object(vm, obj)?;
69-
(self)(&zelf).to_pyresult(vm)
70-
}
71-
}
72-
73-
pub trait IntoPyNoResult {
74-
fn into_noresult(self) -> PyResult<()>;
75-
}
76-
77-
impl IntoPyNoResult for () {
78-
#[inline]
79-
fn into_noresult(self) -> PyResult<()> {
80-
Ok(())
81-
}
82-
}
83-
84-
impl IntoPyNoResult for PyResult<()> {
85-
#[inline]
86-
fn into_noresult(self) -> PyResult<()> {
87-
self
88-
}
89-
}
90-
91-
pub trait IntoPySetterFunc<T>: PyThreadingConstraint + Sized + 'static {
92-
fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()>;
93-
fn into_setter(self) -> PySetterFunc {
94-
Box::new(move |vm, obj, value| self.set(obj, value, vm))
95-
}
96-
}
97-
98-
impl<F, T, V, R> IntoPySetterFunc<(OwnedParam<T>, V, R, VirtualMachine)> for F
99-
where
100-
F: Fn(T, V, &VirtualMachine) -> R + 'static + Send + Sync,
101-
T: TryFromObject,
102-
V: TryFromObject,
103-
R: IntoPyNoResult,
104-
{
105-
fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
106-
let obj = T::try_from_object(vm, obj)?;
107-
let value = V::try_from_object(vm, value)?;
108-
(self)(obj, value, vm).into_noresult()
109-
}
110-
}
111-
112-
impl<F, S, V, R> IntoPySetterFunc<(RefParam<S>, V, R, VirtualMachine)> for F
113-
where
114-
F: Fn(&S, V, &VirtualMachine) -> R + 'static + Send + Sync,
115-
S: PyPayload,
116-
V: TryFromObject,
117-
R: IntoPyNoResult,
118-
{
119-
fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
120-
let zelf = PyRef::<S>::try_from_object(vm, obj)?;
121-
let value = V::try_from_object(vm, value)?;
122-
(self)(&zelf, value, vm).into_noresult()
123-
}
124-
}
125-
126-
impl<F, T, V, R> IntoPySetterFunc<(OwnedParam<T>, V, R)> for F
127-
where
128-
F: Fn(T, V) -> R + 'static + Send + Sync,
129-
T: TryFromObject,
130-
V: TryFromObject,
131-
R: IntoPyNoResult,
132-
{
133-
fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
134-
let obj = T::try_from_object(vm, obj)?;
135-
let value = V::try_from_object(vm, value)?;
136-
(self)(obj, value).into_noresult()
137-
}
138-
}
139-
140-
impl<F, S, V, R> IntoPySetterFunc<(RefParam<S>, V, R)> for F
141-
where
142-
F: Fn(&S, V) -> R + 'static + Send + Sync,
143-
S: PyPayload,
144-
V: TryFromObject,
145-
R: IntoPyNoResult,
146-
{
147-
fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
148-
let zelf = PyRef::<S>::try_from_object(vm, obj)?;
149-
let value = V::try_from_object(vm, value)?;
150-
(self)(&zelf, value).into_noresult()
151-
}
152-
}
153-
15412
#[pyclass(module = false, name = "getset_descriptor")]
15513
pub struct PyGetSet {
15614
name: String,
@@ -244,12 +102,12 @@ impl PyGetSet {
244102
fn descr_set(
245103
zelf: PyObjectRef,
246104
obj: PyObjectRef,
247-
value: Option<PyObjectRef>,
105+
value: PySetterValue<PyObjectRef>,
248106
vm: &VirtualMachine,
249107
) -> PyResult<()> {
250108
let zelf = PyRef::<Self>::try_from_object(vm, zelf)?;
251109
if let Some(ref f) = zelf.setter {
252-
f(vm, obj, value.unwrap_or_else(|| vm.ctx.none()))
110+
f(vm, obj, value)
253111
} else {
254112
Err(vm.new_attribute_error(format!(
255113
"attribute '{}' of '{}' objects is not writable",
@@ -265,11 +123,11 @@ impl PyGetSet {
265123
value: PyObjectRef,
266124
vm: &VirtualMachine,
267125
) -> PyResult<()> {
268-
Self::descr_set(zelf, obj, Some(value), vm)
126+
Self::descr_set(zelf, obj, PySetterValue::Assign(value), vm)
269127
}
270128
#[pymethod]
271129
fn __delete__(zelf: PyObjectRef, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
272-
Self::descr_set(zelf, obj, None, vm)
130+
Self::descr_set(zelf, obj, PySetterValue::Delete, vm)
273131
}
274132

275133
#[pyproperty(magic)]

vm/src/builtins/object.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ use super::{PyDict, PyDictRef, PyList, PyStr, PyStrRef, PyType, PyTypeRef};
22
use crate::common::hash::PyHash;
33
use crate::{
44
class::PyClassImpl,
5-
function::Either,
6-
function::{FuncArgs, PyArithmeticValue, PyComparisonValue},
5+
function::{Either, FuncArgs, PyArithmeticValue, PyComparisonValue, PySetterValue},
76
types::PyComparisonOp,
87
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine,
98
};
@@ -162,20 +161,20 @@ impl PyBaseObject {
162161
value: PyObjectRef,
163162
vm: &VirtualMachine,
164163
) -> PyResult<()> {
165-
obj.generic_setattr(name, Some(value), vm)
164+
obj.generic_setattr(name, PySetterValue::Assign(value), vm)
166165
}
167166

168167
/// Implement delattr(self, name).
169168
#[pymethod]
170169
fn __delattr__(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
171-
obj.generic_setattr(name, None, vm)
170+
obj.generic_setattr(name, PySetterValue::Delete, vm)
172171
}
173172

174173
#[pyslot]
175174
fn slot_setattro(
176175
obj: &PyObject,
177176
attr_name: PyStrRef,
178-
value: Option<PyObjectRef>,
177+
value: PySetterValue,
179178
vm: &VirtualMachine,
180179
) -> PyResult<()> {
181180
obj.generic_setattr(attr_name, value, vm)

vm/src/builtins/property.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use super::{PyType, PyTypeRef};
55
use crate::common::lock::PyRwLock;
66
use crate::{
77
class::PyClassImpl,
8-
function::FuncArgs,
8+
function::{FuncArgs, PySetterValue},
99
types::{Constructor, GetDescriptor, Initializer},
1010
AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
1111
};
@@ -95,19 +95,19 @@ impl PyProperty {
9595
fn descr_set(
9696
zelf: PyObjectRef,
9797
obj: PyObjectRef,
98-
value: Option<PyObjectRef>,
98+
value: PySetterValue,
9999
vm: &VirtualMachine,
100100
) -> PyResult<()> {
101101
let zelf = PyRef::<Self>::try_from_object(vm, zelf)?;
102102
match value {
103-
Some(value) => {
103+
PySetterValue::Assign(value) => {
104104
if let Some(setter) = zelf.setter.read().as_ref() {
105105
vm.invoke(setter, (obj, value)).map(drop)
106106
} else {
107107
Err(vm.new_attribute_error("can't set attribute".to_owned()))
108108
}
109109
}
110-
None => {
110+
PySetterValue::Delete => {
111111
if let Some(deleter) = zelf.deleter.read().as_ref() {
112112
vm.invoke(deleter, (obj,)).map(drop)
113113
} else {
@@ -123,11 +123,11 @@ impl PyProperty {
123123
value: PyObjectRef,
124124
vm: &VirtualMachine,
125125
) -> PyResult<()> {
126-
Self::descr_set(zelf, obj, Some(value), vm)
126+
Self::descr_set(zelf, obj, PySetterValue::Assign(value), vm)
127127
}
128128
#[pymethod]
129129
fn __delete__(zelf: PyObjectRef, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
130-
Self::descr_set(zelf, obj, None, vm)
130+
Self::descr_set(zelf, obj, PySetterValue::Delete, vm)
131131
}
132132

133133
// Access functions

vm/src/builtins/type.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::common::{
1010
use crate::{
1111
builtins::PyBaseExceptionRef,
1212
class::{PyClassImpl, StaticType},
13-
function::{FuncArgs, KwArgs, OptionalArg},
13+
function::{FuncArgs, KwArgs, OptionalArg, PySetterValue},
1414
identifier,
1515
protocol::PyNumberMethods,
1616
types::{Callable, GetAttr, PyTypeFlags, PyTypeSlots, SetAttr},
@@ -757,7 +757,7 @@ impl SetAttr for PyType {
757757
fn setattro(
758758
zelf: &crate::Py<Self>,
759759
attr_name: PyStrRef,
760-
value: Option<PyObjectRef>,
760+
value: PySetterValue,
761761
vm: &VirtualMachine,
762762
) -> PyResult<()> {
763763
// TODO: pass PyRefExact instead of &str
@@ -768,10 +768,10 @@ impl SetAttr for PyType {
768768
return descriptor(attr, zelf.to_owned().into(), value, vm);
769769
}
770770
}
771-
let assign = value.is_some();
771+
let assign = value.is_assign();
772772

773773
let mut attributes = zelf.attributes.write();
774-
if let Some(value) = value {
774+
if let PySetterValue::Assign(value) = value {
775775
attributes.insert(attr_name, value);
776776
} else {
777777
let prev_value = attributes.remove(attr_name);
@@ -848,7 +848,7 @@ fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -
848848
cls.name()
849849
))
850850
})?;
851-
descr_set(descr, obj, Some(value), vm)
851+
descr_set(descr, obj, PySetterValue::Assign(value), vm)
852852
}
853853
None => {
854854
object::object_set_dict(obj, value.try_into_value(vm)?, vm)?;

0 commit comments

Comments
 (0)