Skip to content

Commit d8bb7bb

Browse files
leehanjeongyouknowone
leehanjeong
and
youknowone
authored
builtins: generate accurate __text_signature__ (RustPython#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>
1 parent fdd101b commit d8bb7bb

6 files changed

Lines changed: 161 additions & 68 deletions

File tree

‎Lib/test/test_pydoc/test_pydoc.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1582,7 +1582,6 @@ def test_module_level_callable(self):
15821582
self.assertEqual(self._get_summary_line(os.stat),
15831583
"stat(path, *, dir_fd=None, follow_symlinks=True)")
15841584

1585-
@unittest.expectedFailure # TODO: RUSTPYTHON
15861585
def test_module_level_callable_noargs(self):
15871586
self.assertEqual(self._get_summary_line(time.time),
15881587
"time()")

‎crates/derive-impl/src/pyclass.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,10 @@ where
10961096
args.attrs.push(allow_attr);
10971097
}
10981098

1099-
let doc = args.attrs.doc().map(|doc| format_doc(&sig_doc, &doc));
1099+
let doc = args.attrs.doc().map(|doc| match &sig_doc {
1100+
Some(sig_doc) => format_doc(sig_doc, &doc),
1101+
None => doc,
1102+
});
11001103
args.context.method_items.add_item(MethodNurseryItem {
11011104
py_name,
11021105
cfgs: args.cfgs.to_vec(),

‎crates/derive-impl/src/pymodule.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ struct FunctionNurseryItem {
524524
py_names: Vec<String>,
525525
cfgs: Vec<Attribute>,
526526
ident: Ident,
527-
doc: String,
527+
doc: Option<String>,
528528
call_flags: TokenStream,
529529
}
530530

@@ -556,8 +556,10 @@ impl ToTokens for ValidatedFunctionNursery {
556556
let cfgs = &item.cfgs;
557557
let cfgs = quote!(#(#cfgs)*);
558558
let py_names = &item.py_names;
559-
let doc = &item.doc;
560-
let doc = quote!(Some(#doc));
559+
let doc = match &item.doc {
560+
Some(doc) => quote!(Some(#doc)),
561+
None => quote!(None),
562+
};
561563
let flags = &item.call_flags;
562564

563565
inner_tokens.extend(quote![
@@ -671,10 +673,10 @@ impl ModuleItem for FunctionItem {
671673
.copied()
672674
.map(str::to_owned)
673675
});
674-
let doc = if let Some(doc) = doc {
675-
format_doc(&sig_doc, &doc)
676-
} else {
677-
sig_doc
676+
let doc = match (sig_doc, doc) {
677+
(Some(sig_doc), Some(doc)) => Some(format_doc(&sig_doc, &doc)),
678+
(Some(sig_doc), None) => Some(sig_doc),
679+
(None, doc) => doc,
678680
};
679681

680682
let py_names = {

‎crates/derive-impl/src/util.rs‎

Lines changed: 50 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -732,13 +732,20 @@ where
732732

733733
// Best effort attempt to generate a template from which a
734734
// __text_signature__ can be created.
735-
pub(crate) fn text_signature(sig: &Signature, name: &str) -> String {
736-
let signature = func_sig(sig);
737-
if signature.starts_with("$self") {
735+
//
736+
// Unlike CPython, a `#[pyfunction]` doesn't take the module as an argument yet,
737+
// so there's no module to mark with `$module`.
738+
pub(crate) fn text_signature(sig: &Signature, name: &str) -> Option<String> {
739+
let signature = func_sig(sig)?;
740+
// Arguments bind through `FuncArgs::take_positional`, which never consults
741+
// the keyword map, so they are positional-only. `*args`/`**kwargs` cannot be
742+
// followed by `/`, and an empty parameter list has nothing to mark.
743+
let signature = if signature.is_empty() || signature.contains('*') {
738744
format!("{name}({signature})")
739745
} else {
740-
format!("{}({}, {})", name, "$module", signature)
741-
}
746+
format!("{name}({signature}, /)")
747+
};
748+
Some(signature)
742749
}
743750

744751
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)
812819
}
813820
}
814821

815-
fn func_sig(sig: &Signature) -> String {
816-
sig.inputs
817-
.iter()
818-
.filter_map(|arg| {
819-
let arg = match arg {
820-
FnArg::Typed(typed) => typed,
821-
FnArg::Receiver(_) => return Some("$self".to_owned()),
822-
};
823-
let ty = arg.ty.as_ref();
824-
let ty = quote!(#ty).to_string();
825-
if ty == "FuncArgs" {
826-
return Some("*args, **kwargs".to_owned());
827-
}
828-
if ty.starts_with('&') && ty.ends_with("VirtualMachine") {
829-
return None;
830-
}
831-
let ident = match arg.pat.as_ref() {
832-
syn::Pat::Ident(p) => p.ident.to_string(),
833-
// FIXME: other => unreachable!("function arg pattern must be ident but found `{}`", quote!(fn #ident(.. #other ..))),
834-
other => quote!(#other).to_string(),
835-
};
836-
if ident == "zelf" {
837-
return Some("$self".to_owned());
838-
}
839-
if ident == "vm" {
840-
unreachable!("type &VirtualMachine(`{ty}`) must be filtered already");
822+
/// Returns None when an argument has no name to report, in which case no
823+
/// signature can be generated for the function.
824+
fn func_sig(sig: &Signature) -> Option<String> {
825+
let mut params = Vec::new();
826+
for arg in &sig.inputs {
827+
let arg = match arg {
828+
FnArg::Typed(typed) => typed,
829+
FnArg::Receiver(_) => {
830+
params.push("$self".to_owned());
831+
continue;
841832
}
842-
Some(ident)
843-
})
844-
.collect::<Vec<_>>()
845-
.join(", ")
833+
};
834+
let ty = arg.ty.as_ref();
835+
let ty = quote!(#ty).to_string();
836+
if ty == "FuncArgs" {
837+
params.push("*args, **kwargs".to_owned());
838+
continue;
839+
}
840+
if ty.starts_with('&') && ty.ends_with("VirtualMachine") {
841+
continue;
842+
}
843+
// An argument bound by a destructuring pattern, e.g.
844+
// `fn round(RoundArgs { number, ndigits }: RoundArgs, ..)`, has no name
845+
// to report. Stringifying the pattern would emit Rust syntax, which
846+
// makes inspect.signature() raise "invalid signature".
847+
let syn::Pat::Ident(pat) = arg.pat.as_ref() else {
848+
return None;
849+
};
850+
let ident = pat.ident.to_string();
851+
if ident == "zelf" {
852+
params.push("$self".to_owned());
853+
continue;
854+
}
855+
if ident == "vm" {
856+
unreachable!("type &VirtualMachine(`{ty}`) must be filtered already");
857+
}
858+
params.push(ident);
859+
}
860+
Some(params.join(", "))
846861
}
847862

848863
pub(crate) fn format_doc(sig: &str, doc: &str) -> String {

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

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ mod builtins {
7474
}
7575

7676
#[pyfunction]
77-
fn bin(x: PyIntRef) -> String {
78-
let x = x.as_bigint();
77+
fn bin(number: PyIntRef) -> String {
78+
let x = number.as_bigint();
7979
if x.is_negative() {
8080
format!("-0b{:b}", x.abs())
8181
} else {
@@ -392,11 +392,11 @@ mod builtins {
392392
}
393393

394394
#[pyfunction]
395-
fn delattr(obj: PyObjectRef, attr: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
396-
let attr = attr.try_to_ref::<PyStr>(vm).map_err(|_e| {
395+
fn delattr(obj: PyObjectRef, name: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
396+
let attr = name.try_to_ref::<PyStr>(vm).map_err(|_e| {
397397
vm.new_type_error(format!(
398398
"attribute name must be string, not '{}'",
399-
attr.class().name()
399+
name.class().name()
400400
))
401401
})?;
402402
obj.del_attr(attr, vm)
@@ -408,8 +408,8 @@ mod builtins {
408408
}
409409

410410
#[pyfunction]
411-
fn divmod(a: PyObjectRef, b: PyObjectRef, vm: &VirtualMachine) -> PyResult {
412-
vm._divmod(&a, &b)
411+
fn divmod(x: PyObjectRef, y: PyObjectRef, vm: &VirtualMachine) -> PyResult {
412+
vm._divmod(&x, &y)
413413
}
414414

415415
#[derive(FromArgs)]
@@ -715,11 +715,11 @@ mod builtins {
715715
}
716716

717717
#[pyfunction]
718-
fn hasattr(obj: PyObjectRef, attr: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
719-
let attr = attr.try_to_ref::<PyStr>(vm).map_err(|_e| {
718+
fn hasattr(obj: PyObjectRef, name: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
719+
let attr = name.try_to_ref::<PyStr>(vm).map_err(|_e| {
720720
vm.new_type_error(format!(
721721
"attribute name must be string, not '{}'",
722-
attr.class().name()
722+
name.class().name()
723723
))
724724
})?;
725725
Ok(vm.get_attribute_opt(obj, attr)?.is_some())
@@ -821,13 +821,21 @@ mod builtins {
821821
}
822822

823823
#[pyfunction]
824-
fn isinstance(obj: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
825-
obj.is_instance(&typ, vm)
824+
fn isinstance(
825+
obj: PyObjectRef,
826+
class_or_tuple: PyObjectRef,
827+
vm: &VirtualMachine,
828+
) -> PyResult<bool> {
829+
obj.is_instance(&class_or_tuple, vm)
826830
}
827831

828832
#[pyfunction]
829-
fn issubclass(subclass: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
830-
subclass.is_subclass(&typ, vm)
833+
fn issubclass(
834+
cls: PyObjectRef,
835+
class_or_tuple: PyObjectRef,
836+
vm: &VirtualMachine,
837+
) -> PyResult<bool> {
838+
cls.is_subclass(&class_or_tuple, vm)
831839
}
832840

833841
#[pyfunction]
@@ -848,8 +856,8 @@ mod builtins {
848856
}
849857

850858
#[pyfunction]
851-
fn aiter(iter_target: PyObjectRef, vm: &VirtualMachine) -> PyResult {
852-
iter_target.get_aiter(vm)
859+
fn aiter(async_iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult {
860+
async_iterable.get_aiter(vm)
853861
}
854862

855863
#[pyfunction]
@@ -998,8 +1006,8 @@ mod builtins {
9981006

9991007
#[pyfunction]
10001008
// builtin_ord
1001-
fn ord(c: PyObjectRef, vm: &VirtualMachine) -> PyResult<u32> {
1002-
let bytes = if let Some(string) = c.downcast_ref::<PyStr>() {
1009+
fn ord(character: PyObjectRef, vm: &VirtualMachine) -> PyResult<u32> {
1010+
let bytes = if let Some(string) = character.downcast_ref::<PyStr>() {
10031011
return match string.as_wtf8().code_points().exactly_one() {
10041012
Ok(character) => Ok(character.to_u32()),
10051013
Err(_) => {
@@ -1009,14 +1017,14 @@ mod builtins {
10091017
)))
10101018
}
10111019
};
1012-
} else if let Some(bytes) = c.downcast_ref::<PyBytes>() {
1020+
} else if let Some(bytes) = character.downcast_ref::<PyBytes>() {
10131021
bytes.as_bytes().to_vec()
1014-
} else if let Some(bytearray) = c.downcast_ref::<PyByteArray>() {
1022+
} else if let Some(bytearray) = character.downcast_ref::<PyByteArray>() {
10151023
bytearray.borrow_buf().to_vec()
10161024
} else {
10171025
return Err(vm.new_type_error(format!(
10181026
"ord() expected string of length 1, but {} found",
1019-
c.class().name()
1027+
character.class().name()
10201028
)));
10211029
};
10221030
let bytes_len = bytes.len();
@@ -1149,14 +1157,14 @@ mod builtins {
11491157
#[pyfunction]
11501158
fn setattr(
11511159
obj: PyObjectRef,
1152-
attr: PyObjectRef,
1160+
name: PyObjectRef,
11531161
value: PyObjectRef,
11541162
vm: &VirtualMachine,
11551163
) -> PyResult<()> {
1156-
let attr = attr.try_to_ref::<PyStr>(vm).map_err(|_e| {
1164+
let attr = name.try_to_ref::<PyStr>(vm).map_err(|_e| {
11571165
vm.new_type_error(format!(
11581166
"attribute name must be string, not '{}'",
1159-
attr.class().name()
1167+
name.class().name()
11601168
))
11611169
})?;
11621170
obj.set_attr(attr, value, vm)?;

‎extra_tests/snippets/builtin_signature.py‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import inspect
2+
import sys
3+
4+
# __text_signature__ is generated from the Rust parameter list, so it must not
5+
# describe parameters the function does not actually take, and must mark the
6+
# ones it does take as positional-only.
7+
8+
# No phantom `module` parameter. RustPython's #[pyfunction]s take no module
9+
# argument, so `__self__` is None and inspect has nothing to strip.
10+
for f in (len, abs, hash, id, repr, bin, ord, divmod, hex, oct, chr, callable):
11+
assert "module" not in inspect.signature(f).parameters, f.__name__
12+
13+
# Plain arguments bind through take_positional(), so they are positional-only.
14+
try:
15+
len(obj=[1, 2])
16+
except TypeError:
17+
pass
18+
else:
19+
raise AssertionError("len() should not accept keyword arguments")
20+
21+
assert str(inspect.signature(len)) == "(obj, /)"
22+
assert str(inspect.signature(abs)) == "(x, /)"
23+
assert str(inspect.signature(hash)) == "(obj, /)"
24+
assert str(inspect.signature(chr)) == "(i, /)"
25+
assert str(inspect.signature(callable)) == "(obj, /)"
26+
27+
assert (
28+
inspect.signature(len).parameters["obj"].kind == inspect.Parameter.POSITIONAL_ONLY
29+
)
30+
31+
# *args/**kwargs cannot be followed by `/`. The parameter names themselves still
32+
# differ from CPython here, which is out of scope.
33+
breakpoint_kinds = [p.kind for p in inspect.signature(breakpoint).parameters.values()]
34+
assert breakpoint_kinds == [
35+
inspect.Parameter.VAR_POSITIONAL,
36+
inspect.Parameter.VAR_KEYWORD,
37+
], breakpoint_kinds
38+
39+
# Parameter names follow CPython, so signatures are directly comparable.
40+
assert str(inspect.signature(bin)) == "(number, /)"
41+
assert str(inspect.signature(ord)) == "(character, /)"
42+
assert str(inspect.signature(divmod)) == "(x, y, /)"
43+
assert str(inspect.signature(hasattr)) == "(obj, name, /)"
44+
assert str(inspect.signature(setattr)) == "(obj, name, value, /)"
45+
assert str(inspect.signature(delattr)) == "(obj, name, /)"
46+
assert str(inspect.signature(isinstance)) == "(obj, class_or_tuple, /)"
47+
assert str(inspect.signature(issubclass)) == "(cls, class_or_tuple, /)"
48+
assert str(inspect.signature(aiter)) == "(async_iterable, /)"
49+
50+
if sys.implementation.name == "rustpython":
51+
# Functions whose Rust arguments are destructuring patterns rather than
52+
# plain names get no signature at all, instead of emitting text that is not
53+
# valid Python and makes inspect.signature() raise "invalid signature".
54+
#
55+
# CPython does have signatures for these, hand-written via Argument Clinic.
56+
# We cannot derive them until FromArgs reports the parameters of its own
57+
# structs, so until then we report no signature, which is at least how
58+
# CPython behaves for the builtins it has no signature for.
59+
for f in (round, sum):
60+
assert f.__text_signature__ is None, f.__name__
61+
try:
62+
inspect.signature(f)
63+
except ValueError as e:
64+
assert "no signature found" in str(e), str(e)
65+
else:
66+
raise AssertionError(f"{f.__name__} should have no signature")

0 commit comments

Comments
 (0)