From d8bb7bbd6f4bd581f687e652205870b2309784ab Mon Sep 17 00:00:00 2001 From: Hanjeong Lee Date: Tue, 18 Aug 2026 23:16:45 +0900 Subject: [PATCH] builtins: generate accurate __text_signature__ (#8512) * Drop the $module marker from generated __text_signature__ CPython's C functions receive the module as their first argument, so PyCFunction.__self__ is the module and inspect strips the $module parameter when building a Signature. A #[pyfunction] takes no such argument, PyNativeFunction::zelf is None, and inspect has nothing to strip, so the marker surfaced as a parameter that does not exist: inspect.signature(len) (module, /, obj) # was (obj) # now All 45 builtins shared with CPython carried it. Methods are unaffected; their $self marker comes from func_sig and both branches now produce the same string. Assisted-by: Claude Code:claude-opus-5 * Mark generated __text_signature__ parameters positional-only Arguments bind through `FuncArgs::take_positional`, which pops from the positional list and never consults the keyword map, so a #[pyfunction] argument cannot be passed by name: >>> len(obj=[1, 2]) TypeError The generated signature omitted the `/` marker, so inspect reported those parameters as POSITIONAL_OR_KEYWORD, contradicting the call above. Emit the marker, except for `*args`/`**kwargs`, which cannot be followed by `/`, and for empty parameter lists. 14 of the 45 builtins shared with CPython now report an identical signature, up from 0. Assisted-by: Claude Code:claude-opus-5 * Emit no __text_signature__ when an argument has no name Arguments bound by a destructuring pattern, e.g. fn round(RoundArgs { number, ndigits }: RoundArgs, ..) have no name to report, and func_sig stringified the pattern verbatim: >>> round.__text_signature__ '($module, RoundArgs { number, ndigits })' That is not valid Python, so inspect.signature() raised "builtin has invalid signature". Return None instead, which leaves __text_signature__ unset and makes inspect raise "no signature found", the same as for a CPython builtin that has no signature. Affects round, sum, os.pathconf, binascii.b2a_base64 and binascii.b2a_uu. Their docstrings are unchanged; only the signature prefix is dropped. Assisted-by: Claude Code:claude-opus-5 * Name builtin parameters after CPython These parameters are positional-only, so their names only ever appear in __text_signature__ and cannot be used at a call site. Naming them after CPython makes the generated signatures directly comparable: bin x -> number ord string -> character divmod a, b -> x, y setattr attr -> name delattr attr -> name hasattr attr -> name isinstance typ -> class_or_tuple issubclass subclass,typ -> cls, class_or_tuple aiter iter_target -> async_iterable 23 of the 45 builtins shared with CPython now report an identical signature, up from 0 before this branch. The remainder need FromArgs to report the parameters of its own structs, which is left for a follow-up. Add extra_tests/snippets/builtin_signature.py covering the phantom module parameter, the positional-only marker, the names above, and the signature-less builtins. Assisted-by: Claude Code:claude-opus-5 * Drop expectedFailure from test_module_level_callable_noargs pydoc's summary line for time.time was "time(module)" because the generated signature carried a $module parameter that inspect could not strip. It now reads "time()", as the test expects. Assisted-by: Claude Code:claude-opus-5 * Guard the signature-less assertions to RustPython test_snippets runs every snippet under CPython as well, and CPython does have Argument Clinic signatures for round and sum, so that block only holds for RustPython. Assisted-by: Claude Code:claude-opus-5 * Update crates/derive-impl/src/util.rs * Fix ord's parameter reference after the merge The merge of main took ord's signature from this branch, which renamed the parameter to character, and its body from main, which rewrote ord to accept bytes and bytearray through a parameter named c. The body then referenced a name that no longer existed and the build failed. Assisted-by: Claude Code:claude-opus-5 --------- Co-authored-by: Jeong, YunWon <69878+youknowone@users.noreply.github.com> --- Lib/test/test_pydoc/test_pydoc.py | 1 - crates/derive-impl/src/pyclass.rs | 5 +- crates/derive-impl/src/pymodule.rs | 16 +++-- crates/derive-impl/src/util.rs | 85 +++++++++++++---------- crates/vm/src/stdlib/builtins.rs | 56 ++++++++------- extra_tests/snippets/builtin_signature.py | 66 ++++++++++++++++++ 6 files changed, 161 insertions(+), 68 deletions(-) create mode 100644 extra_tests/snippets/builtin_signature.py diff --git a/Lib/test/test_pydoc/test_pydoc.py b/Lib/test/test_pydoc/test_pydoc.py index d206e5a91..f44c56da6 100644 --- a/Lib/test/test_pydoc/test_pydoc.py +++ b/Lib/test/test_pydoc/test_pydoc.py @@ -1582,7 +1582,6 @@ class TestDescriptions(unittest.TestCase): self.assertEqual(self._get_summary_line(os.stat), "stat(path, *, dir_fd=None, follow_symlinks=True)") - @unittest.expectedFailure # TODO: RUSTPYTHON def test_module_level_callable_noargs(self): self.assertEqual(self._get_summary_line(time.time), "time()") diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index 94bb445fe..dd7973d35 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/crates/derive-impl/src/pyclass.rs @@ -1096,7 +1096,10 @@ where args.attrs.push(allow_attr); } - let doc = args.attrs.doc().map(|doc| format_doc(&sig_doc, &doc)); + let doc = args.attrs.doc().map(|doc| match &sig_doc { + Some(sig_doc) => format_doc(sig_doc, &doc), + None => doc, + }); args.context.method_items.add_item(MethodNurseryItem { py_name, cfgs: args.cfgs.to_vec(), diff --git a/crates/derive-impl/src/pymodule.rs b/crates/derive-impl/src/pymodule.rs index 32d7a0fa6..20c94abe9 100644 --- a/crates/derive-impl/src/pymodule.rs +++ b/crates/derive-impl/src/pymodule.rs @@ -524,7 +524,7 @@ struct FunctionNurseryItem { py_names: Vec, cfgs: Vec, ident: Ident, - doc: String, + doc: Option, call_flags: TokenStream, } @@ -556,8 +556,10 @@ impl ToTokens for ValidatedFunctionNursery { let cfgs = &item.cfgs; let cfgs = quote!(#(#cfgs)*); let py_names = &item.py_names; - let doc = &item.doc; - let doc = quote!(Some(#doc)); + let doc = match &item.doc { + Some(doc) => quote!(Some(#doc)), + None => quote!(None), + }; let flags = &item.call_flags; inner_tokens.extend(quote![ @@ -671,10 +673,10 @@ impl ModuleItem for FunctionItem { .copied() .map(str::to_owned) }); - let doc = if let Some(doc) = doc { - format_doc(&sig_doc, &doc) - } else { - sig_doc + let doc = match (sig_doc, doc) { + (Some(sig_doc), Some(doc)) => Some(format_doc(&sig_doc, &doc)), + (Some(sig_doc), None) => Some(sig_doc), + (None, doc) => doc, }; let py_names = { diff --git a/crates/derive-impl/src/util.rs b/crates/derive-impl/src/util.rs index 1ee878c13..a8b4b6ff4 100644 --- a/crates/derive-impl/src/util.rs +++ b/crates/derive-impl/src/util.rs @@ -732,13 +732,20 @@ where // Best effort attempt to generate a template from which a // __text_signature__ can be created. -pub(crate) fn text_signature(sig: &Signature, name: &str) -> String { - let signature = func_sig(sig); - if signature.starts_with("$self") { +// +// Unlike CPython, a `#[pyfunction]` doesn't take the module as an argument yet, +// so there's no module to mark with `$module`. +pub(crate) fn text_signature(sig: &Signature, name: &str) -> Option { + let signature = func_sig(sig)?; + // Arguments bind through `FuncArgs::take_positional`, which never consults + // the keyword map, so they are positional-only. `*args`/`**kwargs` cannot be + // followed by `/`, and an empty parameter list has nothing to mark. + let signature = if signature.is_empty() || signature.contains('*') { format!("{name}({signature})") } else { - format!("{}({}, {})", name, "$module", signature) - } + format!("{name}({signature}, /)") + }; + Some(signature) } pub(crate) fn infer_native_call_flags(sig: &Signature, drop_first_typed: usize) -> TokenStream { @@ -812,37 +819,45 @@ pub(crate) fn infer_native_call_flags(sig: &Signature, drop_first_typed: usize) } } -fn func_sig(sig: &Signature) -> String { - sig.inputs - .iter() - .filter_map(|arg| { - let arg = match arg { - FnArg::Typed(typed) => typed, - FnArg::Receiver(_) => return Some("$self".to_owned()), - }; - let ty = arg.ty.as_ref(); - let ty = quote!(#ty).to_string(); - if ty == "FuncArgs" { - return Some("*args, **kwargs".to_owned()); +/// Returns None when an argument has no name to report, in which case no +/// signature can be generated for the function. +fn func_sig(sig: &Signature) -> Option { + let mut params = Vec::new(); + for arg in &sig.inputs { + let arg = match arg { + FnArg::Typed(typed) => typed, + FnArg::Receiver(_) => { + params.push("$self".to_owned()); + continue; } - if ty.starts_with('&') && ty.ends_with("VirtualMachine") { - return None; - } - let ident = match arg.pat.as_ref() { - syn::Pat::Ident(p) => p.ident.to_string(), - // FIXME: other => unreachable!("function arg pattern must be ident but found `{}`", quote!(fn #ident(.. #other ..))), - other => quote!(#other).to_string(), - }; - if ident == "zelf" { - return Some("$self".to_owned()); - } - if ident == "vm" { - unreachable!("type &VirtualMachine(`{ty}`) must be filtered already"); - } - Some(ident) - }) - .collect::>() - .join(", ") + }; + let ty = arg.ty.as_ref(); + let ty = quote!(#ty).to_string(); + if ty == "FuncArgs" { + params.push("*args, **kwargs".to_owned()); + continue; + } + if ty.starts_with('&') && ty.ends_with("VirtualMachine") { + continue; + } + // An argument bound by a destructuring pattern, e.g. + // `fn round(RoundArgs { number, ndigits }: RoundArgs, ..)`, has no name + // to report. Stringifying the pattern would emit Rust syntax, which + // makes inspect.signature() raise "invalid signature". + let syn::Pat::Ident(pat) = arg.pat.as_ref() else { + return None; + }; + let ident = pat.ident.to_string(); + if ident == "zelf" { + params.push("$self".to_owned()); + continue; + } + if ident == "vm" { + unreachable!("type &VirtualMachine(`{ty}`) must be filtered already"); + } + params.push(ident); + } + Some(params.join(", ")) } pub(crate) fn format_doc(sig: &str, doc: &str) -> String { diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs index 19e33110a..34be8c617 100644 --- a/crates/vm/src/stdlib/builtins.rs +++ b/crates/vm/src/stdlib/builtins.rs @@ -74,8 +74,8 @@ mod builtins { } #[pyfunction] - fn bin(x: PyIntRef) -> String { - let x = x.as_bigint(); + fn bin(number: PyIntRef) -> String { + let x = number.as_bigint(); if x.is_negative() { format!("-0b{:b}", x.abs()) } else { @@ -392,11 +392,11 @@ mod builtins { } #[pyfunction] - fn delattr(obj: PyObjectRef, attr: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let attr = attr.try_to_ref::(vm).map_err(|_e| { + fn delattr(obj: PyObjectRef, name: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let attr = name.try_to_ref::(vm).map_err(|_e| { vm.new_type_error(format!( "attribute name must be string, not '{}'", - attr.class().name() + name.class().name() )) })?; obj.del_attr(attr, vm) @@ -408,8 +408,8 @@ mod builtins { } #[pyfunction] - fn divmod(a: PyObjectRef, b: PyObjectRef, vm: &VirtualMachine) -> PyResult { - vm._divmod(&a, &b) + fn divmod(x: PyObjectRef, y: PyObjectRef, vm: &VirtualMachine) -> PyResult { + vm._divmod(&x, &y) } #[derive(FromArgs)] @@ -715,11 +715,11 @@ mod builtins { } #[pyfunction] - fn hasattr(obj: PyObjectRef, attr: PyObjectRef, vm: &VirtualMachine) -> PyResult { - let attr = attr.try_to_ref::(vm).map_err(|_e| { + fn hasattr(obj: PyObjectRef, name: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let attr = name.try_to_ref::(vm).map_err(|_e| { vm.new_type_error(format!( "attribute name must be string, not '{}'", - attr.class().name() + name.class().name() )) })?; Ok(vm.get_attribute_opt(obj, attr)?.is_some()) @@ -821,13 +821,21 @@ mod builtins { } #[pyfunction] - fn isinstance(obj: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult { - obj.is_instance(&typ, vm) + fn isinstance( + obj: PyObjectRef, + class_or_tuple: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + obj.is_instance(&class_or_tuple, vm) } #[pyfunction] - fn issubclass(subclass: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult { - subclass.is_subclass(&typ, vm) + fn issubclass( + cls: PyObjectRef, + class_or_tuple: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + cls.is_subclass(&class_or_tuple, vm) } #[pyfunction] @@ -848,8 +856,8 @@ mod builtins { } #[pyfunction] - fn aiter(iter_target: PyObjectRef, vm: &VirtualMachine) -> PyResult { - iter_target.get_aiter(vm) + fn aiter(async_iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult { + async_iterable.get_aiter(vm) } #[pyfunction] @@ -998,8 +1006,8 @@ mod builtins { #[pyfunction] // builtin_ord - fn ord(c: PyObjectRef, vm: &VirtualMachine) -> PyResult { - let bytes = if let Some(string) = c.downcast_ref::() { + fn ord(character: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let bytes = if let Some(string) = character.downcast_ref::() { return match string.as_wtf8().code_points().exactly_one() { Ok(character) => Ok(character.to_u32()), Err(_) => { @@ -1009,14 +1017,14 @@ mod builtins { ))) } }; - } else if let Some(bytes) = c.downcast_ref::() { + } else if let Some(bytes) = character.downcast_ref::() { bytes.as_bytes().to_vec() - } else if let Some(bytearray) = c.downcast_ref::() { + } else if let Some(bytearray) = character.downcast_ref::() { bytearray.borrow_buf().to_vec() } else { return Err(vm.new_type_error(format!( "ord() expected string of length 1, but {} found", - c.class().name() + character.class().name() ))); }; let bytes_len = bytes.len(); @@ -1149,14 +1157,14 @@ mod builtins { #[pyfunction] fn setattr( obj: PyObjectRef, - attr: PyObjectRef, + name: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - let attr = attr.try_to_ref::(vm).map_err(|_e| { + let attr = name.try_to_ref::(vm).map_err(|_e| { vm.new_type_error(format!( "attribute name must be string, not '{}'", - attr.class().name() + name.class().name() )) })?; obj.set_attr(attr, value, vm)?; diff --git a/extra_tests/snippets/builtin_signature.py b/extra_tests/snippets/builtin_signature.py new file mode 100644 index 000000000..320a395d8 --- /dev/null +++ b/extra_tests/snippets/builtin_signature.py @@ -0,0 +1,66 @@ +import inspect +import sys + +# __text_signature__ is generated from the Rust parameter list, so it must not +# describe parameters the function does not actually take, and must mark the +# ones it does take as positional-only. + +# No phantom `module` parameter. RustPython's #[pyfunction]s take no module +# argument, so `__self__` is None and inspect has nothing to strip. +for f in (len, abs, hash, id, repr, bin, ord, divmod, hex, oct, chr, callable): + assert "module" not in inspect.signature(f).parameters, f.__name__ + +# Plain arguments bind through take_positional(), so they are positional-only. +try: + len(obj=[1, 2]) +except TypeError: + pass +else: + raise AssertionError("len() should not accept keyword arguments") + +assert str(inspect.signature(len)) == "(obj, /)" +assert str(inspect.signature(abs)) == "(x, /)" +assert str(inspect.signature(hash)) == "(obj, /)" +assert str(inspect.signature(chr)) == "(i, /)" +assert str(inspect.signature(callable)) == "(obj, /)" + +assert ( + inspect.signature(len).parameters["obj"].kind == inspect.Parameter.POSITIONAL_ONLY +) + +# *args/**kwargs cannot be followed by `/`. The parameter names themselves still +# differ from CPython here, which is out of scope. +breakpoint_kinds = [p.kind for p in inspect.signature(breakpoint).parameters.values()] +assert breakpoint_kinds == [ + inspect.Parameter.VAR_POSITIONAL, + inspect.Parameter.VAR_KEYWORD, +], breakpoint_kinds + +# Parameter names follow CPython, so signatures are directly comparable. +assert str(inspect.signature(bin)) == "(number, /)" +assert str(inspect.signature(ord)) == "(character, /)" +assert str(inspect.signature(divmod)) == "(x, y, /)" +assert str(inspect.signature(hasattr)) == "(obj, name, /)" +assert str(inspect.signature(setattr)) == "(obj, name, value, /)" +assert str(inspect.signature(delattr)) == "(obj, name, /)" +assert str(inspect.signature(isinstance)) == "(obj, class_or_tuple, /)" +assert str(inspect.signature(issubclass)) == "(cls, class_or_tuple, /)" +assert str(inspect.signature(aiter)) == "(async_iterable, /)" + +if sys.implementation.name == "rustpython": + # Functions whose Rust arguments are destructuring patterns rather than + # plain names get no signature at all, instead of emitting text that is not + # valid Python and makes inspect.signature() raise "invalid signature". + # + # CPython does have signatures for these, hand-written via Argument Clinic. + # We cannot derive them until FromArgs reports the parameters of its own + # structs, so until then we report no signature, which is at least how + # CPython behaves for the builtins it has no signature for. + for f in (round, sum): + assert f.__text_signature__ is None, f.__name__ + try: + inspect.signature(f) + except ValueError as e: + assert "no signature found" in str(e), str(e) + else: + raise AssertionError(f"{f.__name__} should have no signature")