Skip to content

Commit 5122921

Browse files
youknowoneyouknowone
committed
Give builtin_method its own type
Split `builtin_method` out as a subclass of `builtin_function_or_method`. Ordinary descriptor bind and `__new__` wrappers allocate a CFunction; only `build_bound_method` uses the METH_METHOD payload. Keep `types.BuiltinMethodType` as an alias of `BuiltinFunctionType`. Assisted-by: Grok 4.6
1 parent 1e72b22 commit 5122921

6 files changed

Lines changed: 80 additions & 31 deletions

File tree

‎crates/vm/src/builtins/builtin_func.rs‎

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -233,16 +233,20 @@ impl PyNativeFunction {
233233
}
234234
}
235235

236-
// PyCMethodObject in CPython
237-
#[pyclass(name = "builtin_function_or_method", module = false, base = PyNativeFunction, ctx = "builtin_function_or_method_type")]
236+
// Bound METH_METHOD object. The payload starts with PyNativeFunction so it can
237+
// be read as a builtin function. The Python type is `builtin_method`.
238+
#[repr(C)]
239+
#[pyclass(
240+
name = "builtin_method",
241+
module = false,
242+
base = PyNativeFunction,
243+
ctx = "builtin_method_type"
244+
)]
238245
pub struct PyNativeMethod {
239246
pub(crate) func: PyNativeFunction,
240-
pub(crate) class: &'static Py<PyType>, // TODO: the actual life is &'self
247+
pub(crate) class: &'static Py<PyType>,
241248
}
242249

243-
// All Python-visible behavior (getters, slots) is registered by PyNativeFunction::extend_class.
244-
// PyNativeMethod only extends the Rust-side struct with the defining class reference.
245-
// The func field at offset 0 (#[repr(C)]) allows NativeFunctionOrMethod to read it safely.
246250
#[pyclass(flags(HAS_WEAKREF, DISALLOW_INSTANTIATION))]
247251
impl PyNativeMethod {}
248252

@@ -296,6 +300,7 @@ pub(crate) fn init(context: &'static Context) {
296300
.slots
297301
.vectorcall
298302
.store(Some(vectorcall_native_function));
303+
PyNativeMethod::extend_class(context, context.types.builtin_method_type);
299304
}
300305

301306
/// Wrapper that provides access to the common PyNativeFunction data
@@ -306,12 +311,59 @@ impl TryFromObject for NativeFunctionOrMethod {
306311
fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
307312
let class = vm.ctx.types.builtin_function_or_method_type;
308313
if obj.fast_isinstance(class) {
309-
// Both PyNativeFunction and PyNativeMethod share the same type now.
310-
// PyNativeMethod has `func: PyNativeFunction` as its first field,
311-
// so we can safely treat the data pointer as PyNativeFunction for reading.
314+
// `builtin_method` is a subclass; the payload starts with PyNativeFunction.
312315
Ok(Self(unsafe { obj.downcast_unchecked() }))
313316
} else {
314317
Err(vm.new_downcast_type_error(class, &obj))
315318
}
316319
}
317320
}
321+
322+
#[cfg(test)]
323+
mod tests {
324+
use super::*;
325+
use crate::{
326+
Interpreter, PyObjectRef,
327+
function::{PyMethodDef, PyMethodFlags},
328+
};
329+
330+
#[test]
331+
fn builtin_method_is_subclass_of_builtin_function() {
332+
Interpreter::without_stdlib(Default::default()).enter(|vm| {
333+
let function_type = vm.ctx.types.builtin_function_or_method_type;
334+
let method_type = vm.ctx.types.builtin_method_type;
335+
assert!(!function_type.is(method_type));
336+
assert_eq!(&*function_type.name(), "builtin_function_or_method");
337+
assert_eq!(&*method_type.name(), "builtin_method");
338+
assert!(method_type.fast_issubclass(function_type));
339+
});
340+
}
341+
342+
#[test]
343+
fn bound_instance_method_uses_function_type() {
344+
fn identity(value: PyObjectRef) -> PyObjectRef {
345+
value
346+
}
347+
const DEF: PyMethodDef = PyMethodDef::new_const(
348+
"identity",
349+
identity,
350+
PyMethodFlags::METHOD,
351+
crate::function::ItemDoc::NONE,
352+
);
353+
354+
Interpreter::without_stdlib(Default::default()).enter(|vm| {
355+
let bound = DEF.build_bound_function(&vm.ctx, vm.ctx.none());
356+
assert!(
357+
bound
358+
.class()
359+
.is(vm.ctx.types.builtin_function_or_method_type)
360+
);
361+
assert!(!bound.as_object().downcastable::<PyNativeMethod>());
362+
363+
let method = DEF.build_bound_method(&vm.ctx, vm.ctx.none(), vm.ctx.types.object_type);
364+
assert!(method.class().is(vm.ctx.types.builtin_method_type));
365+
assert!(method.as_object().downcastable::<PyNativeFunction>());
366+
assert!(method.as_object().downcastable::<PyNativeMethod>());
367+
});
368+
}
369+
}

‎crates/vm/src/builtins/descriptor.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::{PyStr, PyStrInterned, PyTuple, PyType};
22
use crate::{
33
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
4-
builtins::{PyTypeRef, builtin_func::PyNativeMethod, type_},
4+
builtins::{PyTypeRef, builtin_func::PyNativeFunction, type_},
55
class::PyClassImpl,
66
common::hash::PyHash,
77
convert::{ToPyObject, ToPyResult},
@@ -124,8 +124,8 @@ impl Callable for PyMethodDescriptor {
124124
}
125125

126126
impl PyMethodDescriptor {
127-
pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef<PyNativeMethod> {
128-
self.method.build_bound_method(ctx, obj, self.common.typ)
127+
pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef<PyNativeFunction> {
128+
self.method.build_bound_function(ctx, obj)
129129
}
130130
}
131131

@@ -195,8 +195,8 @@ impl PyClassMethodDescriptor {
195195
}
196196
}
197197

198-
pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef<PyNativeMethod> {
199-
self.method.build_bound_method(ctx, obj, self.common.typ)
198+
pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef<PyNativeFunction> {
199+
self.method.build_bound_function(ctx, obj)
200200
}
201201
}
202202

‎crates/vm/src/class.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,9 +375,9 @@ pub trait PyClassImpl: PyClassDef {
375375
&& object_new.is_some_and(|obj_new| fn_addr(slot_new) == fn_addr(obj_new));
376376

377377
if !is_inherited_from_object {
378-
let bound_new =
379-
ctx.slot_new_wrapper
380-
.build_bound_method(ctx, class.to_owned().into(), class);
378+
let bound_new = ctx
379+
.slot_new_wrapper
380+
.build_bound_function(ctx, class.to_owned().into());
381381
class.set_attr(identifier!(ctx, __new__), bound_new.into());
382382
}
383383
}

‎crates/vm/src/function/method.rs‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ impl PyMethodDef {
249249
) -> PyRef<PyNativeMethod> {
250250
PyRef::new_ref(
251251
self.to_bound_method(obj, class),
252-
ctx.types.builtin_function_or_method_type.to_owned(),
252+
ctx.types.builtin_method_type.to_owned(),
253253
None,
254254
)
255255
}
@@ -267,18 +267,10 @@ impl PyMethodDef {
267267
&'static self,
268268
ctx: &Context,
269269
class: &'static Py<PyType>,
270-
) -> PyRef<PyNativeMethod> {
270+
) -> PyRef<PyNativeFunction> {
271271
debug_assert!(self.flags.contains(PyMethodFlags::STATIC));
272-
// Set zelf to the class (m_self = type for static methods).
273-
// Callable::call skips prepending when STATIC flag is set.
274-
let func = PyNativeFunction {
275-
zelf: Some(class.to_owned().into()),
276-
value: self,
277-
module_object: None,
278-
module: crate::object::PyAtomicRef::new_empty(),
279-
_method_def_owner: None,
280-
};
281-
PyNativeMethod { func, class }.into_ref(ctx)
272+
// m_self is the type; STATIC skips prepending it on call.
273+
self.build_bound_function(ctx, class.to_owned().into())
282274
}
283275

284276
/// Concatenate method groups. A pending body is copied from `docs`, then cleared.

‎crates/vm/src/types/zoo.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ impl TypeZoo {
121121
let weakref_type = weakref::PyWeak::init_manually(hierarchy.weakref_type);
122122
let int_type = int::PyInt::init_builtin_type();
123123

124-
// builtin_function_or_method and builtin_method share the same type (CPython behavior)
124+
// `builtin_method` is a subclass and is initialized once this base exists.
125125
let builtin_function_or_method_type = builtin_func::PyNativeFunction::init_builtin_type();
126126

127127
let types = Self {
@@ -164,7 +164,7 @@ impl TypeZoo {
164164
anext_awaitable: asyncgenerator::PyAnextAwaitable::init_builtin_type(),
165165
bound_method_type: function::PyBoundMethod::init_builtin_type(),
166166
builtin_function_or_method_type,
167-
builtin_method_type: builtin_function_or_method_type,
167+
builtin_method_type: builtin_func::PyNativeMethod::init_builtin_type(),
168168
bytearray_iterator_type: bytearray::PyByteArrayIterator::init_builtin_type(),
169169
bytes_iterator_type: bytes::PyBytesIterator::init_builtin_type(),
170170
callable_iterator: iter::PyCallableIterator::init_builtin_type(),

‎extra_tests/snippets/builtin_type.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,12 @@ def my_repr_func():
654654

655655

656656
# https://github.com/RustPython/RustPython/issues/3100
657+
assert types.BuiltinMethodType is types.BuiltinFunctionType
657658
assert issubclass(types.BuiltinMethodType, types.BuiltinFunctionType)
659+
assert type(len) is types.BuiltinFunctionType
660+
assert type([].append) is types.BuiltinFunctionType
661+
assert type([].append).__name__ == "builtin_function_or_method"
662+
assert type(len).__name__ == "builtin_function_or_method"
658663

659664
assert type.__dict__["__dict__"].__objclass__ is type
660665
assert (

0 commit comments

Comments
 (0)