From 8ea9a4465772b544b4671dfd2e9d674fdd14e678 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 24 Sep 2026 17:54:50 +0900 Subject: [PATCH 1/3] Compose native text signatures at compile time from FromArgs FromArgs gains a PARAMS const describing its Python parameters. The derive builds the whole internal doc, `name(sig)\n--\n\ndoc`, in a const context from those params, and `__text_signature__` / `__doc__` parse it from the doc. The macro-time string path and the runtime sig_parts composition are removed. Signatures follow clinic conventions: `$module` for module functions, Rust literal defaults rendered as Python reprs, other defaults as ``, and a new `#[pyarg(py_default = "...")]` for the few fields whose Rust default differs from the Python one. The `#[pyarg]` keys are documented on the FromArgs derive. Module functions keep their module object and return it from `__self__`. PyNativeFunction now traverses its references. os.stat, os.mkdir, os.listdir, os.open, dict.fromkeys, round, sorted, int.to_bytes and bytes.hex take arguments matching their signatures. Assisted-by: Grok:grok-4.7 Assisted-by: Claude:claude-opus-5-5 --- .cspell.dict/rust-more.txt | 1 + Lib/test/test_inspect/test_inspect.py | 1 - Lib/test/test_pydoc/test_pydoc.py | 1 - crates/derive-impl/Cargo.toml | 2 +- crates/derive-impl/src/from_args.rs | 140 +++++++++-- crates/derive-impl/src/pyclass.rs | 32 +-- crates/derive-impl/src/pymodule.rs | 18 +- crates/derive-impl/src/util.rs | 224 +++++++++++------ crates/derive/src/lib.rs | 39 +++ crates/vm/src/builtins/builtin_func.rs | 25 +- crates/vm/src/builtins/descriptor.rs | 16 +- crates/vm/src/builtins/dict.rs | 18 +- crates/vm/src/builtins/int.rs | 9 +- crates/vm/src/builtins/list.rs | 2 +- crates/vm/src/builtins/module.rs | 1 + crates/vm/src/bytes_inner.rs | 15 +- crates/vm/src/function/argument.rs | 22 ++ crates/vm/src/function/method.rs | 4 + crates/vm/src/function/mod.rs | 4 + crates/vm/src/function/signature.rs | 322 +++++++++++++++++++++++++ crates/vm/src/stdlib/builtins.rs | 19 +- crates/vm/src/stdlib/os.rs | 79 ++++-- 22 files changed, 811 insertions(+), 183 deletions(-) create mode 100644 crates/vm/src/function/signature.rs diff --git a/.cspell.dict/rust-more.txt b/.cspell.dict/rust-more.txt index b53639c3b41..c8614f12e65 100644 --- a/.cspell.dict/rust-more.txt +++ b/.cspell.dict/rust-more.txt @@ -60,6 +60,7 @@ powf powi prepended punct +qself replacen retag rmatch diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 8d9d3537e3e..8cf43a48066 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -1363,7 +1363,6 @@ def test(): pass spec = inspect.getfullargspec(test) self.assertEqual(test.__annotations__, spec.annotations) - @unittest.expectedFailure # TODO: RUSTPYTHON; os.stat signature is generated as (path, dir_fd, follow_symlinks, /) @unittest.skipIf(MISSING_C_DOCSTRINGS, "Signature information for builtins requires docstrings") def test_getfullargspec_builtin_methods(self): diff --git a/Lib/test/test_pydoc/test_pydoc.py b/Lib/test/test_pydoc/test_pydoc.py index eeea1010144..66e3fc59e7e 100644 --- a/Lib/test/test_pydoc/test_pydoc.py +++ b/Lib/test/test_pydoc/test_pydoc.py @@ -1573,7 +1573,6 @@ def test_bound_builtin_method(self): "dump(obj, /) method of _pickle.Pickler instance") # this should *never* include self! - @unittest.expectedFailure # TODO: RUSTPYTHON; os.stat signature is generated as (path, dir_fd, follow_symlinks, /) @requires_docstrings def test_module_level_callable(self): self.assertEqual(self._get_summary_line(os.stat), diff --git a/crates/derive-impl/Cargo.toml b/crates/derive-impl/Cargo.toml index 7197c51ecfa..1c2b5704785 100644 --- a/crates/derive-impl/Cargo.toml +++ b/crates/derive-impl/Cargo.toml @@ -14,7 +14,7 @@ rustpython-compiler-core = { workspace = true } rustpython-doc = { workspace = true } itertools = { workspace = true } -syn = { workspace = true, features = ["full", "extra-traits"] } +syn = { workspace = true, features = ["full", "extra-traits", "visit", "visit-mut"] } proc-macro2 = { workspace = true } quote = { workspace = true } diff --git a/crates/derive-impl/src/from_args.rs b/crates/derive-impl/src/from_args.rs index 535c5c4e35d..a9780f9e155 100644 --- a/crates/derive-impl/src/from_args.rs +++ b/crates/derive-impl/src/from_args.rs @@ -2,7 +2,7 @@ use proc_macro2::TokenStream; use quote::{ToTokens, quote}; use syn::ext::IdentExt; use syn::meta::ParseNestedMeta; -use syn::{Attribute, Data, DeriveInput, Expr, Field, Ident, Result, Token, parse_quote}; +use syn::{Attribute, Data, DeriveInput, Expr, Field, Ident, Lit, Result, Token, parse_quote}; /// The kind of the python parameter, this corresponds to the value of Parameter.kind /// (https://docs.python.org/3/library/inspect.html#inspect.Parameter.kind) @@ -37,6 +37,7 @@ struct ArgAttribute { name: Option, kind: ParameterKind, default: Option, + py_default: Option, error_msg: Option, } @@ -64,6 +65,7 @@ impl ArgAttribute { name: None, kind, default: None, + py_default: None, error_msg: None, }); return Ok(()); @@ -91,6 +93,13 @@ impl ArgAttribute { if self.default.is_none() { self.default = Some(None); } + // Text-only signature default. See the `FromArgs` derive docs. + } else if meta.path.is_ident("py_default") { + if self.py_default.is_some() { + return Err(meta.error("py_default already set")); + } + let val = meta.value()?.parse::()?; + self.py_default = Some(val.value()); } else if meta.path.is_ident("name") { if self.name.is_some() { return Err(meta.error("already have a name")); @@ -212,6 +221,96 @@ fn generate_field((i, field): (usize, &Field)) -> Result { Ok(file_output) } +fn is_phantom(field: &Field) -> bool { + field + .ident + .as_ref() + .is_some_and(|ident| ident.unraw().to_string().starts_with("_phantom")) +} + +fn python_str_repr(s: &str) -> String { + let mut out = String::from("'"); + for c in s.chars() { + match c { + '\\' => out.push_str("\\\\"), + '\'' => out.push_str("\\'"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + _ => out.push(c), + } + } + out.push('\''); + out +} + +fn python_default_repr(default: Option<&DefaultValue>, py_default: Option<&str>) -> Option { + if let Some(py_default) = py_default { + return Some(py_default.to_owned()); + } + match default { + None => None, + Some(None) => Some("".to_owned()), + Some(Some(expr)) => match expr { + Expr::Lit(syn::ExprLit { lit, .. }) => match lit { + Lit::Bool(b) => Some(if b.value { + "True".to_owned() + } else { + "False".to_owned() + }), + Lit::Int(i) => Some(i.base10_digits().to_owned()), + Lit::Float(f) => Some(f.base10_digits().to_owned()), + Lit::Str(s) => Some(python_str_repr(&s.value())), + _ => Some("".to_owned()), + }, + Expr::Path(path) if path.qself.is_none() && path.path.is_ident("None") => { + Some("None".to_owned()) + } + _ => Some("".to_owned()), + }, + } +} + +fn param_token(field: &Field, attr: &ArgAttribute) -> Result { + if let ParameterKind::Flatten = attr.kind { + let ty = &field.ty; + return Ok(quote! { + ::rustpython_vm::function::Param::flatten( + <#ty as ::rustpython_vm::function::FromArgs>::PARAMS + ) + }); + } + + let name = field.ident.as_ref().map(|ident| ident.unraw().to_string()); + let pyname = attr + .name + .clone() + .or(name) + .ok_or_else(|| err_span!(field, "field in tuple struct must have name attribute"))?; + let default = python_default_repr(attr.default.as_ref(), attr.py_default.as_deref()); + let default_tok = match &default { + Some(default) => quote!(Some(#default)), + None => quote!(None), + }; + let kind = match attr.kind { + ParameterKind::PositionalOnly => { + quote!(::rustpython_vm::function::ParamKind::PositionalOnly) + } + ParameterKind::PositionalOrKeyword => { + quote!(::rustpython_vm::function::ParamKind::PositionalOrKeyword) + } + ParameterKind::KeywordOnly => quote!(::rustpython_vm::function::ParamKind::KeywordOnly), + ParameterKind::Flatten => unreachable!(), + }; + Ok(quote! { + ::rustpython_vm::function::Param { + name: #pyname, + kind: #kind, + default: #default_tok, + } + }) +} + fn compute_arity_bounds(field_attrs: &[ArgAttribute]) -> (usize, usize) { let positional_fields = field_attrs.iter().filter(|attr| { matches!( @@ -230,27 +329,40 @@ fn compute_arity_bounds(field_attrs: &[ArgAttribute]) -> (usize, usize) { } pub(crate) fn impl_from_args(input: DeriveInput) -> Result { - let (fields, field_attrs) = match input.data { - Data::Struct(syn::DataStruct { fields, .. }) => ( - fields - .iter() - .enumerate() - .map(generate_field) - .collect::>()?, - fields - .iter() - .filter_map(|field| field.try_into().ok()) - .collect::>(), - ), - _ => bail_span!(input, "FromArgs input must be a struct"), + let Data::Struct(syn::DataStruct { + fields: struct_fields, + .. + }) = input.data + else { + bail_span!(input, "FromArgs input must be a struct") }; + let fields = struct_fields + .iter() + .enumerate() + .map(generate_field) + .collect::>()?; + let field_attrs = struct_fields + .iter() + .map(ArgAttribute::try_from) + .collect::>>()?; let (min_arity, max_arity) = compute_arity_bounds(&field_attrs); + let mut params = Vec::new(); + for field in &struct_fields { + if is_phantom(field) { + continue; + } + params.push(param_token(field, &ArgAttribute::try_from(field)?)?); + } + let name = input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let output = quote! { impl #impl_generics ::rustpython_vm::function::FromArgs for #name #ty_generics #where_clause { + const PARAMS: Option<&'static [::rustpython_vm::function::Param]> = + Some(&[#(#params),*]); + fn arity() -> ::std::ops::RangeInclusive { #min_arity..=#max_arity } diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index 320a39feb2a..43e25f2bf19 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/crates/derive-impl/src/pyclass.rs @@ -1,8 +1,8 @@ use super::Diagnostic; use crate::util::{ ALL_ALLOWED_NAMES, ClassItemMeta, ContentItem, ContentItemInner, ErrorVec, ExceptionItemMeta, - ItemMeta, ItemMetaInner, ItemNursery, SimpleItemMeta, format_doc, infer_native_call_flags, - pyclass_ident_and_attrs, pyexception_ident_and_attrs, text_signature, + ItemMeta, ItemMetaInner, ItemNursery, SimpleItemMeta, infer_native_call_flags, + internal_doc_tokens, pyclass_ident_and_attrs, pyexception_ident_and_attrs, }; use core::str::FromStr; use proc_macro2::{Delimiter, Group, Span, TokenStream, TokenTree}; @@ -71,6 +71,9 @@ struct ImplContext { extend_slots_items: ItemNursery, class_extensions: Vec, errors: Vec, + /// Set when the impl has no generic parameters, so `Self` in argument + /// types can be replaced before it appears in a nested const. + self_ty_subst: Option, } fn extract_items_into_context<'a, Item>( @@ -104,6 +107,9 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul let mut context = ImplContext::default(); let mut tokens = match item { Item::Impl(mut imp) => { + if imp.generics.params.is_empty() { + context.self_ty_subst = Some((*imp.self_ty).clone()); + } extract_items_into_context(&mut context, imp.items.iter_mut()); let (impl_ty, payload_guess) = match imp.self_ty.as_ref() { @@ -1123,7 +1129,6 @@ where } }; let drop_first_typed = usize::from(implicit_self.is_some()); - let sig_doc = text_signature(func.sig(), &py_name, implicit_self); let call_flags = infer_native_call_flags(func.sig(), drop_first_typed); // Add #[allow(non_snake_case)] for setter methods like set___name__ @@ -1133,11 +1138,14 @@ where args.attrs.push(allow_attr); } - let doc = match (sig_doc, args.attrs.doc()) { - (Some(sig_doc), Some(doc)) => Some(format_doc(&sig_doc, &doc)), - (Some(sig_doc), None) => Some(format_doc(&sig_doc, "")), - (None, doc) => doc, - }; + let doc = internal_doc_tokens( + func.sig(), + &py_name, + implicit_self, + args.attrs.doc(), + args.context.self_ty_subst.as_ref(), + None, + ); args.context.method_items.add_item(MethodNurseryItem { py_name, cfgs: args.cfgs.to_vec(), @@ -1373,7 +1381,7 @@ struct MethodNurseryItem { ident: Ident, raw: bool, coexist: bool, - doc: Option, + doc: TokenStream, attr_name: AttrName, call_flags: TokenStream, } @@ -1401,11 +1409,7 @@ impl ToTokens for MethodNursery { let py_name = &item.py_name; let ident = &item.ident; let cfgs = &item.cfgs; - let doc = if let Some(doc) = item.doc.as_ref() { - quote! { Some(#doc) } - } else { - quote! { None } - }; + let doc = &item.doc; let binding_flags = match &item.attr_name { AttrName::Method => { quote! { rustpython_vm::function::PyMethodFlags::METHOD } diff --git a/crates/derive-impl/src/pymodule.rs b/crates/derive-impl/src/pymodule.rs index 5d3c7c3fa7b..d0cf5657e33 100644 --- a/crates/derive-impl/src/pymodule.rs +++ b/crates/derive-impl/src/pymodule.rs @@ -2,8 +2,8 @@ use crate::error::Diagnostic; use crate::pystructseq::PyStructSequenceMeta; use crate::util::{ ALL_ALLOWED_NAMES, AttrItemMeta, AttributeExt, ClassItemMeta, ContentItem, ContentItemInner, - ErrorVec, ItemMeta, ItemNursery, ModuleItemMeta, SimpleItemMeta, format_doc, - infer_native_call_flags, iter_use_idents, pyclass_ident_and_attrs, text_signature, + ErrorVec, ItemMeta, ItemNursery, ModuleItemMeta, SimpleItemMeta, infer_native_call_flags, + internal_doc_tokens, iter_use_idents, pyclass_ident_and_attrs, }; use core::str::FromStr; use proc_macro2::{Delimiter, Group, TokenStream, TokenTree}; @@ -524,7 +524,7 @@ struct FunctionNurseryItem { py_names: Vec, cfgs: Vec, ident: Ident, - doc: Option, + doc: TokenStream, call_flags: TokenStream, } @@ -556,10 +556,7 @@ impl ToTokens for ValidatedFunctionNursery { let cfgs = &item.cfgs; let cfgs = quote!(#(#cfgs)*); let py_names = &item.py_names; - let doc = match &item.doc { - Some(doc) => quote!(Some(#doc)), - None => quote!(None), - }; + let doc = &item.doc; let flags = &item.call_flags; inner_tokens.extend(quote![ @@ -664,7 +661,6 @@ impl ModuleItem for FunctionItem { let item_meta = SimpleItemMeta::from_attr(ident.clone(), &item_attr)?; let py_name = item_meta.simple_name()?; - let sig_doc = text_signature(func.sig(), &py_name, None); let module = args.module_name(); // TODO: doc must exist at least one of code or CPython @@ -673,11 +669,7 @@ impl ModuleItem for FunctionItem { .copied() .map(str::to_owned) }); - let doc = match (sig_doc, doc) { - (Some(sig_doc), Some(doc)) => Some(format_doc(&sig_doc, &doc)), - (Some(sig_doc), None) => Some(format_doc(&sig_doc, "")), - (None, doc) => doc, - }; + let doc = internal_doc_tokens(func.sig(), &py_name, None, doc, None, Some("$module")); let py_names = { if self.py_attrs.is_empty() { diff --git a/crates/derive-impl/src/util.rs b/crates/derive-impl/src/util.rs index fa53de0ac02..dca88136c80 100644 --- a/crates/derive-impl/src/util.rs +++ b/crates/derive-impl/src/util.rs @@ -2,7 +2,11 @@ use itertools::Itertools; use proc_macro2::{Span, TokenStream}; use quote::{ToTokens, quote}; use std::collections::{HashMap, HashSet}; -use syn::{Attribute, FnArg, Ident, Result, Signature, UseTree, ext::IdentExt, spanned::Spanned}; +use syn::visit::Visit; +use syn::visit_mut::VisitMut; +use syn::{ + Attribute, FnArg, Ident, Result, Signature, Type, UseTree, ext::IdentExt, spanned::Spanned, +}; use syn_ext::{ ext::{AttributeExt as SynAttributeExt, *}, types::*, @@ -736,26 +740,157 @@ where Ok(()) } -// Best effort attempt to generate a template from which a -// __text_signature__ can be created. -// -// 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( +enum SigPiece { + Marker(String), + Arg { + name: String, + ty: Type, + }, + /// Parameters of the argument a method binds as its receiver. + Implicit(Type), +} + +fn is_vm_or_callee(ty: &Type) -> bool { + let ty = quote!(#ty).to_string().replace(' ', ""); + (ty.starts_with('&') && ty.ends_with("VirtualMachine")) || ty.ends_with("Callee") +} + +fn arg_name(pat: &syn::Pat) -> String { + let syn::Pat::Ident(pat) = pat else { + return String::new(); + }; + let ident = pat.ident.unraw().to_string(); + ident.strip_prefix('_').unwrap_or(&ident).to_owned() +} + +fn mentions_self(ty: &Type) -> bool { + struct Finder(bool); + impl Visit<'_> for Finder { + fn visit_ident(&mut self, ident: &Ident) { + if ident == "Self" { + self.0 = true; + } + } + } + let mut finder = Finder(false); + finder.visit_type(ty); + finder.0 +} + +fn subst_self(ty: &Type, self_ty: Option<&Type>) -> Type { + struct Subst<'a>(&'a Type); + impl VisitMut for Subst<'_> { + fn visit_type_mut(&mut self, ty: &mut Type) { + if let Type::Path(path) = ty + && path.qself.is_none() + && path.path.is_ident("Self") + { + *ty = self.0.clone(); + return; + } + syn::visit_mut::visit_type_mut(self, ty); + } + } + let Some(self_ty) = self_ty else { + return ty.clone(); + }; + let mut ty = ty.clone(); + Subst(self_ty).visit_type_mut(&mut ty); + ty +} + +fn sig_pieces( sig: &Signature, - name: &str, implicit_self: Option<&str>, -) -> Option { - let signature = func_sig(sig, implicit_self)?; - // 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}({signature}, /)") + self_ty: Option<&Type>, + leading_marker: Option<&str>, +) -> Vec { + let mut pieces = Vec::new(); + if let Some(marker) = leading_marker { + pieces.push(SigPiece::Marker(marker.to_owned())); + } + let mut implicit_self = implicit_self.map(str::to_owned); + for arg in &sig.inputs { + match arg { + FnArg::Receiver(_) => pieces.push(SigPiece::Marker("$self".to_owned())), + FnArg::Typed(typed) => { + if is_vm_or_callee(&typed.ty) { + continue; + } + let name = arg_name(&typed.pat); + let ty = subst_self(&typed.ty, self_ty); + // References carry a lifetime, so `FromArgs` cannot be named + // from a const item. They are leaf positional parameters. + let leaf = mentions_self(&ty) || matches!(ty, Type::Reference(_)); + if let Some(marker) = implicit_self.take() { + // A nested const cannot name `Self`. The receiver is the + // marker; only a type that supplies parameters is kept. + pieces.push(SigPiece::Marker(marker)); + if !leaf { + pieces.push(SigPiece::Implicit(ty)); + } + } else if leaf { + pieces.push(SigPiece::Marker(name)); + } else { + pieces.push(SigPiece::Arg { name, ty }); + } + } + } + } + pieces +} + +fn sig_arg_tokens(piece: &SigPiece) -> TokenStream { + match piece { + SigPiece::Marker(name) => quote!(::rustpython_vm::function::SigArg::marker(#name)), + SigPiece::Arg { name, ty } => { + quote!(::rustpython_vm::function::SigArg::from_arg::<#ty>(#name)) + } + SigPiece::Implicit(ty) => quote!(::rustpython_vm::function::SigArg::implicit::<#ty>()), + } +} + +fn args_const(pieces: &[SigPiece]) -> TokenStream { + let args = pieces.iter().map(sig_arg_tokens); + quote! { + const ARGS: &[::rustpython_vm::function::SigArg] = &[#(#args),*]; + } +} + +/// Expression of type `Option<&'static str>`: the internal doc, or the plain +/// doc when the arguments cannot form a signature. +pub(crate) fn internal_doc_tokens( + sig: &Signature, + py_name: &str, + implicit_self: Option<&str>, + doc: Option, + self_ty: Option<&Type>, + leading_marker: Option<&str>, +) -> TokenStream { + let args_const = args_const(&sig_pieces(sig, implicit_self, self_ty, leading_marker)); + let plain = match &doc { + Some(doc) => quote!(Some(#doc)), + None => quote!(None), }; - Some(signature) + let doc_text = doc.unwrap_or_default(); + quote! { + { + #args_const + if !::rustpython_vm::function::has_signature(ARGS) { + #plain + } else { + const DOC: &str = #doc_text; + const N: usize = ::rustpython_vm::function::internal_doc_len(#py_name, ARGS, DOC); + const B: [u8; N] = + ::rustpython_vm::function::internal_doc_bytes::(#py_name, ARGS, DOC); + const S: &str = match ::core::str::from_utf8(&B) { + Ok(s) => s, + Err(_) => panic!(), + }; + Some(S) + } + } + } } pub(crate) fn infer_native_call_flags(sig: &Signature, drop_first_typed: usize) -> TokenStream { @@ -829,56 +964,3 @@ pub(crate) fn infer_native_call_flags(sig: &Signature, drop_first_typed: usize) } } } - -/// Returns None when an argument has no name to report, in which case no -/// signature can be generated for the function. -/// -/// `implicit_self` is the marker to report for a first argument that the call -/// binds to without a `&self` receiver. -fn func_sig(sig: &Signature, mut implicit_self: Option<&str>) -> 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; - } - }; - let ty = arg.ty.as_ref(); - let ty = quote!(#ty).to_string(); - if ty == "FuncArgs" { - // The bundle carries the receiver along with everything else, so - // report both rather than spending the marker on it. - params.extend(implicit_self.take().map(str::to_owned)); - params.push("*args, **kwargs".to_owned()); - continue; - } - if (ty.starts_with('&') && ty.ends_with("VirtualMachine")) || ty.ends_with("Callee") { - continue; - } - if let Some(marker) = implicit_self.take() { - params.push(marker.to_owned()); - 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.unraw().to_string(); - if ident == "vm" { - unreachable!("type &VirtualMachine(`{ty}`) must be filtered already"); - } - // A leading `_` only marks the argument unused in Rust. A parameter whose - // Python name really starts with `_` has to be a FromArgs field instead. - params.push(ident.strip_prefix('_').unwrap_or(&ident).to_owned()); - } - Some(params.join(", ")) -} - -pub(crate) fn format_doc(sig: &str, doc: &str) -> String { - format!("{sig}\n--\n\n{doc}") -} diff --git a/crates/derive/src/lib.rs b/crates/derive/src/lib.rs index af60a3cddda..d478024c1bc 100644 --- a/crates/derive/src/lib.rs +++ b/crates/derive/src/lib.rs @@ -7,6 +7,45 @@ use rustpython_derive_impl as derive_impl; use syn::parse_macro_input; use syn::punctuated::Punctuated; +/// Derive `FromArgs` for a struct whose fields are Python arguments. +/// +/// Each field takes one `#[pyarg(...)]`. The first item is the parameter kind. +/// # Arguments +/// - `positional`: positional-only. +/// - `any`: positional or keyword. +/// - `named`: keyword-only. +/// - `flatten`: take this field from the same argument list. No other keys. +/// - `name = "..."`: Python parameter name. The field name is used when omitted. +/// - `default`: missing argument stores `Default::default()`. Affects parsing. +/// - `default = `: missing argument stores that Rust value. Affects parsing. +/// - `optional`: same as a bare `default`. +/// - `py_default = ""`: text copied verbatim into `__text_signature__`. +/// Never affects parsing. +/// - `error_msg = "..."`: type-error text when conversion fails. +/// # Signature default +/// An explicit `py_default` is used as written. Otherwise a Rust literal is +/// converted to its Python repr (`True`/`False`, an int, a float, a quoted +/// str, and the path `None`). Anything else renders ``, and +/// `inspect.signature` raises `ValueError`. +/// +/// Prefer `default = ` when that literal is the Python default. +/// Use `py_default` only when the Rust value must differ. +/// ```rust, ignore +/// #[derive(FromArgs)] +/// struct OpenArgs { +/// #[pyarg(any, default = 0o777)] +/// mode: i32, // signature shows 511 +/// } +/// +/// #[derive(FromArgs)] +/// struct PrintOptions { +/// // None means a space; the string is filled in when printing. +/// #[pyarg(named, default, py_default = "' '")] +/// sep: Option, +/// #[pyarg(named, default = None)] +/// file: Option, +/// } +/// ``` #[proc_macro_derive(FromArgs, attributes(pyarg))] pub fn derive_from_args(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input); diff --git a/crates/vm/src/builtins/builtin_func.rs b/crates/vm/src/builtins/builtin_func.rs index 3d9d83e0c27..e41e56ef427 100644 --- a/crates/vm/src/builtins/builtin_func.rs +++ b/crates/vm/src/builtins/builtin_func.rs @@ -11,10 +11,14 @@ use alloc::fmt; // PyCFunctionObject in CPython #[repr(C)] -#[pyclass(name = "builtin_function_or_method", module = false)] +#[pyclass(name = "builtin_function_or_method", module = false, traverse)] pub struct PyNativeFunction { + #[pytraverse(skip)] pub(crate) value: &'static PyMethodDef, pub(crate) zelf: Option, + // Module that owns this function. Not passed as a call argument. + pub(crate) module_object: Option, + #[pytraverse(skip)] pub(crate) module: Option<&'static PyStrInterned>, // None for bound method /// Prevent HeapMethodDef from being freed while this function references it pub(crate) _method_def_owner: Option, @@ -46,6 +50,11 @@ impl PyNativeFunction { self } + pub fn with_module_object(mut self, module: PyObjectRef) -> Self { + self.module_object = Some(module); + self + } + pub fn into_ref(self, ctx: &Context) -> PyRef { PyRef::new_ref( self, @@ -99,6 +108,12 @@ impl Comparable for PyNativeFunction { (None, None) => true, _ => false, }; + let eq = eq + && match (zelf.module_object.as_ref(), other.module_object.as_ref()) { + (Some(z), Some(o)) => z.is(o), + (None, None) => true, + _ => false, + }; let eq = eq && core::ptr::eq(zelf.value, other.value); Ok(eq.into()) } else { @@ -176,7 +191,13 @@ impl PyNativeFunction { // meth_get__self__ in CPython #[pygetset] fn __self__(zelf: NativeFunctionOrMethod, vm: &VirtualMachine) -> PyObjectRef { - zelf.0.zelf.clone().unwrap_or_else(|| vm.ctx.none()) + if let Some(bound) = &zelf.0.zelf { + return bound.clone(); + } + if let Some(module) = &zelf.0.module_object { + return module.clone(); + } + vm.ctx.none() } // meth_reduce in CPython diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index 97086cc74f9..75ea4a10328 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/crates/vm/src/builtins/descriptor.rs @@ -144,11 +144,9 @@ impl PyMethodDescriptor { } #[pygetset] - fn __text_signature__(&self) -> Option { - self.method.doc.and_then(|doc| { - type_::get_text_signature_from_internal_doc(self.method.name, doc) - .map(|signature| signature.to_string()) - }) + fn __text_signature__(&self) -> Option<&'static str> { + let doc = self.method.doc?; + type_::get_text_signature_from_internal_doc(self.method.name, doc) } #[pygetset] @@ -305,11 +303,9 @@ impl PyClassMethodDescriptor { } #[pygetset] - fn __text_signature__(&self) -> Option { - self.method.doc.and_then(|doc| { - type_::get_text_signature_from_internal_doc(self.method.name, doc) - .map(|signature| signature.to_string()) - }) + fn __text_signature__(&self) -> Option<&'static str> { + let doc = self.method.doc?; + type_::get_text_signature_from_internal_doc(self.method.name, doc) } #[pygetset] diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index d4a5a6e10de..83e9305b4a7 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -367,6 +367,14 @@ impl PyDict { } } +#[derive(FromArgs)] +struct FromKeysArgs { + #[pyarg(positional)] + iterable: ArgIterable, + #[pyarg(positional, default = None)] + value: Option, +} + // Python dict methods: #[pyclass( with( @@ -385,13 +393,9 @@ impl PyDict { )] impl PyDict { #[pyclassmethod] - fn fromkeys( - class: PyTypeRef, - iterable: ArgIterable, - value: OptionalArg, - vm: &VirtualMachine, - ) -> PyResult { - let value = value.unwrap_or_none(vm); + fn fromkeys(class: PyTypeRef, args: FromKeysArgs, vm: &VirtualMachine) -> PyResult { + let FromKeysArgs { iterable, value } = args; + let value = value.unwrap_or_else(|| vm.ctx.none()); let d = PyType::call(&class, ().into(), vm)?; match d.downcast_exact::(vm) { Ok(pydict) => { diff --git a/crates/vm/src/builtins/int.rs b/crates/vm/src/builtins/int.rs index 23f91c6292f..31a497fd0cb 100644 --- a/crates/vm/src/builtins/int.rs +++ b/crates/vm/src/builtins/int.rs @@ -589,7 +589,7 @@ impl PyInt { #[pymethod] fn to_bytes(&self, args: IntToByteArgs, vm: &VirtualMachine) -> PyResult { - let signed = args.signed.map_or(false, Into::into); + let signed = args.signed; let byte_len = args.length; let value = self.as_bigint(); @@ -820,10 +820,11 @@ struct IntFromByteArgs { struct IntToByteArgs { #[pyarg(any, default = 1)] length: usize, - #[pyarg(any, default = ArgByteOrder::Big)] + // ArgByteOrder::Big is not the text 'big'. + #[pyarg(any, default = ArgByteOrder::Big, py_default = "'big'")] byteorder: ArgByteOrder, - #[pyarg(named, optional)] - signed: OptionalArg, + #[pyarg(named, default = false)] + signed: bool, } fn try_int_radix(obj: &PyObject, base: u32, vm: &VirtualMachine) -> PyResult { diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 8cff4500cb9..9b188aa7a54 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -158,7 +158,7 @@ impl PyList { #[derive(FromArgs, Default, Traverse)] pub(crate) struct SortOptions { - #[pyarg(named, default)] + #[pyarg(named, default = None)] key: Option, #[pytraverse(skip)] #[pyarg(named, default = false)] diff --git a/crates/vm/src/builtins/module.rs b/crates/vm/src/builtins/module.rs index 3fbd9704cf9..65d48795ca8 100644 --- a/crates/vm/src/builtins/module.rs +++ b/crates/vm/src/builtins/module.rs @@ -141,6 +141,7 @@ impl Py { let func = method .to_function() .with_module(self.name.unwrap()) + .with_module_object(self.to_owned().into()) .into_ref(&vm.ctx); vm.__module_set_attr(self, vm.ctx.intern_str(method.name), func)?; } diff --git a/crates/vm/src/bytes_inner.rs b/crates/vm/src/bytes_inner.rs index 9640bc1064b..0e57e8ba7a8 100644 --- a/crates/vm/src/bytes_inner.rs +++ b/crates/vm/src/bytes_inner.rs @@ -512,7 +512,7 @@ impl PyBytesInner { swapcase_ascii(self.as_bytes()) } - pub fn hex(&self, sep: Option, bytes_per_sep: OptionalArg) -> String { + pub fn hex(&self, sep: Option, bytes_per_sep: isize) -> String { bytes_to_hex(self.elements.as_slice(), sep, bytes_per_sep) } @@ -1231,8 +1231,8 @@ pub(crate) fn bytes_decode( pub(crate) struct ByteInnerHexOptions { #[pyarg(any, optional)] pub sep: OptionalArg>, - #[pyarg(any, optional)] - pub bytes_per_sep: OptionalArg, + #[pyarg(any, default = 1)] + pub bytes_per_sep: isize, } impl ByteInnerHexOptions { @@ -1240,7 +1240,7 @@ impl ByteInnerHexOptions { /// /// Measuring the separator runs Python, so it happens here, before the /// bytes to be written out are borrowed. _Py_strhex_impl - pub(crate) fn resolve(self, vm: &VirtualMachine) -> PyResult<(Option, OptionalArg)> { + pub(crate) fn resolve(self, vm: &VirtualMachine) -> PyResult<(Option, isize)> { let Self { sep, bytes_per_sep } = self; let OptionalArg::Present(sep) = sep else { return Ok((None, bytes_per_sep)); @@ -1323,12 +1323,7 @@ fn hex_impl(bytes: &[u8], sep: u8, bytes_per_sep: isize) -> String { unsafe { String::from_utf8_unchecked(buf) } } -pub(crate) fn bytes_to_hex( - bytes: &[u8], - sep: Option, - bytes_per_sep: OptionalArg, -) -> String { - let bytes_per_sep = bytes_per_sep.unwrap_or(1); +pub(crate) fn bytes_to_hex(bytes: &[u8], sep: Option, bytes_per_sep: isize) -> String { match sep { Some(sep) if bytes_per_sep != 0 && !bytes.is_empty() => hex_impl(bytes, sep, bytes_per_sep), _ => hex_impl_no_sep(bytes), diff --git a/crates/vm/src/function/argument.rs b/crates/vm/src/function/argument.rs index 589422aa16e..4c380480294 100644 --- a/crates/vm/src/function/argument.rs +++ b/crates/vm/src/function/argument.rs @@ -1,3 +1,4 @@ +use super::signature::Param; use crate::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine, builtins::{PyBaseExceptionRef, PyTupleRef, PyType}, @@ -99,6 +100,9 @@ impl From for FuncArgs { } impl FromArgs for FuncArgs { + const PARAMS: Option<&'static [Param]> = + Some(&[Param::var_positional("args"), Param::var_keyword("kwargs")]); + fn from_args(_vm: &VirtualMachine, args: &mut FuncArgs) -> Result { Ok(core::mem::take(args)) } @@ -519,6 +523,14 @@ pub trait FromArgs: Sized { 0..=0 } + /// Parameters this type contributes to a text signature. + /// + /// `None`: the argument is one positional-only parameter, named by the + /// function argument. `Some(&[])`: the argument contributes nothing. + /// `Some(params)`: the type supplies those parameters and the argument name + /// is ignored. + const PARAMS: Option<&'static [Param]> = None; + /// Extracts this item from the next argument(s). fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result; } @@ -662,6 +674,8 @@ impl FromArgs for KwArgs where T: TryFromObject, { + const PARAMS: Option<&'static [Param]> = Some(&[Param::var_keyword("kwargs")]); + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { let mut kwargs = KwArgsMap::default(); for (name, value) in args.remaining_keywords() { @@ -736,6 +750,8 @@ impl FromArgs for PosArgs where T: TryFromObject, { + const PARAMS: Option<&'static [Param]> = Some(&[Param::var_positional("args")]); + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { let mut varargs = Vec::new(); while let Some(value) = args.take_positional() { @@ -808,6 +824,12 @@ impl FromArgs for OptionalArg where T: TryFromObject, { + const PARAMS: Option<&'static [Param]> = Some(&[Param { + name: "", + kind: super::signature::ParamKind::PositionalOnly, + default: Some(""), + }]); + fn arity() -> RangeInclusive { 0..=1 } diff --git a/crates/vm/src/function/method.rs b/crates/vm/src/function/method.rs index d76fb31e77f..b5a46241f30 100644 --- a/crates/vm/src/function/method.rs +++ b/crates/vm/src/function/method.rs @@ -124,6 +124,7 @@ impl PyMethodDef { PyNativeFunction { zelf: None, value: self, + module_object: None, module: None, _method_def_owner: None, } @@ -146,6 +147,7 @@ impl PyMethodDef { func: PyNativeFunction { zelf: Some(obj), value: self, + module_object: None, module: None, _method_def_owner: None, }, @@ -165,6 +167,7 @@ impl PyMethodDef { let function = PyNativeFunction { zelf: Some(obj), value: self, + module_object: None, module: None, _method_def_owner: None, }; @@ -218,6 +221,7 @@ impl PyMethodDef { let func = PyNativeFunction { zelf: Some(class.to_owned().into()), value: self, + module_object: None, module: None, _method_def_owner: None, }; diff --git a/crates/vm/src/function/mod.rs b/crates/vm/src/function/mod.rs index 22351c410e1..62591783e6b 100644 --- a/crates/vm/src/function/mod.rs +++ b/crates/vm/src/function/mod.rs @@ -9,6 +9,7 @@ mod getset; pub(crate) mod method; mod number; mod protocol; +mod signature; mod time; pub use argument::{ @@ -29,6 +30,9 @@ pub(super) use getset::{IntoPyGetterFunc, IntoPySetterFunc, PyGetterFunc, PySett pub use method::{HeapMethodDef, PyMethodDef, PyMethodFlags}; pub use number::{ArgIndex, ArgIntoBool, ArgIntoComplex, ArgIntoFloat, ArgPrimitiveIndex, ArgSize}; pub use protocol::{ArgCallable, ArgIterable, ArgMapping, ArgSequence}; +pub use signature::{ + Param, ParamKind, SigArg, has_signature, internal_doc_bytes, internal_doc_len, +}; pub use time::TimeoutSeconds; use crate::{ diff --git a/crates/vm/src/function/signature.rs b/crates/vm/src/function/signature.rs new file mode 100644 index 00000000000..8167638e0b7 --- /dev/null +++ b/crates/vm/src/function/signature.rs @@ -0,0 +1,322 @@ +//! Compile-time text signatures for native functions. +//! +//! The internal doc is `name(params)\n--\n\ndoc`, built in a const context from +//! [`FromArgs`] metadata. Nothing here allocates. + +use super::argument::FromArgs; + +#[derive(Clone, Copy, Debug)] +pub enum ParamKind { + PositionalOnly, + PositionalOrKeyword, + KeywordOnly, + VarPositional, + VarKeyword, + /// Parameters contributed by a nested [`FromArgs`] type. + /// `None` contributes nothing. + Flatten(Option<&'static [Param]>), +} + +#[derive(Clone, Copy, Debug)] +pub struct Param { + pub name: &'static str, + pub kind: ParamKind, + pub default: Option<&'static str>, +} + +impl Param { + #[must_use] + pub const fn positional_only(name: &'static str) -> Self { + Self { + name, + kind: ParamKind::PositionalOnly, + default: None, + } + } + + #[must_use] + pub const fn positional_or_keyword(name: &'static str) -> Self { + Self { + name, + kind: ParamKind::PositionalOrKeyword, + default: None, + } + } + + #[must_use] + pub const fn keyword_only(name: &'static str, default: Option<&'static str>) -> Self { + Self { + name, + kind: ParamKind::KeywordOnly, + default, + } + } + + #[must_use] + pub const fn var_positional(name: &'static str) -> Self { + Self { + name, + kind: ParamKind::VarPositional, + default: None, + } + } + + #[must_use] + pub const fn var_keyword(name: &'static str) -> Self { + Self { + name, + kind: ParamKind::VarKeyword, + default: None, + } + } + + #[must_use] + pub const fn flatten(params: Option<&'static [Self]>) -> Self { + Self { + name: "", + kind: ParamKind::Flatten(params), + default: None, + } + } +} + +/// One Rust argument of a native function. +/// +/// `params == None`: one positional-only parameter named [`name`](Self::name). +/// `Some(ps)`: the type supplies `ps` and `name` is ignored. +/// `Some(&[])`: the argument contributes nothing. +#[derive(Clone, Copy, Debug)] +pub struct SigArg { + pub name: &'static str, + pub params: Option<&'static [Param]>, +} + +impl SigArg { + #[must_use] + pub const fn from_arg(name: &'static str) -> Self { + Self { + name, + params: T::PARAMS, + } + } + + /// `$self` / `$type` receiver marker. Renders as one positional-only parameter. + #[must_use] + pub const fn marker(name: &'static str) -> Self { + Self { name, params: None } + } + + /// Parameters supplied by the argument a method binds as its receiver. + /// `None` metadata contributes nothing, so this never looks nameless. + #[must_use] + pub const fn implicit() -> Self { + Self { + name: "", + params: Some(match T::PARAMS { + Some(ps) => ps, + None => &[], + }), + } + } +} + +/// False when an argument is a destructured pattern whose type has no [`FromArgs::PARAMS`]. +#[must_use] +pub const fn has_signature(args: &[SigArg]) -> bool { + let mut i = 0; + while i < args.len() { + if args[i].params.is_none() && args[i].name.is_empty() { + return false; + } + i += 1; + } + true +} + +#[must_use] +pub const fn internal_doc_len(name: &str, args: &[SigArg], doc: &str) -> usize { + write_internal_doc(&mut [], name, args, doc) +} + +#[must_use] +pub const fn internal_doc_bytes(name: &str, args: &[SigArg], doc: &str) -> [u8; N] { + let mut buf = [0u8; N]; + let written = write_internal_doc(&mut buf, name, args, doc); + assert!(written == N); + buf +} + +struct St { + n: usize, + emitted: bool, + po_left: usize, + var_pos_seen: bool, + star_emitted: bool, +} + +const fn write_internal_doc(buf: &mut [u8], name: &str, args: &[SigArg], doc: &str) -> usize { + let mut n = put_str(buf, 0, name); + n = put_byte(buf, n, b'('); + let st = write_args( + buf, + St { + n, + emitted: false, + po_left: count_po_args(args), + var_pos_seen: false, + star_emitted: false, + }, + args, + ); + n = put_str(buf, st.n, ")\n--\n\n"); + put_str(buf, n, doc) +} + +const fn put(buf: &mut [u8], i: usize, bytes: &[u8]) -> usize { + let mut k = 0; + while k < bytes.len() { + let at = i + k; + if at < buf.len() { + buf[at] = bytes[k]; + } + k += 1; + } + i + bytes.len() +} + +const fn put_str(buf: &mut [u8], i: usize, s: &str) -> usize { + put(buf, i, s.as_bytes()) +} + +const fn put_byte(buf: &mut [u8], i: usize, b: u8) -> usize { + put(buf, i, &[b]) +} + +const fn count_po_params(params: &[Param]) -> usize { + let mut n = 0; + let mut i = 0; + while i < params.len() { + match params[i].kind { + ParamKind::PositionalOnly => n += 1, + ParamKind::Flatten(Some(inner)) => n += count_po_params(inner), + _ => {} + } + i += 1; + } + n +} + +const fn count_po_args(args: &[SigArg]) -> usize { + let mut n = 0; + let mut i = 0; + while i < args.len() { + match args[i].params { + None => n += 1, + Some(ps) => n += count_po_params(ps), + } + i += 1; + } + n +} + +const fn emit_sep(buf: &mut [u8], st: St) -> St { + if st.emitted { + St { + n: put_str(buf, st.n, ", "), + ..st + } + } else { + st + } +} + +const fn emit_text(buf: &mut [u8], st: St, text: &str) -> St { + let st = emit_sep(buf, st); + St { + n: put_str(buf, st.n, text), + emitted: true, + ..st + } +} + +const fn emit_named(buf: &mut [u8], st: St, prefix: &str, name: &str, default: Option<&str>) -> St { + let st = emit_sep(buf, st); + let n = put_str(buf, st.n, prefix); + let n = put_str(buf, n, name); + let n = if let Some(default) = default { + let n = put_byte(buf, n, b'='); + put_str(buf, n, default) + } else { + n + }; + St { + n, + emitted: true, + ..st + } +} + +const fn param_name<'a>(name: &'a str, fallback: &'a str) -> &'a str { + if name.is_empty() { fallback } else { name } +} + +const fn write_params(buf: &mut [u8], mut st: St, params: &[Param], fallback: &str) -> St { + let mut i = 0; + while i < params.len() { + st = write_one(buf, st, params[i], fallback); + i += 1; + } + st +} + +const fn write_one(buf: &mut [u8], mut st: St, param: Param, fallback: &str) -> St { + let name = param_name(param.name, fallback); + match param.kind { + ParamKind::Flatten(None) => st, + ParamKind::Flatten(Some(inner)) => write_params(buf, st, inner, ""), + ParamKind::PositionalOnly => { + st = emit_named(buf, st, "", name, param.default); + st.po_left -= 1; + if st.po_left == 0 { + st = emit_text(buf, st, "/"); + } + st + } + ParamKind::PositionalOrKeyword => emit_named(buf, st, "", name, param.default), + ParamKind::KeywordOnly => { + if !st.var_pos_seen && !st.star_emitted { + st = emit_text(buf, st, "*"); + st.star_emitted = true; + } + emit_named(buf, st, "", name, param.default) + } + ParamKind::VarPositional => { + st.var_pos_seen = true; + emit_named(buf, st, "*", name, param.default) + } + ParamKind::VarKeyword => emit_named(buf, st, "**", name, param.default), + } +} + +const fn write_args(buf: &mut [u8], mut st: St, args: &[SigArg]) -> St { + let mut i = 0; + while i < args.len() { + match args[i].params { + None => { + st = write_one( + buf, + st, + Param { + name: args[i].name, + kind: ParamKind::PositionalOnly, + default: None, + }, + "", + ); + } + Some(ps) => st = write_params(buf, st, ps, args[i].name), + } + i += 1; + } + st +} diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs index 2f7145047ef..d4fcb3af94c 100644 --- a/crates/vm/src/stdlib/builtins.rs +++ b/crates/vm/src/stdlib/builtins.rs @@ -1076,14 +1076,17 @@ mod builtins { #[derive(Debug, Default, FromArgs)] pub struct PrintOptions { - #[pyarg(named, default)] + // None means a space; the string is filled in when printing. + #[pyarg(named, default, py_default = "' '")] sep: Option, - #[pyarg(named, default)] + // None means a newline; the string is filled in when printing. + #[pyarg(named, default, py_default = "'\\n'")] end: Option, - #[pyarg(named, default = ArgIntoBool::FALSE)] - flush: ArgIntoBool, - #[pyarg(named, default)] + #[pyarg(named, default = None)] file: Option, + // ArgIntoBool::FALSE is not the literal false. + #[pyarg(named, default = ArgIntoBool::FALSE, py_default = "False")] + flush: ArgIntoBool, } #[pyfunction] @@ -1139,8 +1142,8 @@ mod builtins { #[derive(FromArgs)] pub(super) struct RoundArgs { number: PyObjectRef, - #[pyarg(any, optional)] - ndigits: OptionalOption, + #[pyarg(any, default = None)] + ndigits: Option, } #[pyfunction] @@ -1153,7 +1156,7 @@ mod builtins { number.class().slot_name() )) })?; - match ndigits.flatten() { + match ndigits { Some(obj) => { let ndigits = obj.try_index(vm)?; meth.invoke((ndigits,), vm) diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 0e2cde94261..8f7b7361490 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -5,7 +5,7 @@ use crate::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyResult, TryFromObject, VirtualMachine, builtins::{PyModule, PySet}, convert::{IntoPyException, ToPyException, ToPyObject}, - function::{ArgumentError, FromArgs, FuncArgs}, + function::{ArgumentError, FromArgs, FuncArgs, Param}, host_env::{crt_fd, posix::RawMode}, ospath::OsPath, }; @@ -36,6 +36,7 @@ const DEFAULT_DIR_FD: crt_fd::Borrowed<'static> = unsafe { crt_fd::Borrowed::bor pub trait DirFdKeyword: Clone + Copy + Eq + PartialEq { const NAME: &'static str; + const PARAMS: &'static [Param] = &[Param::keyword_only(Self::NAME, Some("None"))]; } #[derive(Clone, Copy, Eq, PartialEq)] @@ -91,6 +92,8 @@ impl<'fd, KW: DirFdKeyword> DirFd<'fd, 1, KW> { } impl FromArgs for DirFd<'_, AVAILABLE, KW> { + const PARAMS: Option<&'static [Param]> = Some(KW::PARAMS); + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { let fd = match args.take_keyword(KW::NAME) { Some(o) if vm.is_none(&o) => Ok(DEFAULT_DIR_FD), @@ -271,8 +274,8 @@ pub(super) mod _os { struct OpenArgs<'fd> { path: OsPath, flags: i32, - #[pyarg(any, default)] - mode: Option, + #[pyarg(any, default = 0o777)] + mode: i32, #[pyarg(flatten)] dir_fd: DirFd<'fd, { OPEN_DIR_FD as usize }>, } @@ -286,11 +289,10 @@ pub(super) mod _os { pub(crate) fn os_open( name: OsPath, flags: i32, - mode: Option, + mode: i32, dir_fd: DirFd<'_, { OPEN_DIR_FD as usize }>, vm: &VirtualMachine, ) -> PyResult { - let mode = mode.unwrap_or(0o777); #[cfg(windows)] let fd = { let [] = dir_fd.0; @@ -399,17 +401,20 @@ pub(super) mod _os { } } - #[pyfunction] - fn mkdir( + #[derive(FromArgs)] + struct MkdirArgs<'a> { + #[pyarg(any)] path: OsPath, - mode: OptionalArg, - #[cfg_attr(not(any(unix, target_os = "wasi")), expect(unused_variables))] dir_fd: DirFd< - '_, - { MKDIR_DIR_FD as usize }, - >, - vm: &VirtualMachine, - ) -> PyResult<()> { - let mode = mode.unwrap_or(0o777); + #[pyarg(any, default = 0o777)] + mode: RawMode, + #[pyarg(flatten)] + #[cfg_attr(not(any(unix, target_os = "wasi")), expect(unused))] + dir_fd: DirFd<'a, { MKDIR_DIR_FD as usize }>, + } + + #[pyfunction] + fn mkdir(args: MkdirArgs<'_>, vm: &VirtualMachine) -> PyResult<()> { + let MkdirArgs { path, mode, dir_fd } = args; #[cfg(any(unix, target_os = "wasi"))] let dir_fd = dir_fd.get_opt(); #[cfg(not(any(unix, target_os = "wasi")))] @@ -446,13 +451,16 @@ pub(super) mod _os { const LISTDIR_FD: bool = cfg!(all(unix, not(target_os = "redox"))); + #[derive(FromArgs)] + struct ListDirArgs<'a> { + #[pyarg(any, default = None)] + path: Option>, + } + #[pyfunction] - fn listdir( - path: OptionalArg>>, - vm: &VirtualMachine, - ) -> PyResult> { - let path = path - .flatten() + fn listdir(args: ListDirArgs<'_>, vm: &VirtualMachine) -> PyResult> { + let path = args + .path .unwrap_or_else(|| OsPathOrFd::Path(OsPath::new_str("."))); let list = match path { OsPathOrFd::Path(path) => { @@ -822,7 +830,7 @@ pub(super) mod _os { dir_fd }; let do_stat = |follow_symlinks| { - stat( + stat_at( OsPath { path: self.pathval.as_os_str().to_owned(), origin: None, @@ -1457,9 +1465,17 @@ pub(super) mod _os { } } - #[pyfunction] - #[pyfunction(name = "fstat")] - fn stat( + #[derive(FromArgs)] + struct StatArgs<'a> { + #[pyarg(any)] + path: OsPathOrFd<'a>, + #[pyarg(flatten)] + dir_fd: DirFd<'a, { STAT_DIR_FD as usize }>, + #[pyarg(flatten)] + follow_symlinks: FollowSymlinks, + } + + fn stat_at( path: OsPathOrFd<'_>, dir_fd: DirFd<'_, { STAT_DIR_FD as usize }>, follow_symlinks: FollowSymlinks, @@ -1474,13 +1490,24 @@ pub(super) mod _os { Ok(StatResultData::from_stat(&stat, vm).to_pyobject(vm)) } + #[pyfunction] + #[pyfunction(name = "fstat")] + fn stat(args: StatArgs<'_>, vm: &VirtualMachine) -> PyResult { + let StatArgs { + path, + dir_fd, + follow_symlinks, + } = args; + stat_at(path, dir_fd, follow_symlinks, vm) + } + #[pyfunction] fn lstat( path: OsPath, dir_fd: DirFd<'_, { STAT_DIR_FD as usize }>, vm: &VirtualMachine, ) -> PyResult { - stat(path.into(), dir_fd, FollowSymlinks(false), vm) + stat_at(path.into(), dir_fd, FollowSymlinks(false), vm) } fn curdir_inner(vm: &VirtualMachine) -> PyResult { From 269d548b24aefaea550aa8ea5bc156774e8edd39 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 24 Sep 2026 18:33:52 +0900 Subject: [PATCH 2/3] Update builtin_signature snippet and fix mkdir on Windows round and sum now report their FromArgs parameters, so the snippet asserts their signatures instead of their absence. sum's start is a PyObjectRef defaulting to int 0 with py_default "0". mkdir reads dir_fd only on platforms that use it. Assisted-by: Claude:claude-opus-5-5 --- crates/vm/src/stdlib/builtins.rs | 10 ++++---- crates/vm/src/stdlib/os.rs | 7 +++--- extra_tests/snippets/builtin_signature.py | 28 +++++++---------------- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs index d4fcb3af94c..c37ff12bb3b 100644 --- a/crates/vm/src/stdlib/builtins.rs +++ b/crates/vm/src/stdlib/builtins.rs @@ -1201,8 +1201,9 @@ mod builtins { pub(super) struct SumArgs { #[pyarg(positional)] iterable: ArgIterable, - #[pyarg(any, optional)] - start: OptionalArg, + // The int object needs the VM, so the default is not a literal. + #[pyarg(any, default = vm.ctx.new_int(0).into(), py_default = "0")] + start: PyObjectRef, } #[expect( @@ -1211,10 +1212,7 @@ mod builtins { )] #[pyfunction] fn sum(SumArgs { iterable, start }: SumArgs, vm: &VirtualMachine) -> PyResult { - // Start with zero and add at will: - let mut sum = start - .into_option() - .unwrap_or_else(|| vm.ctx.new_int(0).into()); + let mut sum = start; match_class!(match sum { PyStr => diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 8f7b7361490..1ac11560097 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -414,14 +414,13 @@ pub(super) mod _os { #[pyfunction] fn mkdir(args: MkdirArgs<'_>, vm: &VirtualMachine) -> PyResult<()> { - let MkdirArgs { path, mode, dir_fd } = args; #[cfg(any(unix, target_os = "wasi"))] - let dir_fd = dir_fd.get_opt(); + let dir_fd = args.dir_fd.get_opt(); #[cfg(not(any(unix, target_os = "wasi")))] let dir_fd = None; - crate::host_env::posix::make_dir(dir_fd, &path.path, mode) - .map_err(|err| OSErrorBuilder::with_filename(&err, path, vm)) + crate::host_env::posix::make_dir(dir_fd, &args.path.path, args.mode) + .map_err(|err| OSErrorBuilder::with_filename(&err, args.path, vm)) } #[pyfunction] diff --git a/extra_tests/snippets/builtin_signature.py b/extra_tests/snippets/builtin_signature.py index 88857ac4b5d..be0d53e9e3c 100644 --- a/extra_tests/snippets/builtin_signature.py +++ b/extra_tests/snippets/builtin_signature.py @@ -1,13 +1,12 @@ import inspect import os -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. +# No phantom `module` parameter. The signature marks it as `$module` and +# `__self__` is the module, so inspect strips it. 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__ @@ -59,20 +58,9 @@ assert str(inspect.signature([].__dir__)) == "()" assert str(inspect.signature(float.fromhex)) == "(string, /)" -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") +# Functions whose arguments come from a FromArgs struct report the struct's +# parameters. +assert round.__text_signature__ == "($module, /, number, ndigits=None)" +assert sum.__text_signature__ == "($module, iterable, /, start=0)" +assert str(inspect.signature(round)) == "(number, ndigits=None)" +assert str(inspect.signature(sum)) == "(iterable, /, start=0)" From 74a56e6cd877a63d87907c0304756834010c57a9 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 24 Sep 2026 19:20:33 +0900 Subject: [PATCH 3/3] Remove expectedFailure from two signature tests test_cmp_to_signature and test_autospec_on_bound_builtin_function now pass. The TestCmpToKeyC override existed only for the marker. Assisted-by: Claude:claude-opus-5-5 --- Lib/test/test_functools.py | 4 ---- Lib/test/test_unittest/testmock/testhelpers.py | 1 - 2 files changed, 5 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 431e586485f..c06140dbd58 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -1238,10 +1238,6 @@ class TestCmpToKeyC(TestCmpToKey, unittest.TestCase): if c_functools: cmp_to_key = c_functools.cmp_to_key - @unittest.expectedFailure # TODO: RUSTPYTHON; inspect.signature reports (args, /) until FromArgs fields generate __text_signature__ - def test_cmp_to_signature(self): - super().test_cmp_to_signature() - @support.cpython_only def test_disallow_instantiation(self): # Ensure that the type disallows instantiation (bpo-43916) diff --git a/Lib/test/test_unittest/testmock/testhelpers.py b/Lib/test/test_unittest/testmock/testhelpers.py index 6877e92cb3c..bcae14c5b3c 100644 --- a/Lib/test/test_unittest/testmock/testhelpers.py +++ b/Lib/test/test_unittest/testmock/testhelpers.py @@ -929,7 +929,6 @@ def check_data_descriptor(mock_attr): check_data_descriptor(foo.desc) - @unittest.expectedFailure # TODO: RUSTPYTHON; time.ctime has an auto-generated __text_signature__, so inspect.signature() succeeds instead of raising ValueError def test_autospec_on_bound_builtin_function(self): meth = types.MethodType(time.ctime, time.time()) self.assertIsInstance(meth(), str)