diff --git a/CHANGELOG.md b/CHANGELOG.md index 43aa84578a1..6b3b569168d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `as_sequence` methods to `PyList` and `PyTuple`. [#1860](https://github.com/PyO3/pyo3/pull/1860) - Add `abi3-py310` feature. [#1889](https://github.com/PyO3/pyo3/pull/1889) - Add `PyCFunction::new_closure` to create a Python function from a Rust closure. [#1901](https://github.com/PyO3/pyo3/pull/1901) +- Add support for positional-only arguments in `#[pyfunction]` [#1925](https://github.com/PyO3/pyo3/pull/1925) ### Changed diff --git a/guide/src/class.md b/guide/src/class.md index 3551ad20a30..f3455c83f4a 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -691,6 +691,9 @@ the form of `attr_name="default value"`. Each parameter has to match the method Each parameter can be one of the following types: + * `"/"`: positional-only arguments separator, each parameter defined before `"/"` is a + positional-only parameter. + Corresponds to python's `def meth(arg1, arg2, ..., /, argN..)`. * `"*"`: var arguments separator, each parameter defined after `"*"` is a keyword-only parameter. Corresponds to python's `def meth(*, arg1.., arg2=..)`. * `args="*"`: "args" is var args, corresponds to Python's `def meth(*args)`. Type of the `args` @@ -745,7 +748,7 @@ impl MyClass { } } ``` -N.B. the position of the `"*"` argument (if included) controls the system of handling positional and keyword arguments. In Python: +N.B. the position of the `"/"` and `"*"` arguments (if included) control the system of handling positional and keyword arguments. In Python: ```python import mymodule diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index e05d67e41ea..6bfd0692fd3 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -449,6 +449,17 @@ impl<'a> FnSpec<'a> { None } + pub fn is_pos_only(&self, name: &syn::Ident) -> bool { + for s in self.attrs.iter() { + if let Argument::PosOnlyArg(path, _) = s { + if path.is_ident(name) { + return true; + } + } + } + false + } + pub fn is_kw_only(&self, name: &syn::Ident) -> bool { for s in self.attrs.iter() { if let Argument::Kwarg(path, _) = s { diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index 45259b77990..de39f1bce78 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -78,6 +78,7 @@ pub fn impl_arg_params( }; let mut positional_parameter_names = Vec::new(); + let mut positional_only_parameters = 0usize; let mut required_positional_parameters = 0usize; let mut keyword_only_parameters = Vec::new(); @@ -86,6 +87,7 @@ pub fn impl_arg_params( continue; } let name = arg.name.unraw().to_string(); + let posonly = spec.is_pos_only(arg.name); let kwonly = spec.is_kw_only(arg.name); let required = !(arg.optional.is_some() || spec.default_value(arg.name).is_some()); @@ -100,6 +102,9 @@ pub fn impl_arg_params( if required { required_positional_parameters += 1; } + if posonly { + positional_only_parameters += 1; + } positional_parameter_names.push(name); } } @@ -154,8 +159,7 @@ pub fn impl_arg_params( cls_name: #cls_name, func_name: stringify!(#python_name), positional_parameter_names: &[#(#positional_parameter_names),*], - // TODO: https://github.com/PyO3/pyo3/issues/1439 - support specifying these - positional_only_parameters: 0, + positional_only_parameters: #positional_only_parameters, required_positional_parameters: #required_positional_parameters, keyword_only_parameters: &[#(#keyword_only_parameters),*], accept_varargs: #accept_args, diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index 8e3e872f02d..890b7450509 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -22,9 +22,11 @@ use syn::{ #[derive(Debug, Clone, PartialEq)] pub enum Argument { + PosOnlyArgsSeparator, VarArgsSeparator, VarArgs(syn::Path), KeywordArgs(syn::Path), + PosOnlyArg(syn::Path, Option), Arg(syn::Path, Option), Kwarg(syn::Path, Option), } @@ -34,6 +36,7 @@ pub enum Argument { pub struct PyFunctionSignature { pub arguments: Vec, has_kw: bool, + has_posonly_args: bool, has_varargs: bool, has_kwargs: bool, } @@ -126,7 +129,22 @@ impl PyFunctionSignature { self.arguments.push(Argument::VarArgsSeparator); Ok(()) } - _ => bail_spanned!(item.span() => "expected \"*\""), + syn::Lit::Str(lits) if lits.value() == "/" => { + // "/" + self.posonly_arg_is_ok(item)?; + self.has_posonly_args = true; + // any arguments _before_ this become positional-only + self.arguments.iter_mut().for_each(|a| { + if let Argument::Arg(path, name) = a { + *a = Argument::PosOnlyArg(path.clone(), name.clone()); + } else { + unreachable!(); + } + }); + self.arguments.push(Argument::PosOnlyArgsSeparator); + Ok(()) + } + _ => bail_spanned!(item.span() => "expected \"/\" or \"*\""), } } @@ -143,6 +161,14 @@ impl PyFunctionSignature { Ok(()) } + fn posonly_arg_is_ok(&self, item: &NestedMeta) -> syn::Result<()> { + ensure_spanned!( + !(self.has_posonly_args || self.has_kwargs || self.has_varargs), + item.span() => "/ is not allowed after /, varargs(*), or kwargs(**)" + ); + Ok(()) + } + fn vararg_is_ok(&self, item: &NestedMeta) -> syn::Result<()> { ensure_spanned!( !(self.has_kwargs || self.has_varargs), diff --git a/tests/test_methods.rs b/tests/test_methods.rs index f238278091b..40afb4f3b53 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -215,6 +215,46 @@ impl MethArgs { [a.to_object(py), args.into(), kwargs.to_object(py)].to_object(py) } + #[args(a, b, "/")] + fn get_pos_only(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", b)] + fn get_pos_only_and_pos(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", b, c = 5)] + fn get_pos_only_and_pos_and_kw(&self, a: i32, b: i32, c: i32) -> i32 { + a + b + c + } + + #[args(a, "/", "*", b)] + fn get_pos_only_and_kw_only(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", "*", b = 3)] + fn get_pos_only_and_kw_only_with_default(&self, a: i32, b: i32) -> i32 { + a + b + } + + #[args(a, "/", b, "*", c, d = 5)] + fn get_all_arg_types_together(&self, a: i32, b: i32, c: i32, d: i32) -> i32 { + a + b + c + d + } + + #[args(a, "/", args = "*")] + fn get_pos_only_with_varargs(&self, a: i32, args: Vec) -> i32 { + a + args.iter().sum::() + } + + #[args(a, "/", kwargs = "**")] + fn get_pos_only_with_kwargs(&self, py: Python, a: i32, kwargs: Option<&PyDict>) -> PyObject { + [a.to_object(py), kwargs.to_object(py)].to_object(py) + } + #[args("*", a = 2, b = 3)] fn get_kwargs_only_with_defaults(&self, a: i32, b: i32) -> i32 { a + b @@ -308,6 +348,165 @@ fn meth_args() { py_expect_exception!(py, inst, "inst.get_pos_arg_kw(1, a=1)", PyTypeError); py_expect_exception!(py, inst, "inst.get_pos_arg_kw(b=2)", PyTypeError); + py_run!(py, inst, "assert inst.get_pos_only(10, 11) == 21"); + py_expect_exception!(py, inst, "inst.get_pos_only(10, b = 11)", PyTypeError); + py_expect_exception!(py, inst, "inst.get_pos_only(a = 10, b = 11)", PyTypeError); + + py_run!(py, inst, "assert inst.get_pos_only_and_pos(10, 11) == 21"); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos(10, b = 11) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_pos(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, 11) == 26" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, b = 11) == 26" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, 11, c = 0) == 21" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_pos_and_kw(10, b = 11, c = 0) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_pos_and_kw(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_and_kw_only(10, b = 11) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only(10, 11)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_and_kw_only_with_default(10) == 13" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_and_kw_only_with_default(10, b = 11) == 21" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only_with_default(10, 11)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_and_kw_only_with_default(a = 10, b = 11)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_all_arg_types_together(10, 10, c = 10) == 35" + ); + py_run!( + py, + inst, + "assert inst.get_all_arg_types_together(10, 10, c = 10, d = 10) == 40" + ); + py_run!( + py, + inst, + "assert inst.get_all_arg_types_together(10, b = 10, c = 10, d = 10) == 40" + ); + py_expect_exception!( + py, + inst, + "inst.get_all_arg_types_together(10, 10, 10)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_all_arg_types_together(a = 10, b = 10, c = 10)", + PyTypeError + ); + + py_run!(py, inst, "assert inst.get_pos_only_with_varargs(10) == 10"); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_varargs(10, 10) == 20" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_varargs(10, 10, 10, 10, 10) == 50" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_with_varargs(a = 10)", + PyTypeError + ); + + py_run!( + py, + inst, + "assert inst.get_pos_only_with_kwargs(10) == [10, None]" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_kwargs(10, b = 10) == [10, {'b': 10}]" + ); + py_run!( + py, + inst, + "assert inst.get_pos_only_with_kwargs(10, b = 10, c = 10, d = 10, e = 10) == [10, {'b': 10, 'c': 10, 'd': 10, 'e': 10}]" + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_with_kwargs(a = 10)", + PyTypeError + ); + py_expect_exception!( + py, + inst, + "inst.get_pos_only_with_kwargs(a = 10, b = 10)", + PyTypeError + ); + py_run!(py, inst, "assert inst.get_kwargs_only_with_defaults() == 5"); py_run!( py, diff --git a/tests/ui/invalid_macro_args.rs b/tests/ui/invalid_macro_args.rs index 241d35c6b26..677c0cfbfd5 100644 --- a/tests/ui/invalid_macro_args.rs +++ b/tests/ui/invalid_macro_args.rs @@ -10,4 +10,24 @@ fn kw_after_kwargs(py: Python, kwargs: &PyDict, a: i32) -> PyObject { [a.to_object(py), vararg.into()].to_object(py) } +#[pyfunction(a, "*", b, "/", c)] +fn pos_only_after_kw_only(py: Python, a: i32, b: i32, c: i32) -> i32 { + a + b + c +} + +#[pyfunction(a, args="*", "/", b)] +fn pos_only_after_args(py: Python, a: i32, args: Vec, b: i32) -> i32 { + a + b + c +} + +#[pyfunction(a, kwargs="**", "/", b)] +fn pos_only_after_kwargs(py: Python, a: i32, args: Vec, b: i32) -> i32 { + a + b +} + +#[pyfunction(kwargs = "**", "*", a)] +fn kw_only_after_kwargs(py: Python, kwargs: &PyDict, a: i32) -> PyObject { + [a.to_object(py), vararg.into()].to_object(py) +} + fn main() {} diff --git a/tests/ui/invalid_macro_args.stderr b/tests/ui/invalid_macro_args.stderr index b0965fc930b..a2bc0d945c5 100644 --- a/tests/ui/invalid_macro_args.stderr +++ b/tests/ui/invalid_macro_args.stderr @@ -9,3 +9,27 @@ error: keyword argument or kwargs(**) is not allowed after kwargs(**) | 8 | #[pyfunction(kwargs = "**", a = 5)] | ^ + +error: / is not allowed after /, varargs(*), or kwargs(**) + --> tests/ui/invalid_macro_args.rs:13:25 + | +13 | #[pyfunction(a, "*", b, "/", c)] + | ^^^ + +error: / is not allowed after /, varargs(*), or kwargs(**) + --> tests/ui/invalid_macro_args.rs:18:27 + | +18 | #[pyfunction(a, args="*", "/", b)] + | ^^^ + +error: / is not allowed after /, varargs(*), or kwargs(**) + --> tests/ui/invalid_macro_args.rs:23:30 + | +23 | #[pyfunction(a, kwargs="**", "/", b)] + | ^^^ + +error: * is not allowed after varargs(*) or kwargs(**) + --> tests/ui/invalid_macro_args.rs:28:29 + | +28 | #[pyfunction(kwargs = "**", "*", a)] + | ^^^