Visitar URL original
Reject unsupported native keywords before argument binding by widehyo1 · Pull Request #8957 · RustPython/RustPython · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Reject unsupported native keywords before argument binding
Add explicit keyword dispatch metadata independently of inferred Rust
calling flags. Reject nonempty keywords before binding and conversion,
while preserving descriptor self checks and keyword-capable calls.

Annotate 31 builtins and list.append/list.clear, derive C-API policies
from ml_flags, and group dynamic registration metadata in MethodDefSpec.
Add regression coverage and remove the id doctest expected-failure marker.

Validation: workspace and C-API tests/clippy, VM without doc, keyword
regressions, and test_call/test_extcall/test_builtin/test_list passed.
The full snippet suite had 475 passes and 7 failures: six CPython-side
expectation mismatches and one missing optional SQLite module.

Assisted-by: Codex:GPT-6
  • Loading branch information
widehyo1 authored and youknowone committed Oct 8, 2026
commit dcb72df459cc43341df0f0e9e6d6ad46c15a18c3
2 changes: 1 addition & 1 deletion Lib/test/test_extcall.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@
... False
True

>>> id(1, **{'foo': 1}) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE
>>> id(1, **{'foo': 1})
Traceback (most recent call last):
...
TypeError: id() takes no keyword arguments
Expand Down
55 changes: 46 additions & 9 deletions crates/capi/src/methodobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::pystate::with_vm;
use crate::util::{CStrExt, FfiPtrExt};
use core::ffi::{c_char, c_int};
use core::fmt::Debug;
use rustpython_vm::function::{FuncArgs, HeapMethodDef, ItemDoc, PosArgs, PyMethodFlags};
use rustpython_vm::function::{
FuncArgs, HeapMethodDef, ItemDoc, KeywordDispatch, MethodDefSpec, PosArgs, PyMethodFlags,
};
use rustpython_vm::{AsObject, PyObjectRef, PyRef, PyResult, VirtualMachine};

define_py_check!(fn PyCFunction_Check, types.builtin_function_or_method_type);
Expand Down Expand Up @@ -80,6 +82,19 @@ pub(crate) fn build_method_def(

let flags = PyMethodFlags::from_bits(ml.ml_flags as u32)
.ok_or_else(|| vm.new_system_error("PyMethodDef contains unknown flags"))?;
let keyword_dispatch = if flags.contains(PyMethodFlags::KEYWORDS) {
KeywordDispatch::PassToBinder
} else if flags.contains(PyMethodFlags::VARARGS) {
KeywordDispatch::RejectNonemptyUnqualified
} else {
KeywordDispatch::RejectNonempty
};
let spec = MethodDefSpec {
name,
flags,
doc,
keyword_dispatch,
};
let has_self = has_self && !flags.contains(PyMethodFlags::STATIC);

let method = ml.ml_meth;
Expand All @@ -102,51 +117,51 @@ pub(crate) fn build_method_def(
let f = method.PyCFunction;
f(zelf.as_raw().cast_mut(), core::ptr::null_mut()).assume_owned_or_err(vm)
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
} else {
let callable = move |vm: &VirtualMachine| unsafe {
let f = method.PyCFunction;
f(core::ptr::null_mut(), core::ptr::null_mut()).assume_owned_or_err(vm)
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
}
},
PyMethodFlags::VARARGS => {
let callable = move |args: PosArgs, vm: &VirtualMachine| unsafe {
call_function(vm, method, flags, Some(args))
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
},
PyMethodFlags::VARARGS | PyMethodFlags::KEYWORDS => {
let callable = move | args: FuncArgs, vm: &VirtualMachine| unsafe {
call_function_with_keywords(vm, method, flags, args)
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
},
PyMethodFlags::FASTCALL | PyMethodFlags::KEYWORDS => {
let callable = move |args: FuncArgs, vm: &VirtualMachine| unsafe {
call_fast_function_with_keywords(vm, method, flags, args)
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
},
PyMethodFlags::FASTCALL => {
let callable = move |args: PosArgs, vm: &VirtualMachine| unsafe {
call_fast_function(vm, method, flags, args)
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
},
PyMethodFlags::O => {
let f = unsafe { method.PyCFunction };
if has_self {
let callable = move |zelf: PyObjectRef, arg: PyObjectRef, vm: &VirtualMachine| -> PyResult {
unsafe { f(zelf.as_raw().cast_mut(), arg.as_raw().cast_mut()).assume_owned_or_err(vm) }
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
} else {
let callable = move |arg: PyObjectRef, vm: &VirtualMachine| -> PyResult {
unsafe { f(core::ptr::null_mut(), arg.as_raw().cast_mut()).assume_owned_or_err(vm) }
};
Ok(vm.ctx.new_method_def(name, callable, flags, doc))
Ok(vm.ctx.new_method_def(callable, spec))
}
},
_ => {
Expand Down Expand Up @@ -385,4 +400,26 @@ mod tests {
func.call0().unwrap();
});
}

#[test]
fn no_keywords_before_arity() {
Python::attach(|py| {
unsafe extern "C" fn c_fn(_self: *mut PyObject, _args: *mut PyObject) -> *mut PyObject {
unsafe { PyLong_FromLong(42) }
}
let f = PyCFunction::new(py, c_fn, c"no_keywords", c"", None).unwrap();
let kwargs = pyo3::types::PyDict::new(py);
kwargs.set_item("unexpected", 1).unwrap();
let err = f.call((1,), Some(&kwargs)).unwrap_err();
assert_eq!(
err.value(py).str().unwrap().to_str().unwrap(),
"no_keywords() takes no keyword arguments"
);
kwargs.clear();
assert_eq!(
f.call((), Some(&kwargs)).unwrap().extract::<i32>().unwrap(),
42
);
});
}
}
12 changes: 10 additions & 2 deletions crates/derive-impl/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,7 @@ where
}

let raw = item_meta.raw()?;
let no_keywords = item_meta.inner()._bool("no_keywords")?;
let has_receiver = func
.sig()
.inputs
Expand Down Expand Up @@ -1336,6 +1337,7 @@ where
doc,
doc_body_pending,
raw,
no_keywords,
coexist,
attr_name: self.inner.attr_name,
});
Expand Down Expand Up @@ -1530,6 +1532,7 @@ struct MethodNurseryItem {
cfgs: Vec<Attribute>,
ident: Ident,
raw: bool,
no_keywords: bool,
coexist: bool,
doc: TokenStream,
doc_body_pending: TokenStream,
Expand Down Expand Up @@ -1561,6 +1564,11 @@ impl ToTokens for MethodNursery {
let cfgs = &item.cfgs;
let doc = &item.doc;
let doc_body_pending = &item.doc_body_pending;
let keyword_dispatch = if item.no_keywords {
quote!(rustpython_vm::function::KeywordDispatch::RejectNonempty)
} else {
quote!(rustpython_vm::function::KeywordDispatch::PassToBinder)
};
let binding_flags = match &item.attr_name {
AttrName::Method => {
quote! { rustpython_vm::function::PyMethodFlags::METHOD }
Expand Down Expand Up @@ -1603,7 +1611,7 @@ impl ToTokens for MethodNursery {
Self::#ident,
#flags,
#doc,
);
).with_keyword_dispatch(#keyword_dispatch);
::rustpython_vm::__cfg_doc!({
def.doc_body_pending = #doc_body_pending;
} else {});
Expand Down Expand Up @@ -1731,7 +1739,7 @@ impl ToTokens for GetSetNursery {
struct MethodItemMeta(ItemMetaInner);

impl ItemMeta for MethodItemMeta {
const ALLOWED_NAMES: &'static [&'static str] = &["name", "raw", "coexist"];
const ALLOWED_NAMES: &'static [&'static str] = &["name", "raw", "coexist", "no_keywords"];

fn from_inner(inner: ItemMetaInner) -> Self {
Self(inner)
Expand Down
30 changes: 26 additions & 4 deletions crates/derive-impl/src/pymodule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, internal_doc_tokens,
iter_use_idents, pyclass_ident_and_attrs,
ErrorVec, ItemMeta, ItemMetaInner, ItemNursery, ModuleItemMeta, SimpleItemMeta,
internal_doc_tokens, iter_use_idents, pyclass_ident_and_attrs,
};
use core::str::FromStr;
use proc_macro2::{Delimiter, Group, TokenStream, TokenTree};
Expand Down Expand Up @@ -536,6 +536,7 @@ struct FunctionNurseryItem {
ident: Ident,
/// One internal doc per [`py_names`](Self::py_names) entry.
docs: Vec<TokenStream>,
no_keywords: bool,
}

impl FunctionNursery {
Expand Down Expand Up @@ -565,6 +566,11 @@ impl ToTokens for ValidatedFunctionNursery {
let ident = &item.ident;
let cfgs = &item.cfgs;
let cfgs = quote!(#(#cfgs)*);
let keyword_dispatch = if item.no_keywords {
quote!(rustpython_vm::function::KeywordDispatch::RejectNonempty)
} else {
quote!(rustpython_vm::function::KeywordDispatch::PassToBinder)
};
for (py_name, doc) in item.py_names.iter().zip(&item.docs) {
inner_tokens.extend(quote![
#cfgs
Expand All @@ -573,7 +579,7 @@ impl ToTokens for ValidatedFunctionNursery {
#ident,
rustpython_vm::function::PyMethodFlags::empty(),
#doc,
),
).with_keyword_dispatch(#keyword_dispatch),
]);
}
}
Expand Down Expand Up @@ -670,7 +676,8 @@ impl ModuleItem for FunctionItem {
let ident = &func.sig().ident;

let item_attr = args.attrs.remove(self.index());
let item_meta = SimpleItemMeta::from_attr(ident.clone(), &item_attr)?;
let item_meta = FunctionItemMeta::from_attr(ident.clone(), &item_attr)?;
let no_keywords = item_meta.inner()._bool("no_keywords")?;

let py_name = item_meta.simple_name()?;
let mut py_names = vec![py_name];
Expand Down Expand Up @@ -715,6 +722,7 @@ impl ModuleItem for FunctionItem {
py_names,
cfgs: args.cfgs.to_vec(),
docs,
no_keywords,
});
Ok(())
}
Expand Down Expand Up @@ -1082,3 +1090,17 @@ impl ModuleItem for AttributeItem {
Ok(())
}
}

struct FunctionItemMeta(ItemMetaInner);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you still need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FunctionItemMeta was introduced to support the no_keywords marker. Now that the marker is gone, it is equivalent to SimpleItemMeta, so I’ll remove it.


impl ItemMeta for FunctionItemMeta {
const ALLOWED_NAMES: &'static [&'static str] = &["name", "no_keywords"];

fn from_inner(inner: ItemMetaInner) -> Self {
Self(inner)
}

fn inner(&self) -> &ItemMetaInner {
&self.0
}
}
38 changes: 37 additions & 1 deletion crates/vm/src/builtins/builtin_func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ use crate::{
class::PyClassImpl,
common::wtf8::Wtf8,
convert::TryFromObject,
function::{Callee, FuncArgs, PyComparisonValue, PyMethodDef, PyMethodFlags, PyNativeFn},
function::{
Callee, FuncArgs, KeywordDispatch, PyComparisonValue, PyMethodDef, PyMethodFlags,
PyNativeFn,
},
types::{Callable, Comparable, PyComparisonOp, Representable},
};
use alloc::fmt;
Expand Down Expand Up @@ -87,6 +90,9 @@ impl Callable for PyNativeFunction {
type Args = FuncArgs;
#[inline]
fn call(zelf: &Py<Self>, mut args: FuncArgs, vm: &VirtualMachine) -> PyResult {
if !args.kwargs.is_empty() && zelf.value.keyword_dispatch != KeywordDispatch::PassToBinder {
return Err(native_no_keywords_error(zelf, vm)?);
}
let mut callee = Callee::named(zelf.value.name);
if let Some(z) = &zelf.zelf {
// STATIC methods store the class in zelf for qualname/repr purposes,
Expand Down Expand Up @@ -272,6 +278,12 @@ fn vectorcall_native_function(
) -> PyResult {
let zelf: &Py<PyNativeFunction> = zelf_obj.downcast_ref().unwrap();

if kwnames.is_some_and(|names| !names.is_empty())
&& zelf.value.keyword_dispatch != KeywordDispatch::PassToBinder
{
return Err(native_no_keywords_error(zelf, vm)?);
}

// Build FuncArgs with self already at position 0 (no insert(0) needed)
let needs_self = zelf
.zelf
Expand All @@ -292,6 +304,30 @@ fn vectorcall_native_function(
(zelf.value.func)(vm, func_args, callee)
}

#[cold]
fn native_no_keywords_error(
zelf: &Py<PyNativeFunction>,
vm: &VirtualMachine,
) -> PyResult<crate::exceptions::types::PyBaseExceptionRef> {
if zelf.value.keyword_dispatch == KeywordDispatch::RejectNonemptyUnqualified {
return Ok(vm.new_type_error(format!("{}() takes no keyword arguments", zelf.value.name)));
}
let qualname = PyNativeFunction::__qualname__(NativeFunctionOrMethod(zelf.to_owned()), vm)?;
let module = zelf
.module
.load_owned()
.filter(|module| !vm.is_none(module));
let module = module.map(|module| module.str(vm)).transpose()?;
let name = if let Some(module) = module
&& module.to_string() != "builtins"
{
format!("{module}.{qualname}")
} else {
qualname.to_string()
};
Ok(vm.new_type_error(format!("{name}() takes no keyword arguments")))
}

pub(crate) fn init(context: &'static Context) {
PyNativeFunction::extend_class(context, context.types.builtin_function_or_method_type);
context
Expand Down
Loading