Remove duplicated richcompare pymethods and add missing rops' wrapper, __getattr__, proper __bool__ impl (#6579)

* slots: remove duplicate pymethods from object

* Add missing __rmul__

* migrate __bool__

* subclass_update, __getattr__
This commit is contained in:
Jeong, YunWon authored and GitHub committed 2025-12-31 20:26:38 +09:00
1 parent d3afd465f3
commit a8cfa36c8a
13 files changed
+203 -153

No files matched your search

-72
View File
@@ -326,66 +326,6 @@ impl PyBaseObject {
Ok(res)
}
/// Return self==value.
#[pymethod]
fn __eq__(
zelf: PyObjectRef,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<PyComparisonValue> {
Self::cmp(&zelf, &value, PyComparisonOp::Eq, vm)
}
/// Return self!=value.
#[pymethod]
fn __ne__(
zelf: PyObjectRef,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<PyComparisonValue> {
Self::cmp(&zelf, &value, PyComparisonOp::Ne, vm)
}
/// Return self<value.
#[pymethod]
fn __lt__(
zelf: PyObjectRef,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<PyComparisonValue> {
Self::cmp(&zelf, &value, PyComparisonOp::Lt, vm)
}
/// Return self<=value.
#[pymethod]
fn __le__(
zelf: PyObjectRef,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<PyComparisonValue> {
Self::cmp(&zelf, &value, PyComparisonOp::Le, vm)
}
/// Return self>=value.
#[pymethod]
fn __ge__(
zelf: PyObjectRef,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<PyComparisonValue> {
Self::cmp(&zelf, &value, PyComparisonOp::Ge, vm)
}
/// Return self>value.
#[pymethod]
fn __gt__(
zelf: PyObjectRef,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<PyComparisonValue> {
Self::cmp(&zelf, &value, PyComparisonOp::Gt, vm)
}
/// Implement setattr(self, name, value).
#[pymethod]
fn __setattr__(
@@ -450,12 +390,6 @@ impl PyBaseObject {
}
}
/// Return repr(self).
#[pymethod]
fn __repr__(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyStrRef> {
Self::slot_repr(&zelf, vm)
}
#[pyclassmethod]
fn __subclasshook__(_args: FuncArgs, vm: &VirtualMachine) -> PyObjectRef {
vm.ctx.not_implemented()
@@ -590,12 +524,6 @@ impl PyBaseObject {
Ok(zelf.get_id() as _)
}
/// Return hash(self).
#[pymethod]
fn __hash__(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyHash> {
Self::slot_hash(&zelf, vm)
}
#[pymethod]
fn __sizeof__(zelf: PyObjectRef) -> usize {
zelf.class().slots.basicsize
+16 -7
View File
@@ -8,9 +8,9 @@ use crate::{
class::PyClassImpl,
common::hash::PyHash,
function::{ArgIndex, FuncArgs, OptionalArg, PyComparisonValue},
protocol::{PyIterReturn, PyMappingMethods, PySequenceMethods},
protocol::{PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods},
types::{
AsMapping, AsSequence, Comparable, Hashable, IterNext, Iterable, PyComparisonOp,
AsMapping, AsNumber, AsSequence, Comparable, Hashable, IterNext, Iterable, PyComparisonOp,
Representable, SelfIter,
},
};
@@ -176,6 +176,7 @@ pub fn init(context: &Context) {
with(
Py,
AsMapping,
AsNumber,
AsSequence,
Hashable,
Comparable,
@@ -269,11 +270,6 @@ impl PyRange {
self.compute_length()
}
#[pymethod]
fn __bool__(&self) -> bool {
!self.is_empty()
}
#[pymethod]
fn __reduce__(&self, vm: &VirtualMachine) -> (PyTypeRef, PyTupleRef) {
let range_parameters: Vec<PyObjectRef> = [&self.start, &self.stop, &self.step]
@@ -426,6 +422,19 @@ impl AsSequence for PyRange {
}
}
impl AsNumber for PyRange {
fn as_number() -> &'static PyNumberMethods {
static AS_NUMBER: PyNumberMethods = PyNumberMethods {
boolean: Some(|number, _vm| {
let zelf = number.obj.downcast_ref::<PyRange>().unwrap();
Ok(!zelf.is_empty())
}),
..PyNumberMethods::NOT_IMPLEMENTED
};
&AS_NUMBER
}
}
impl Hashable for PyRange {
fn hash(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyHash> {
let length = zelf.compute_length();
+15 -15
View File
@@ -50,12 +50,7 @@ impl Constructor for PyNone {
}
#[pyclass(with(Constructor, AsNumber, Representable))]
impl PyNone {
#[pymethod]
const fn __bool__(&self) -> bool {
false
}
}
impl PyNone {}
impl Representable for PyNone {
#[inline]
@@ -103,22 +98,27 @@ impl Constructor for PyNotImplemented {
}
}
#[pyclass(with(Constructor))]
#[pyclass(with(Constructor, AsNumber, Representable))]
impl PyNotImplemented {
// TODO: As per https://bugs.python.org/issue35712, using NotImplemented
// in boolean contexts will need to raise a DeprecationWarning in 3.9
// and, eventually, a TypeError.
#[pymethod]
const fn __bool__(&self) -> bool {
true
}
#[pymethod]
fn __reduce__(&self, vm: &VirtualMachine) -> PyStrRef {
vm.ctx.names.NotImplemented.to_owned()
}
}
impl AsNumber for PyNotImplemented {
fn as_number() -> &'static PyNumberMethods {
// TODO: As per https://bugs.python.org/issue35712, using NotImplemented
// in boolean contexts will need to raise a DeprecationWarning in 3.9
// and, eventually, a TypeError.
static AS_NUMBER: PyNumberMethods = PyNumberMethods {
boolean: Some(|_number, _vm| Ok(true)),
..PyNumberMethods::NOT_IMPLEMENTED
};
&AS_NUMBER
}
}
impl Representable for PyNotImplemented {
#[inline]
fn repr(_zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyStrRef> {
+16 -8
View File
@@ -10,12 +10,12 @@ use crate::{
convert::{ToPyObject, TransmuteFromObject},
function::{ArgSize, FuncArgs, OptionalArg, PyArithmeticValue, PyComparisonValue},
iter::PyExactSizeIterator,
protocol::{PyIterReturn, PyMappingMethods, PySequenceMethods},
protocol::{PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods},
recursion::ReprGuard,
sequence::{OptionalRangeArgs, SequenceExt},
sliceable::{SequenceIndex, SliceableSequenceOp},
types::{
AsMapping, AsSequence, Comparable, Constructor, Hashable, IterNext, Iterable,
AsMapping, AsNumber, AsSequence, Comparable, Constructor, Hashable, IterNext, Iterable,
PyComparisonOp, Representable, SelfIter,
},
utils::collection_repr,
@@ -260,7 +260,7 @@ impl<T> PyTuple<PyRef<T>> {
#[pyclass(
itemsize = core::mem::size_of::<crate::PyObjectRef>(),
flags(BASETYPE, SEQUENCE, _MATCH_SELF),
with(AsMapping, AsSequence, Hashable, Comparable, Iterable, Constructor, Representable)
with(AsMapping, AsNumber, AsSequence, Hashable, Comparable, Iterable, Constructor, Representable)
)]
impl PyTuple {
#[pymethod]
@@ -286,11 +286,6 @@ impl PyTuple {
PyArithmeticValue::from_option(added.ok())
}
#[pymethod]
const fn __bool__(&self) -> bool {
!self.elements.is_empty()
}
#[pymethod]
fn count(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
let mut count: usize = 0;
@@ -423,6 +418,19 @@ impl AsSequence for PyTuple {
}
}
impl AsNumber for PyTuple {
fn as_number() -> &'static PyNumberMethods {
static AS_NUMBER: PyNumberMethods = PyNumberMethods {
boolean: Some(|number, _vm| {
let zelf = number.obj.downcast_ref::<PyTuple>().unwrap();
Ok(!zelf.elements.is_empty())
}),
..PyNumberMethods::NOT_IMPLEMENTED
};
&AS_NUMBER
}
}
impl Hashable for PyTuple {
#[inline]
fn hash(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyHash> {
+17 -8
View File
@@ -4,11 +4,11 @@ use crate::{
class::PyClassImpl,
common::hash::PyHash,
function::{OptionalArg, PyComparisonValue, PySetterValue},
protocol::{PyIter, PyIterReturn, PyMappingMethods, PySequenceMethods},
protocol::{PyIter, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods},
stdlib::builtins::reversed,
types::{
AsMapping, AsSequence, Comparable, Constructor, GetAttr, Hashable, IterNext, Iterable,
PyComparisonOp, Representable, SetAttr,
AsMapping, AsNumber, AsSequence, Comparable, Constructor, GetAttr, Hashable, IterNext,
Iterable, PyComparisonOp, Representable, SetAttr,
},
};
use std::sync::LazyLock;
@@ -68,6 +68,7 @@ crate::common::static_cell! {
SetAttr,
Constructor,
Comparable,
AsNumber,
AsSequence,
AsMapping,
Representable,
@@ -87,11 +88,6 @@ impl PyWeakProxy {
self.try_upgrade(vm)?.length(vm)
}
#[pymethod]
fn __bool__(&self, vm: &VirtualMachine) -> PyResult<bool> {
self.try_upgrade(vm)?.is_true(vm)
}
#[pymethod]
fn __bytes__(&self, vm: &VirtualMachine) -> PyResult {
self.try_upgrade(vm)?.bytes(vm)
@@ -171,6 +167,19 @@ impl SetAttr for PyWeakProxy {
}
}
impl AsNumber for PyWeakProxy {
fn as_number() -> &'static PyNumberMethods {
static AS_NUMBER: LazyLock<PyNumberMethods> = LazyLock::new(|| PyNumberMethods {
boolean: Some(|number, vm| {
let zelf = number.obj.downcast_ref::<PyWeakProxy>().unwrap();
zelf.try_upgrade(vm)?.is_true(vm)
}),
..PyNumberMethods::NOT_IMPLEMENTED
});
&AS_NUMBER
}
}
impl Comparable for PyWeakProxy {
fn cmp(
zelf: &Py<Self>,
+7
View File
@@ -34,6 +34,13 @@ pub fn add_operators(class: &'static Py<PyType>, ctx: &Context) {
continue;
}
// __getattr__ should only have a wrapper if the type explicitly defines it.
// Unlike __getattribute__, __getattr__ is not present on object by default.
// Both map to TpGetattro, but only __getattribute__ gets a wrapper from the slot.
if def.name == "__getattr__" {
continue;
}
// Get the slot function wrapped in SlotFunc
let Some(slot_func) = def.accessor.get_slot_func_with_op(&class.slots, def.op) else {
continue;
+17 -8
View File
@@ -12,13 +12,13 @@ mod _collections {
common::lock::{PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard},
function::{KwArgs, OptionalArg, PyComparisonValue},
iter::PyExactSizeIterator,
protocol::{PyIterReturn, PySequenceMethods},
protocol::{PyIterReturn, PyNumberMethods, PySequenceMethods},
recursion::ReprGuard,
sequence::{MutObjectSequenceOp, OptionalRangeArgs},
sliceable::SequenceIndexOp,
types::{
AsSequence, Comparable, Constructor, DefaultConstructor, Initializer, IterNext,
Iterable, PyComparisonOp, Representable, SelfIter,
AsNumber, AsSequence, Comparable, Constructor, DefaultConstructor, Initializer,
IterNext, Iterable, PyComparisonOp, Representable, SelfIter,
},
utils::collection_repr,
};
@@ -60,6 +60,7 @@ mod _collections {
with(
Constructor,
Initializer,
AsNumber,
AsSequence,
Comparable,
Iterable,
@@ -354,11 +355,6 @@ mod _collections {
self.borrow_deque().len()
}
#[pymethod]
fn __bool__(&self) -> bool {
!self.borrow_deque().is_empty()
}
#[pymethod]
fn __add__(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult<Self> {
self.concat(&other, vm)
@@ -496,6 +492,19 @@ mod _collections {
}
}
impl AsNumber for PyDeque {
fn as_number() -> &'static PyNumberMethods {
static AS_NUMBER: PyNumberMethods = PyNumberMethods {
boolean: Some(|number, _vm| {
let zelf = number.obj.downcast_ref::<PyDeque>().unwrap();
Ok(!zelf.borrow_deque().is_empty())
}),
..PyNumberMethods::NOT_IMPLEMENTED
};
&AS_NUMBER
}
}
impl AsSequence for PyDeque {
fn as_sequence() -> &'static PySequenceMethods {
static AS_SEQUENCE: PySequenceMethods = PySequenceMethods {
+17 -7
View File
@@ -12,8 +12,8 @@ use crate::{
builtins::{PyBytes, PyDict, PyNone, PyStr, PyTuple, PyType, PyTypeRef},
class::StaticType,
function::FuncArgs,
protocol::{BufferDescriptor, PyBuffer},
types::{AsBuffer, Callable, Constructor, Initializer, Representable},
protocol::{BufferDescriptor, PyBuffer, PyNumberMethods},
types::{AsBuffer, AsNumber, Callable, Constructor, Initializer, Representable},
vm::thread::with_current_vm,
};
use alloc::borrow::Cow;
@@ -1772,7 +1772,10 @@ impl AsBuffer for PyCFuncPtr {
}
}
#[pyclass(flags(BASETYPE), with(Callable, Constructor, Representable, AsBuffer))]
#[pyclass(
flags(BASETYPE),
with(Callable, Constructor, AsNumber, Representable, AsBuffer)
)]
impl PyCFuncPtr {
// restype getter/setter
#[pygetset]
@@ -1848,11 +1851,18 @@ impl PyCFuncPtr {
.map(|stg| stg.flags.bits())
.unwrap_or(StgInfoFlags::empty().bits())
}
}
// bool conversion - check if function pointer is set
#[pymethod]
fn __bool__(&self) -> bool {
self.get_func_ptr() != 0
impl AsNumber for PyCFuncPtr {
fn as_number() -> &'static PyNumberMethods {
static AS_NUMBER: PyNumberMethods = PyNumberMethods {
boolean: Some(|number, _vm| {
let zelf = number.obj.downcast_ref::<PyCFuncPtr>().unwrap();
Ok(zelf.get_func_ptr() != 0)
}),
..PyNumberMethods::NOT_IMPLEMENTED
};
&AS_NUMBER
}
}
+14 -7
View File
@@ -261,7 +261,7 @@ impl Initializer for PyCPointer {
#[pyclass(
flags(BASETYPE, IMMUTABLETYPE),
with(Constructor, Initializer, AsBuffer)
with(Constructor, Initializer, AsNumber, AsBuffer)
)]
impl PyCPointer {
/// Get the pointer value stored in buffer as usize
@@ -279,12 +279,6 @@ impl PyCPointer {
}
}
/// Pointer_bool: returns True if pointer is not NULL
#[pymethod]
fn __bool__(&self) -> bool {
self.get_ptr_value() != 0
}
/// contents getter - reads address from b_ptr and creates an instance of the pointed-to type
#[pygetset]
fn contents(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
@@ -779,6 +773,19 @@ impl PyCPointer {
}
}
impl AsNumber for PyCPointer {
fn as_number() -> &'static PyNumberMethods {
static AS_NUMBER: PyNumberMethods = PyNumberMethods {
boolean: Some(|number, _vm| {
let zelf = number.obj.downcast_ref::<PyCPointer>().unwrap();
Ok(zelf.get_ptr_value() != 0)
}),
..PyNumberMethods::NOT_IMPLEMENTED
};
&AS_NUMBER
}
}
impl AsBuffer for PyCPointer {
fn as_buffer(zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<PyBuffer> {
let stg_info = zelf
-8
View File
@@ -1133,14 +1133,6 @@ impl PyCSimple {
self.0.base.read().clone()
}
/// return True if any byte in buffer is non-zero
#[pymethod]
fn __bool__(&self) -> bool {
let buffer = self.0.buffer.read();
// Simple_bool: memcmp(self->b_ptr, zeros, self->b_size)
buffer.iter().any(|&b| b != 0)
}
#[pygetset]
pub fn value(instance: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
let zelf: &Py<Self> = instance
+3 -12
View File
@@ -212,11 +212,6 @@ mod winreg {
Ok(old_hkey as usize)
}
#[pymethod]
fn __bool__(&self) -> bool {
!self.hkey.load().is_null()
}
#[pymethod]
fn __enter__(zelf: PyRef<Self>, _vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
Ok(zelf)
@@ -268,13 +263,9 @@ mod winreg {
negative: Some(|_a, vm| PyHkey::unary_fail(vm)),
positive: Some(|_a, vm| PyHkey::unary_fail(vm)),
absolute: Some(|_a, vm| PyHkey::unary_fail(vm)),
boolean: Some(|a, vm| {
if let Some(a) = a.downcast_ref::<PyHkey>() {
Ok(a.__bool__())
} else {
PyHkey::unary_fail(vm)?;
unreachable!()
}
boolean: Some(|a, _vm| {
let zelf = a.obj.downcast_ref::<PyHkey>().unwrap();
Ok(!zelf.hkey.load().is_null())
}),
invert: Some(|_a, vm| PyHkey::unary_fail(vm)),
lshift: Some(|_a, _b, vm| PyHkey::binary_fail(vm)),
+69 -1
View File
@@ -601,6 +601,7 @@ impl PyType {
/// Update slots based on dunder method changes
///
/// Iterates SLOT_DEFS to find all slots matching the given name and updates them.
/// Also recursively updates subclasses that don't have their own definition.
pub(crate) fn update_slot<const ADD: bool>(&self, name: &'static PyStrInterned, ctx: &Context) {
debug_assert!(name.as_str().starts_with("__"));
debug_assert!(name.as_str().ends_with("__"));
@@ -609,6 +610,36 @@ impl PyType {
for def in find_slot_defs_by_name(name.as_str()) {
self.update_one_slot::<ADD>(&def.accessor, name, ctx);
}
// Recursively update subclasses that don't have their own definition
self.update_subclasses::<ADD>(name, ctx);
}
/// Recursively update subclasses' slots
/// recurse_down_subclasses
fn update_subclasses<const ADD: bool>(&self, name: &'static PyStrInterned, ctx: &Context) {
let subclasses = self.subclasses.read();
for weak_ref in subclasses.iter() {
let Some(subclass) = weak_ref.upgrade() else {
continue;
};
let Some(subclass) = subclass.downcast_ref::<PyType>() else {
continue;
};
// Skip if subclass has its own definition for this attribute
if subclass.attributes.read().contains_key(name) {
continue;
}
// Update subclass's slots
for def in find_slot_defs_by_name(name.as_str()) {
subclass.update_one_slot::<ADD>(&def.accessor, name, ctx);
}
// Recurse into subclass's subclasses
subclass.update_subclasses::<ADD>(name, ctx);
}
}
/// Update a single slot
@@ -706,7 +737,44 @@ impl PyType {
}
}
SlotAccessor::TpDel => update_main_slot!(del, del_wrapper, Del),
SlotAccessor::TpGetattro => update_main_slot!(getattro, getattro_wrapper, GetAttro),
SlotAccessor::TpGetattro => {
// __getattribute__ and __getattr__ both map to TpGetattro.
// If __getattr__ is defined anywhere in MRO, we must use the wrapper
// because the native slot won't call __getattr__.
let __getattr__ = identifier!(ctx, __getattr__);
let has_getattr = {
let attrs = self.attributes.read();
let in_self = attrs.contains_key(__getattr__);
drop(attrs);
// mro[0] is self, so skip it
in_self
|| self
.mro
.read()
.iter()
.skip(1)
.any(|cls| cls.attributes.read().contains_key(__getattr__))
};
if has_getattr {
// Must use wrapper to handle __getattr__
self.slots.getattro.store(Some(getattro_wrapper));
} else if ADD {
if let Some(func) = self.lookup_slot_in_mro(name, ctx, |sf| {
if let SlotFunc::GetAttro(f) = sf {
Some(*f)
} else {
None
}
}) {
self.slots.getattro.store(Some(func));
} else {
self.slots.getattro.store(Some(getattro_wrapper));
}
} else {
accessor.inherit_from_mro(self);
}
}
SlotAccessor::TpSetattro => {
// __setattr__ and __delattr__ share the same slot
if ADD {
+12
View File
@@ -986,6 +986,12 @@ pub static SLOT_DEFS: &[SlotDef] = &[
op: None,
doc: "Return getattr(self, name).",
},
SlotDef {
name: "__getattr__",
accessor: SlotAccessor::TpGetattro,
op: None,
doc: "Implement getattr(self, name).",
},
SlotDef {
name: "__setattr__",
accessor: SlotAccessor::TpSetattro,
@@ -1419,6 +1425,12 @@ pub static SLOT_DEFS: &[SlotDef] = &[
op: None,
doc: "Return self*value.",
},
SlotDef {
name: "__rmul__",
accessor: SlotAccessor::SqRepeat,
op: None,
doc: "Return value*self.",
},
SlotDef {
name: "__iadd__",
accessor: SlotAccessor::SqInplaceConcat,