From ca9dbb40f89044072a051ed3a12cea40745d0ad3 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Thu, 8 Oct 2026 14:01:52 +0900 Subject: [PATCH] Give builtin_method its own type (#9004) 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 --- crates/vm/src/builtins/builtin_func.rs | 70 ++++++++++++++++++++++---- crates/vm/src/builtins/descriptor.rs | 10 ++-- crates/vm/src/class.rs | 6 +-- crates/vm/src/function/method.rs | 16 ++---- crates/vm/src/types/zoo.rs | 4 +- extra_tests/snippets/builtin_type.py | 5 ++ 6 files changed, 80 insertions(+), 31 deletions(-) diff --git a/crates/vm/src/builtins/builtin_func.rs b/crates/vm/src/builtins/builtin_func.rs index 79e2c1ca7..17e3e3424 100644 --- a/crates/vm/src/builtins/builtin_func.rs +++ b/crates/vm/src/builtins/builtin_func.rs @@ -233,16 +233,20 @@ impl PyNativeFunction { } } -// PyCMethodObject in CPython -#[pyclass(name = "builtin_function_or_method", module = false, base = PyNativeFunction, ctx = "builtin_function_or_method_type")] +// Bound METH_METHOD object. The payload starts with PyNativeFunction so it can +// be read as a builtin function. The Python type is `builtin_method`. +#[repr(C)] +#[pyclass( + name = "builtin_method", + module = false, + base = PyNativeFunction, + ctx = "builtin_method_type" +)] pub struct PyNativeMethod { pub(crate) func: PyNativeFunction, - pub(crate) class: &'static Py, // TODO: the actual life is &'self + pub(crate) class: &'static Py, } -// All Python-visible behavior (getters, slots) is registered by PyNativeFunction::extend_class. -// PyNativeMethod only extends the Rust-side struct with the defining class reference. -// The func field at offset 0 (#[repr(C)]) allows NativeFunctionOrMethod to read it safely. #[pyclass(flags(HAS_WEAKREF, DISALLOW_INSTANTIATION))] impl PyNativeMethod {} @@ -296,6 +300,7 @@ pub(crate) fn init(context: &'static Context) { .slots .vectorcall .store(Some(vectorcall_native_function)); + PyNativeMethod::extend_class(context, context.types.builtin_method_type); } /// Wrapper that provides access to the common PyNativeFunction data @@ -306,12 +311,59 @@ impl TryFromObject for NativeFunctionOrMethod { fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { let class = vm.ctx.types.builtin_function_or_method_type; if obj.fast_isinstance(class) { - // Both PyNativeFunction and PyNativeMethod share the same type now. - // PyNativeMethod has `func: PyNativeFunction` as its first field, - // so we can safely treat the data pointer as PyNativeFunction for reading. + // `builtin_method` is a subclass; the payload starts with PyNativeFunction. Ok(Self(unsafe { obj.downcast_unchecked() })) } else { Err(vm.new_downcast_type_error(class, &obj)) } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + Interpreter, PyObjectRef, + function::{PyMethodDef, PyMethodFlags}, + }; + + #[test] + fn builtin_method_is_subclass_of_builtin_function() { + Interpreter::without_stdlib(Default::default()).enter(|vm| { + let function_type = vm.ctx.types.builtin_function_or_method_type; + let method_type = vm.ctx.types.builtin_method_type; + assert!(!function_type.is(method_type)); + assert_eq!(&*function_type.name(), "builtin_function_or_method"); + assert_eq!(&*method_type.name(), "builtin_method"); + assert!(method_type.fast_issubclass(function_type)); + }); + } + + #[test] + fn bound_instance_method_uses_function_type() { + fn identity(value: PyObjectRef) -> PyObjectRef { + value + } + const DEF: PyMethodDef = PyMethodDef::new_const( + "identity", + identity, + PyMethodFlags::METHOD, + crate::function::ItemDoc::NONE, + ); + + Interpreter::without_stdlib(Default::default()).enter(|vm| { + let bound = DEF.build_bound_function(&vm.ctx, vm.ctx.none()); + assert!( + bound + .class() + .is(vm.ctx.types.builtin_function_or_method_type) + ); + assert!(!bound.as_object().downcastable::()); + + let method = DEF.build_bound_method(&vm.ctx, vm.ctx.none(), vm.ctx.types.object_type); + assert!(method.class().is(vm.ctx.types.builtin_method_type)); + assert!(method.as_object().downcastable::()); + assert!(method.as_object().downcastable::()); + }); + } +} diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index b3189e592..301448654 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/crates/vm/src/builtins/descriptor.rs @@ -1,7 +1,7 @@ use super::{PyStr, PyStrInterned, PyTuple, PyType}; use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, - builtins::{PyTypeRef, builtin_func::PyNativeMethod, type_}, + builtins::{PyTypeRef, builtin_func::PyNativeFunction, type_}, class::PyClassImpl, common::hash::PyHash, convert::{ToPyObject, ToPyResult}, @@ -124,8 +124,8 @@ impl Callable for PyMethodDescriptor { } impl PyMethodDescriptor { - pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef { - self.method.build_bound_method(ctx, obj, self.common.typ) + pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef { + self.method.build_bound_function(ctx, obj) } } @@ -195,8 +195,8 @@ impl PyClassMethodDescriptor { } } - pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef { - self.method.build_bound_method(ctx, obj, self.common.typ) + pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef { + self.method.build_bound_function(ctx, obj) } } diff --git a/crates/vm/src/class.rs b/crates/vm/src/class.rs index 8a7a8306d..4afdf6329 100644 --- a/crates/vm/src/class.rs +++ b/crates/vm/src/class.rs @@ -375,9 +375,9 @@ pub trait PyClassImpl: PyClassDef { && object_new.is_some_and(|obj_new| fn_addr(slot_new) == fn_addr(obj_new)); if !is_inherited_from_object { - let bound_new = - ctx.slot_new_wrapper - .build_bound_method(ctx, class.to_owned().into(), class); + let bound_new = ctx + .slot_new_wrapper + .build_bound_function(ctx, class.to_owned().into()); class.set_attr(identifier!(ctx, __new__), bound_new.into()); } } diff --git a/crates/vm/src/function/method.rs b/crates/vm/src/function/method.rs index 0a80efc93..c9953f8a7 100644 --- a/crates/vm/src/function/method.rs +++ b/crates/vm/src/function/method.rs @@ -249,7 +249,7 @@ impl PyMethodDef { ) -> PyRef { PyRef::new_ref( self.to_bound_method(obj, class), - ctx.types.builtin_function_or_method_type.to_owned(), + ctx.types.builtin_method_type.to_owned(), None, ) } @@ -267,18 +267,10 @@ impl PyMethodDef { &'static self, ctx: &Context, class: &'static Py, - ) -> PyRef { + ) -> PyRef { debug_assert!(self.flags.contains(PyMethodFlags::STATIC)); - // Set zelf to the class (m_self = type for static methods). - // Callable::call skips prepending when STATIC flag is set. - let func = PyNativeFunction { - zelf: Some(class.to_owned().into()), - value: self, - module_object: None, - module: crate::object::PyAtomicRef::new_empty(), - _method_def_owner: None, - }; - PyNativeMethod { func, class }.into_ref(ctx) + // m_self is the type; STATIC skips prepending it on call. + self.build_bound_function(ctx, class.to_owned().into()) } /// Concatenate method groups. A pending body is copied from `docs`, then cleared. diff --git a/crates/vm/src/types/zoo.rs b/crates/vm/src/types/zoo.rs index 9672e4cf3..fce1a1e9e 100644 --- a/crates/vm/src/types/zoo.rs +++ b/crates/vm/src/types/zoo.rs @@ -121,7 +121,7 @@ impl TypeZoo { let weakref_type = weakref::PyWeak::init_manually(hierarchy.weakref_type); let int_type = int::PyInt::init_builtin_type(); - // builtin_function_or_method and builtin_method share the same type (CPython behavior) + // `builtin_method` is a subclass and is initialized once this base exists. let builtin_function_or_method_type = builtin_func::PyNativeFunction::init_builtin_type(); let types = Self { @@ -164,7 +164,7 @@ impl TypeZoo { anext_awaitable: asyncgenerator::PyAnextAwaitable::init_builtin_type(), bound_method_type: function::PyBoundMethod::init_builtin_type(), builtin_function_or_method_type, - builtin_method_type: builtin_function_or_method_type, + builtin_method_type: builtin_func::PyNativeMethod::init_builtin_type(), bytearray_iterator_type: bytearray::PyByteArrayIterator::init_builtin_type(), bytes_iterator_type: bytes::PyBytesIterator::init_builtin_type(), callable_iterator: iter::PyCallableIterator::init_builtin_type(), diff --git a/extra_tests/snippets/builtin_type.py b/extra_tests/snippets/builtin_type.py index 06b40fe41..f00baaa57 100644 --- a/extra_tests/snippets/builtin_type.py +++ b/extra_tests/snippets/builtin_type.py @@ -654,7 +654,12 @@ assert repr(my_repr_func).startswith("