Implement Number protocol for PyNone #3880
Conversation
vm/src/builtins/singletons.rs
Outdated
| @@ -52,6 +54,13 @@ impl PyNone { | |||
| } | |||
| } | |||
|
|
|||
| impl AsNumber for PyNone { | |||
| const AS_NUMBER: PyNumberMethods = PyNumberMethods { | |||
| boolean: Some(|number, _vm| Ok(self::number_downcast(number).value.is_zero())) | |||
There was a problem hiding this comment.
is_zero? the value looking swapped
There was a problem hiding this comment.
non_bool(), which is of none_as_number, returns 0 in cpython Objects/object.c
static int
none_bool(PyObject *v)
{
return 0;
}
There was a problem hiding this comment.
return zero means false in this case. As our bool returns false as you know, we just need to return what bool returns exactly.
There was a problem hiding this comment.
oh, sorry. I was confused with bool.
as you said, it returns false(0). But this code is not simply returning false. make this line to do return false same to the code you shared.
There was a problem hiding this comment.
Hi. I fixed my code. Could you please review the new commit?
vm/src/builtins/singletons.rs
Outdated
| @@ -39,7 +41,7 @@ impl Constructor for PyNone { | |||
| } | |||
| } | |||
|
|
|||
| #[pyimpl(with(Constructor))] | |||
| #[pyimpl(flags(BASETYPE), with(Constructor, AsNumber))] | |||
There was a problem hiding this comment.
| #[pyimpl(flags(BASETYPE), with(Constructor, AsNumber))] | |
| #[pyimpl(with(Constructor, AsNumber))] |
It seems PyNone does not have BASETYPE flags in cpython. Is there any reason for adding it?
There was a problem hiding this comment.
RustPython (by current change)
>>>>> Py_TPFLAGS_BASETYPE = 1 << 10
>>>>> type(None).__flags__ & Py_TPFLAGS_BASETYPE
1024
>>>>> CPython 3.10
>>> Py_TPFLAGS_BASETYPE = 1 << 10
>>> type(None).__flags__ & Py_TPFLAGS_BASETYPE
0
vm/src/builtins/singletons.rs
Outdated
| @@ -39,7 +41,7 @@ impl Constructor for PyNone { | |||
| } | |||
| } | |||
|
|
|||
| #[pyimpl(with(Constructor))] | |||
| #[pyimpl(flags(BASETYPE), with(Constructor, AsNumber))] | |||
There was a problem hiding this comment.
Is this flag required for PyNone or AsNumber? I don't think None is a BASETYPE.
|
Please run |
Okay. I'll run it. |
vm/src/builtins/singletons.rs
Outdated
| @@ -52,6 +52,13 @@ impl PyNone { | |||
| } | |||
| } | |||
|
|
|||
| impl AsNumber for PyNone { | |||
| const AS_NUMBER: PyNumberMethods = PyNumberMethods { | |||
| boolean: Some(|number, _vm| Ok(Self::number_downcast(number).bool())), | |||
There was a problem hiding this comment.
| boolean: Some(|number, _vm| Ok(Self::number_downcast(number).bool())), | |
| boolean: Some(|number, _vm| Ok(false)), |
None is singleton. I am pretty sure you can skip actual object value.
There was a problem hiding this comment.
Good idea. BTW, what is the object's functions such as repr() and bool() used be for?
There was a problem hiding this comment.
They are exposed as python method by #[pymethod] macro attached. You can call them below way.
>>>>> None.__bool__()
False
>>>>> None.__repr__()
'None'There was a problem hiding this comment.
And those functions @Snowapril referred are used by facade functions bool(None) and repr(None)
vm/src/builtins/singletons.rs
Outdated
| class::PyClassImpl, convert::ToPyObject, protocol::PyNumberMethods, types::AsNumber, | ||
| types::Constructor, Context, Py, PyObjectRef, PyPayload, PyResult, VirtualMachine, |
There was a problem hiding this comment.
you can also group type traits
types::{AsNumber, Constructor}
Implemented based on #3838