Merge pull request #1440 from davidhewitt/fix-multiple-kw-only-arg

pyfunction: refactor argument extraction
This commit is contained in:
Yuji Kanagawa 2021-03-04 00:15:34 +09:00 committed by GitHub
commit 21b26fcf3a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 606 additions and 180 deletions

View File

@ -33,6 +33,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- `PYO3_CROSS_LIB_DIR` enviroment variable no long required when compiling for x86-64 Python from macOS arm64 and reverse. [#1428](https://github.com/PyO3/pyo3/pull/1428)
- Fix FFI definition `_PyEval_RequestCodeExtraIndex` which took an argument of the wrong type. [#1429](https://github.com/PyO3/pyo3/pull/1429)
- Fix FFI definition `PyIndex_Check` missing with the `abi3` feature. [#1436](https://github.com/PyO3/pyo3/pull/1436)
- Fix incorrect `TypeError` raised when keyword-only argument passed along with a positional argument in `*args`. [#1440](https://github.com/PyO3/pyo3/pull/1440)
- Fix inability to use a named lifetime for `&PyTuple` of `*args` in `#[pyfunction]`. [#1440](https://github.com/PyO3/pyo3/pull/1440)
## [0.13.2] - 2021-02-12
### Packaging

View File

@ -0,0 +1,16 @@
[package]
authors = ["PyO3 Authors"]
name = "pyo3-benchmarks"
version = "0.1.0"
description = "Python-based benchmarks for various PyO3 functionality"
edition = "2018"
[dependencies]
[dependencies.pyo3]
path = "../../"
features = ["extension-module"]
[lib]
name = "_pyo3_benchmarks"
crate-type = ["cdylib"]

View File

@ -0,0 +1,2 @@
include pyproject.toml Cargo.toml
recursive-include src *

View File

@ -0,0 +1,17 @@
# rustapi_module
A simple extension module built using PyO3.
## Build
```shell
python setup.py install
```
## Testing
To test install tox globally and run
```shell
tox -e py
```

View File

@ -0,0 +1 @@
from ._pyo3_benchmarks import *

View File

@ -0,0 +1,6 @@
pip>=19.1
hypothesis>=3.55
pytest>=3.5.0
setuptools-rust>=0.10.2
psutil>=5.6
pytest-benchmark~=3.2

View File

@ -0,0 +1,44 @@
import sys
import platform
from setuptools import setup
from setuptools_rust import RustExtension
def get_py_version_cfgs():
# For now each Cfg Py_3_X flag is interpreted as "at least 3.X"
version = sys.version_info[0:2]
py3_min = 6
out_cfg = []
for minor in range(py3_min, version[1] + 1):
out_cfg.append("--cfg=Py_3_%d" % minor)
if platform.python_implementation() == "PyPy":
out_cfg.append("--cfg=PyPy")
return out_cfg
setup(
name="pyo3-benchmarks",
version="0.1.0",
classifiers=[
"License :: OSI Approved :: MIT License",
"Development Status :: 3 - Alpha",
"Intended Audience :: Developers",
"Programming Language :: Python",
"Programming Language :: Rust",
"Operating System :: POSIX",
"Operating System :: MacOS :: MacOS X",
],
packages=["pyo3_benchmarks"],
rust_extensions=[
RustExtension(
"pyo3_benchmarks._pyo3_benchmarks",
rustc_flags=get_py_version_cfgs(),
debug=False,
),
],
include_package_data=True,
zip_safe=False,
)

View File

@ -0,0 +1,33 @@
use pyo3::prelude::*;
use pyo3::types::{PyDict, PyTuple};
use pyo3::wrap_pyfunction;
#[pyfunction(args = "*", kwargs = "**")]
fn args_and_kwargs<'a>(
args: &'a PyTuple,
kwargs: Option<&'a PyDict>,
) -> (&'a PyTuple, Option<&'a PyDict>) {
(args, kwargs)
}
#[pyfunction(a, b = 2, args = "*", c = 4, kwargs = "**")]
fn mixed_args<'a>(
a: i32,
b: i32,
args: &'a PyTuple,
c: i32,
kwargs: Option<&'a PyDict>,
) -> (i32, i32, &'a PyTuple, i32, Option<&'a PyDict>) {
(a, b, args, c, kwargs)
}
#[pyfunction]
fn no_args() {}
#[pymodule]
fn _pyo3_benchmarks(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_function(wrap_pyfunction!(args_and_kwargs, m)?)?;
m.add_function(wrap_pyfunction!(mixed_args, m)?)?;
m.add_function(wrap_pyfunction!(no_args, m)?)?;
Ok(())
}

View File

@ -0,0 +1,46 @@
import pyo3_benchmarks
def test_args_and_kwargs(benchmark):
benchmark(pyo3_benchmarks.args_and_kwargs, 1, 2, 3, a=4, foo=10)
def args_and_kwargs_py(*args, **kwargs):
return (args, kwargs)
def test_args_and_kwargs_py(benchmark):
rust = pyo3_benchmarks.args_and_kwargs(1, 2, 3, bar=4, foo=10)
py = args_and_kwargs_py(1, 2, 3, bar=4, foo=10)
assert rust == py
benchmark(args_and_kwargs_py, 1, 2, 3, bar=4, foo=10)
def test_mixed_args(benchmark):
benchmark(pyo3_benchmarks.mixed_args, 1, 2, 3, bar=4, foo=10)
def mixed_args_py(a, b=2, *args, c=4, **kwargs):
return (a, b, args, c, kwargs)
def test_mixed_args_py(benchmark):
rust = pyo3_benchmarks.mixed_args(1, 2, 3, bar=4, foo=10)
py = mixed_args_py(1, 2, 3, bar=4, foo=10)
assert rust == py
benchmark(mixed_args_py, 1, 2, 3, bar=4, foo=10)
def test_no_args(benchmark):
benchmark(pyo3_benchmarks.no_args)
def no_args_py():
return None
def test_no_args_py(benchmark):
rust = pyo3_benchmarks.no_args()
py = no_args_py()
assert rust == py
benchmark(no_args_py)

View File

@ -0,0 +1,10 @@
[tox]
# can't install from sdist because local pyo3 repo can't be included in the sdist
skipsdist = true
[testenv]
description = Run the unit tests under {basepython}
deps = -rrequirements-dev.txt
commands =
python setup.py install
pytest {posargs}

View File

@ -143,12 +143,11 @@ def test_time_fold(fold):
assert t.fold == fold
@pytest.mark.xfail
@pytest.mark.parametrize(
"args", [(-1, 0, 0, 0), (0, -1, 0, 0), (0, 0, -1, 0), (0, 0, 0, -1)]
)
def test_invalid_time_fails_xfail(args):
with pytest.raises(ValueError):
def test_invalid_time_fails_overflow(args):
with pytest.raises(OverflowError):
rdt.make_time(*args)

View File

@ -8,7 +8,7 @@ use crate::pymethod::get_arg_names;
use crate::utils;
use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote};
use syn::{spanned::Spanned, Ident};
use syn::{spanned::Spanned, Ident, Result};
/// Generates the function that is called by the python interpreter to initialize the native
/// module
@ -197,7 +197,7 @@ pub fn add_fn_to_module(
let name = &func.sig.ident;
let wrapper_ident = format_ident!("__pyo3_raw_{}", name);
let wrapper = function_c_wrapper(name, &wrapper_ident, &spec, pyfn_attrs.pass_module);
let wrapper = function_c_wrapper(name, &wrapper_ident, &spec, pyfn_attrs.pass_module)?;
Ok(quote! {
#wrapper
pub(crate) fn #function_wrapper_ident<'a>(
@ -223,7 +223,7 @@ fn function_c_wrapper(
wrapper_ident: &Ident,
spec: &method::FnSpec<'_>,
pass_module: bool,
) -> TokenStream {
) -> Result<TokenStream> {
let names: Vec<Ident> = get_arg_names(&spec);
let cb;
let slf_module;
@ -240,8 +240,8 @@ fn function_c_wrapper(
};
slf_module = None;
};
let body = pymethod::impl_arg_params(spec, None, cb);
quote! {
let body = pymethod::impl_arg_params(spec, None, cb)?;
Ok(quote! {
unsafe extern "C" fn #wrapper_ident(
_slf: *mut pyo3::ffi::PyObject,
_args: *mut pyo3::ffi::PyObject,
@ -256,5 +256,5 @@ fn function_c_wrapper(
#body
})
}
}
})
}

View File

@ -4,8 +4,8 @@ use crate::konst::ConstSpec;
use crate::method::{FnArg, FnSpec, FnType, SelfType};
use crate::utils;
use proc_macro2::{Span, TokenStream};
use quote::quote;
use syn::{ext::IdentExt, spanned::Spanned};
use quote::{quote, quote_spanned};
use syn::{ext::IdentExt, spanned::Spanned, Result};
pub enum PropertyType<'a> {
Descriptor(&'a syn::Field),
@ -22,29 +22,29 @@ pub fn gen_py_method(
cls: &syn::Type,
sig: &mut syn::Signature,
meth_attrs: &mut Vec<syn::Attribute>,
) -> syn::Result<GeneratedPyMethod> {
) -> Result<GeneratedPyMethod> {
check_generic(sig)?;
let spec = FnSpec::parse(sig, &mut *meth_attrs, true)?;
Ok(match &spec.tp {
FnType::Fn(self_ty) => GeneratedPyMethod::Method(impl_py_method_def(
&spec,
&impl_wrap(cls, &spec, self_ty, true),
&impl_wrap(cls, &spec, self_ty, true)?,
)),
FnType::FnNew => {
GeneratedPyMethod::New(impl_py_method_def_new(cls, &impl_wrap_new(cls, &spec)))
GeneratedPyMethod::New(impl_py_method_def_new(cls, &impl_wrap_new(cls, &spec)?))
}
FnType::FnCall(self_ty) => GeneratedPyMethod::Call(impl_py_method_def_call(
cls,
&impl_wrap(cls, &spec, self_ty, false),
&impl_wrap(cls, &spec, self_ty, false)?,
)),
FnType::FnClass => GeneratedPyMethod::Method(impl_py_method_def_class(
&spec,
&impl_wrap_class(cls, &spec),
&impl_wrap_class(cls, &spec)?,
)),
FnType::FnStatic => GeneratedPyMethod::Method(impl_py_method_def_static(
&spec,
&impl_wrap_static(cls, &spec),
&impl_wrap_static(cls, &spec)?,
)),
FnType::ClassAttribute => GeneratedPyMethod::Method(impl_py_method_class_attribute(
&spec,
@ -98,7 +98,7 @@ pub fn impl_wrap(
spec: &FnSpec<'_>,
self_ty: &SelfType,
noargs: bool,
) -> TokenStream {
) -> Result<TokenStream> {
let body = impl_call(cls, &spec);
let slf = self_ty.receiver(cls);
impl_wrap_common(cls, spec, noargs, slf, body)
@ -110,10 +110,10 @@ fn impl_wrap_common(
noargs: bool,
slf: TokenStream,
body: TokenStream,
) -> TokenStream {
) -> Result<TokenStream> {
let python_name = &spec.python_name;
if spec.args.is_empty() && noargs {
quote! {
Ok(quote! {
unsafe extern "C" fn __wrap(
_slf: *mut pyo3::ffi::PyObject,
_args: *mut pyo3::ffi::PyObject,
@ -126,11 +126,10 @@ fn impl_wrap_common(
#body
})
}
}
})
} else {
let body = impl_arg_params(&spec, Some(cls), body);
quote! {
let body = impl_arg_params(&spec, Some(cls), body)?;
Ok(quote! {
unsafe extern "C" fn __wrap(
_slf: *mut pyo3::ffi::PyObject,
_args: *mut pyo3::ffi::PyObject,
@ -146,18 +145,22 @@ fn impl_wrap_common(
#body
})
}
}
})
}
}
/// Generate function wrapper for protocol method (PyCFunction, PyCFunctionWithKeywords)
pub fn impl_proto_wrap(cls: &syn::Type, spec: &FnSpec<'_>, self_ty: &SelfType) -> TokenStream {
pub fn impl_proto_wrap(
cls: &syn::Type,
spec: &FnSpec<'_>,
self_ty: &SelfType,
) -> Result<TokenStream> {
let python_name = &spec.python_name;
let cb = impl_call(cls, &spec);
let body = impl_arg_params(&spec, Some(cls), cb);
let body = impl_arg_params(&spec, Some(cls), cb)?;
let slf = self_ty.receiver(cls);
quote! {
Ok(quote! {
#[allow(unused_mut)]
unsafe extern "C" fn __wrap(
_slf: *mut pyo3::ffi::PyObject,
@ -173,27 +176,25 @@ pub fn impl_proto_wrap(cls: &syn::Type, spec: &FnSpec<'_>, self_ty: &SelfType) -
#body
})
}
}
})
}
/// Generate class method wrapper (PyCFunction, PyCFunctionWithKeywords)
pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result<TokenStream> {
let name = &spec.name;
let python_name = &spec.python_name;
let names: Vec<syn::Ident> = get_arg_names(&spec);
let cb = quote! { #cls::#name(#(#names),*) };
let body = impl_arg_params(spec, Some(cls), cb);
let body = impl_arg_params(spec, Some(cls), cb)?;
quote! {
Ok(quote! {
#[allow(unused_mut)]
unsafe extern "C" fn __wrap(
subtype: *mut pyo3::ffi::PyTypeObject,
_args: *mut pyo3::ffi::PyObject,
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
{
use pyo3::type_object::PyTypeInfo;
use pyo3::callback::IntoPyCallbackOutput;
use std::convert::TryFrom;
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
pyo3::callback::handle_panic(|_py| {
@ -205,19 +206,19 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
Ok(cell as *mut pyo3::ffi::PyObject)
})
}
}
})
}
/// Generate class method wrapper (PyCFunction, PyCFunctionWithKeywords)
pub fn impl_wrap_class(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
pub fn impl_wrap_class(cls: &syn::Type, spec: &FnSpec<'_>) -> Result<TokenStream> {
let name = &spec.name;
let python_name = &spec.python_name;
let names: Vec<syn::Ident> = get_arg_names(&spec);
let cb = quote! { pyo3::callback::convert(_py, #cls::#name(&_cls, #(#names),*)) };
let body = impl_arg_params(spec, Some(cls), cb);
let body = impl_arg_params(spec, Some(cls), cb)?;
quote! {
Ok(quote! {
#[allow(unused_mut)]
unsafe extern "C" fn __wrap(
_cls: *mut pyo3::ffi::PyObject,
@ -233,19 +234,19 @@ pub fn impl_wrap_class(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
#body
})
}
}
})
}
/// Generate static method wrapper (PyCFunction, PyCFunctionWithKeywords)
pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> Result<TokenStream> {
let name = &spec.name;
let python_name = &spec.python_name;
let names: Vec<syn::Ident> = get_arg_names(&spec);
let cb = quote! { pyo3::callback::convert(_py, #cls::#name(#(#names),*)) };
let body = impl_arg_params(spec, Some(cls), cb);
let body = impl_arg_params(spec, Some(cls), cb)?;
quote! {
Ok(quote! {
#[allow(unused_mut)]
unsafe extern "C" fn __wrap(
_slf: *mut pyo3::ffi::PyObject,
@ -260,7 +261,7 @@ pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
#body
})
}
}
})
}
/// Generate a wrapper for initialization of a class attribute from a method
@ -399,14 +400,14 @@ pub fn impl_arg_params(
spec: &FnSpec<'_>,
self_: Option<&syn::Type>,
body: TokenStream,
) -> TokenStream {
) -> Result<TokenStream> {
if spec.args.is_empty() {
return quote! {
#body
};
return Ok(body);
}
let mut params = Vec::new();
let mut positional_parameter_names = Vec::new();
let mut required_positional_parameters = 0usize;
let mut keyword_only_parameters = Vec::new();
for arg in spec.args.iter() {
if arg.py || spec.is_args(&arg.name) || spec.is_kwargs(&arg.name) {
@ -414,21 +415,29 @@ pub fn impl_arg_params(
}
let name = arg.name.unraw().to_string();
let kwonly = spec.is_kw_only(&arg.name);
let opt = arg.optional.is_some() || spec.default_value(&arg.name).is_some();
let required = !(arg.optional.is_some() || spec.default_value(&arg.name).is_some());
params.push(quote! {
pyo3::derive_utils::ParamDescription {
name: #name,
is_optional: #opt,
kw_only: #kwonly
if kwonly {
keyword_only_parameters.push(quote! {
pyo3::derive_utils::KeywordOnlyParameterDescription {
name: #name,
required: #required,
}
});
} else {
if required {
required_positional_parameters += 1;
}
});
positional_parameter_names.push(name);
}
}
let num_params = positional_parameter_names.len() + keyword_only_parameters.len();
let mut param_conversion = Vec::new();
let mut option_pos = 0;
for (idx, arg) in spec.args.iter().enumerate() {
param_conversion.push(impl_arg_param(&arg, &spec, idx, self_, &mut option_pos));
param_conversion.push(impl_arg_param(&arg, &spec, idx, self_, &mut option_pos)?);
}
let (mut accept_args, mut accept_kwargs) = (false, false);
@ -441,31 +450,29 @@ pub fn impl_arg_params(
_ => continue,
}
}
let num_normal_params = params.len();
// create array of arguments, and then parse
quote! {{
const PARAMS: &'static [pyo3::derive_utils::ParamDescription] = &[
#(#params),*
];
Ok(quote! {
{
const DESCRIPTION: pyo3::derive_utils::FunctionDescription = pyo3::derive_utils::FunctionDescription {
fname: _LOCATION,
positional_parameter_names: &[#(#positional_parameter_names),*],
// TODO: https://github.com/PyO3/pyo3/issues/1439 - support specifying these
positional_only_parameters: 0,
required_positional_parameters: #required_positional_parameters,
keyword_only_parameters: &[#(#keyword_only_parameters),*],
accept_varargs: #accept_args,
accept_varkeywords: #accept_kwargs,
};
let mut output = [None; #num_normal_params];
let mut _args = _args;
let mut _kwargs = _kwargs;
let mut output = [None; #num_params];
let (_args, _kwargs) = DESCRIPTION.extract_arguments(_args, _kwargs, &mut output)?;
let (_args, _kwargs) = pyo3::derive_utils::parse_fn_args(
Some(_LOCATION),
PARAMS,
_args,
_kwargs,
#accept_args,
#accept_kwargs,
&mut output
)?;
#(#param_conversion)*
#(#param_conversion)*
#body
}}
#body
}
})
}
/// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument
@ -476,13 +483,14 @@ fn impl_arg_param(
idx: usize,
self_: Option<&syn::Type>,
option_pos: &mut usize,
) -> TokenStream {
) -> Result<TokenStream> {
let arg_name = syn::Ident::new(&format!("arg{}", idx), Span::call_site());
if arg.py {
return quote! {
return Ok(quote_spanned! {
arg.ty.span() =>
let #arg_name = _py;
};
});
}
let ty = arg.ty;
@ -492,15 +500,28 @@ fn impl_arg_param(
};
if spec.is_args(&name) {
return quote! {
let #arg_name = <#ty as pyo3::FromPyObject>::extract(_args.as_ref())
ensure_spanned!(
arg.optional.is_none(),
arg.name.span() => "args cannot be optional"
);
return Ok(quote_spanned! {
arg.ty.span() =>
let #arg_name = _args.unwrap().extract()
.map_err(#transform_error)?;
};
});
} else if spec.is_kwargs(&name) {
return quote! {
let #arg_name = _kwargs;
};
ensure_spanned!(
arg.optional.is_some(),
arg.name.span() => "kwargs must be Option<_>"
);
return Ok(quote_spanned! {
arg.ty.span() =>
let #arg_name = _kwargs.map(|kwargs| kwargs.extract())
.transpose()
.map_err(#transform_error)?;
});
}
let arg_value = quote!(output[#option_pos]);
*option_pos += 1;
@ -537,20 +558,22 @@ fn impl_arg_param(
)
};
quote! {
Ok(quote_spanned! {
arg.ty.span() =>
let #mut_ _tmp: #target_ty = match #arg_value {
Some(_obj) => #extract,
None => #default,
};
let #arg_name = #borrow_tmp;
}
})
} else {
quote! {
Ok(quote_spanned! {
arg.ty.span() =>
let #arg_name = match #arg_value {
Some(_obj) => #extract,
None => #default,
};
}
})
};
/// Replace `Self`, remove lifetime and get mutability from the type

View File

@ -65,7 +65,7 @@ fn impl_proto_impl(
let fn_spec = FnSpec::parse(&mut met.sig, &mut met.attrs, false)?;
let method = if let FnType::Fn(self_ty) = &fn_spec.tp {
pymethod::impl_proto_wrap(ty, &fn_spec, &self_ty)
pymethod::impl_proto_wrap(ty, &fn_spec, &self_ty)?
} else {
bail_spanned!(
met.sig.span() => "expected method with receiver for #[pyproto] method"

View File

@ -10,106 +10,252 @@ use crate::exceptions::PyTypeError;
use crate::instance::PyNativeType;
use crate::pyclass::PyClass;
use crate::types::{PyAny, PyDict, PyModule, PyString, PyTuple};
use crate::{ffi, GILPool, IntoPy, PyCell, Python};
use crate::{ffi, GILPool, PyCell, Python};
use std::cell::UnsafeCell;
/// Description of a python parameter; used for `parse_args()`.
#[derive(Debug)]
pub struct ParamDescription {
/// The name of the parameter.
pub struct KeywordOnlyParameterDescription {
pub name: &'static str,
/// Whether the parameter is optional.
pub is_optional: bool,
/// Whether the parameter is optional.
pub kw_only: bool,
pub required: bool,
}
/// Parse argument list
///
/// * fname: Name of the current function
/// * params: Declared parameters of the function
/// * args: Positional arguments
/// * kwargs: Keyword arguments
/// * output: Output array that receives the arguments.
/// Must have same length as `params` and must be initialized to `None`.
pub fn parse_fn_args<'p>(
fname: Option<&str>,
params: &[ParamDescription],
args: &'p PyTuple,
kwargs: Option<&'p PyDict>,
accept_args: bool,
accept_kwargs: bool,
output: &mut [Option<&'p PyAny>],
) -> PyResult<(&'p PyTuple, Option<&'p PyDict>)> {
let nargs = args.len();
let mut used_args = 0;
macro_rules! raise_error {
($s: expr $(,$arg:expr)*) => (return Err(PyTypeError::new_err(format!(
concat!("{} ", $s), fname.unwrap_or("function") $(,$arg)*
))))
}
// Copy kwargs not to modify it
let kwargs = match kwargs {
Some(k) => Some(k.copy()?),
None => None,
};
// Iterate through the parameters and assign values to output:
for (i, (p, out)) in params.iter().zip(output).enumerate() {
*out = match kwargs.and_then(|d| d.get_item(p.name)) {
Some(kwarg) => {
if i < nargs {
raise_error!("got multiple values for argument: {}", p.name)
}
kwargs.as_ref().unwrap().del_item(p.name)?;
Some(kwarg)
/// Function argument specification for a #[pyfunction] or #[pymethod].
#[derive(Debug)]
pub struct FunctionDescription {
pub fname: &'static str,
pub positional_parameter_names: &'static [&'static str],
pub positional_only_parameters: usize,
pub required_positional_parameters: usize,
pub keyword_only_parameters: &'static [KeywordOnlyParameterDescription],
pub accept_varargs: bool,
pub accept_varkeywords: bool,
}
impl FunctionDescription {
/// Extracts the `args` and `kwargs` provided into `output`, according to this function
/// definition.
///
/// `output` must have the same length as this function has positional and keyword-only
/// parameters (as per the `positional_parameter_names` and `keyword_only_parameters`
/// respectively).
///
/// If `accept_varargs` or `accept_varkeywords`, then the returned `&PyTuple` and `&PyDict` may
/// be `Some` if there are extra arguments.
///
/// Unexpected, duplicate or invalid arguments will cause this function to return `TypeError`.
pub fn extract_arguments<'p>(
&self,
args: &'p PyTuple,
kwargs: Option<&'p PyDict>,
output: &mut [Option<&'p PyAny>],
) -> PyResult<(Option<&'p PyTuple>, Option<&'p PyDict>)> {
let num_positional_parameters = self.positional_parameter_names.len();
debug_assert!(self.positional_only_parameters <= num_positional_parameters);
debug_assert!(self.required_positional_parameters <= num_positional_parameters);
debug_assert_eq!(
output.len(),
num_positional_parameters + self.keyword_only_parameters.len()
);
// Handle positional arguments
let (args_provided, varargs) = {
let args_provided = args.len();
if self.accept_varargs {
(
std::cmp::min(num_positional_parameters, args_provided),
Some(args.slice(num_positional_parameters as isize, args_provided as isize)),
)
} else if args_provided > num_positional_parameters {
return Err(self.too_many_positional_arguments(args_provided));
} else {
(args_provided, None)
}
None => {
if p.kw_only {
if !p.is_optional {
raise_error!("missing required keyword-only argument: {}", p.name)
}
None
} else if i < nargs {
used_args += 1;
Some(args.get_item(i))
} else {
if !p.is_optional {
raise_error!("missing required positional argument: {}", p.name)
}
None
}
};
// Copy positional arguments into output
for (out, arg) in output[..args_provided].iter_mut().zip(args) {
*out = Some(arg);
}
// Handle keyword arguments
let varkeywords = match (kwargs, self.accept_varkeywords) {
(Some(kwargs), true) => {
let mut varkeywords = None;
self.extract_keyword_arguments(kwargs, output, |name, value| {
varkeywords
.get_or_insert_with(|| PyDict::new(kwargs.py()))
.set_item(name, value)
})?;
varkeywords
}
(Some(kwargs), false) => {
self.extract_keyword_arguments(kwargs, output, |name, _| {
Err(self.unexpected_keyword_argument(name))
})?;
None
}
(None, _) => None,
};
// Check that there's sufficient positional arguments once keyword arguments are specified
if args_provided < self.required_positional_parameters {
let missing_positional_arguments: Vec<_> = self
.positional_parameter_names
.iter()
.take(self.required_positional_parameters)
.zip(output.iter())
.filter_map(|(param, out)| if out.is_none() { Some(*param) } else { None })
.collect();
if !missing_positional_arguments.is_empty() {
return Err(
self.missing_required_arguments("positional", &missing_positional_arguments)
);
}
}
// Check no missing required keyword arguments
let missing_keyword_only_arguments: Vec<_> = self
.keyword_only_parameters
.iter()
.zip(&output[num_positional_parameters..])
.filter_map(|(keyword_desc, out)| {
if keyword_desc.required && out.is_none() {
Some(keyword_desc.name)
} else {
None
}
})
.collect();
if !missing_keyword_only_arguments.is_empty() {
return Err(self.missing_required_arguments("keyword", &missing_keyword_only_arguments));
}
Ok((varargs, varkeywords))
}
let is_kwargs_empty = kwargs.as_ref().map_or(true, |dict| dict.is_empty());
// Raise an error when we get an unknown key
if !accept_kwargs && !is_kwargs_empty {
let (key, _) = kwargs.unwrap().iter().next().unwrap();
raise_error!("got an unexpected keyword argument: {}", key)
#[inline]
fn extract_keyword_arguments<'p>(
&self,
kwargs: &'p PyDict,
output: &mut [Option<&'p PyAny>],
mut unexpected_keyword_handler: impl FnMut(&'p PyAny, &'p PyAny) -> PyResult<()>,
) -> PyResult<()> {
let (args_output, kwargs_output) =
output.split_at_mut(self.positional_parameter_names.len());
let mut positional_only_keyword_arguments = Vec::new();
for (kwarg_name, value) in kwargs {
let utf8_string = match kwarg_name.downcast::<PyString>()?.to_str() {
Ok(utf8_string) => utf8_string,
// This keyword is not a UTF8 string: all PyO3 argument names are guaranteed to be
// UTF8 by construction.
Err(_) => {
unexpected_keyword_handler(kwarg_name, value)?;
continue;
}
};
// Compare the keyword name against each parameter in turn. This is exactly the same method
// which CPython uses to map keyword names. Although it's O(num_parameters), the number of
// parameters is expected to be small so it's not worth constructing a mapping.
if let Some(i) = self
.keyword_only_parameters
.iter()
.position(|param| utf8_string == param.name)
{
kwargs_output[i] = Some(value);
continue;
}
// Repeat for positional parameters
if let Some((i, param)) = self
.positional_parameter_names
.iter()
.enumerate()
.find(|&(_, param)| utf8_string == *param)
{
if i < self.positional_only_parameters {
positional_only_keyword_arguments.push(*param);
} else if args_output[i].replace(value).is_some() {
return Err(self.multiple_values_for_argument(param));
}
continue;
}
unexpected_keyword_handler(kwarg_name, value)?;
}
if positional_only_keyword_arguments.is_empty() {
Ok(())
} else {
Err(self.positional_only_keyword_arguments(&positional_only_keyword_arguments))
}
}
// Raise an error when we get too many positional args
if !accept_args && used_args < nargs {
raise_error!(
"takes at most {} positional argument{} ({} given)",
used_args,
if used_args == 1 { "" } else { "s" },
nargs
)
fn too_many_positional_arguments(&self, args_provided: usize) -> PyErr {
let was = if args_provided == 1 { "was" } else { "were" };
let msg = if self.required_positional_parameters != self.positional_parameter_names.len() {
format!(
"{} takes from {} to {} positional arguments but {} {} given",
self.fname,
self.required_positional_parameters,
self.positional_parameter_names.len(),
args_provided,
was
)
} else {
format!(
"{} takes {} positional arguments but {} {} given",
self.fname,
self.positional_parameter_names.len(),
args_provided,
was
)
};
PyTypeError::new_err(msg)
}
fn multiple_values_for_argument(&self, argument: &str) -> PyErr {
PyTypeError::new_err(format!(
"{} got multiple values for argument '{}'",
self.fname, argument
))
}
fn unexpected_keyword_argument(&self, argument: &PyAny) -> PyErr {
PyTypeError::new_err(format!(
"{} got an unexpected keyword argument '{}'",
self.fname, argument
))
}
fn positional_only_keyword_arguments(&self, parameter_names: &[&str]) -> PyErr {
let mut msg = format!(
"{} got some positional-only arguments passed as keyword arguments: ",
self.fname
);
push_parameter_list(&mut msg, parameter_names);
PyTypeError::new_err(msg)
}
fn missing_required_arguments(&self, argument_type: &str, parameter_names: &[&str]) -> PyErr {
let arguments = if parameter_names.len() == 1 {
"argument"
} else {
"arguments"
};
let mut msg = format!(
"{} missing {} required {} {}: ",
self.fname,
parameter_names.len(),
argument_type,
arguments,
);
push_parameter_list(&mut msg, parameter_names);
PyTypeError::new_err(msg)
}
// Adjust the remaining args
let args = if accept_args {
let py = args.py();
let slice = args.slice(used_args as isize, nargs as isize).into_py(py);
py.checked_cast_as(slice).unwrap()
} else {
args
};
let kwargs = if accept_kwargs && is_kwargs_empty {
None
} else {
kwargs
};
Ok((args, kwargs))
}
/// Add the argument name to the error message of an error which occurred during argument extraction
@ -252,3 +398,63 @@ impl<'a> From<&'a PyModule> for PyFunctionArguments<'a> {
PyFunctionArguments::PyModule(module)
}
}
fn push_parameter_list(msg: &mut String, parameter_names: &[&str]) {
for (i, parameter) in parameter_names.iter().enumerate() {
if i != 0 {
if parameter_names.len() > 2 {
msg.push(',');
}
if i == parameter_names.len() - 1 {
msg.push_str(" and ")
} else {
msg.push(' ')
}
}
msg.push('\'');
msg.push_str(parameter);
msg.push('\'');
}
}
#[cfg(test)]
mod tests {
use super::push_parameter_list;
#[test]
fn push_parameter_list_empty() {
let mut s = String::new();
push_parameter_list(&mut s, &[]);
assert_eq!(&s, "");
}
#[test]
fn push_parameter_list_one() {
let mut s = String::new();
push_parameter_list(&mut s, &["a"]);
assert_eq!(&s, "'a'");
}
#[test]
fn push_parameter_list_two() {
let mut s = String::new();
push_parameter_list(&mut s, &["a", "b"]);
assert_eq!(&s, "'a' and 'b'");
}
#[test]
fn push_parameter_list_three() {
let mut s = String::new();
push_parameter_list(&mut s, &["a", "b", "c"]);
assert_eq!(&s, "'a', 'b', and 'c'");
}
#[test]
fn push_parameter_list_four() {
let mut s = String::new();
push_parameter_list(&mut s, &["a", "b", "c", "d"]);
assert_eq!(&s, "'a', 'b', 'c', and 'd'");
}
}

View File

@ -49,7 +49,6 @@ impl InstanceMethodWithArgs {
}
#[test]
#[allow(dead_code)]
fn instance_method_with_args() {
let gil = Python::acquire_gil();
let py = gil.python();
@ -245,6 +244,11 @@ impl MethArgs {
a + b
}
#[args(args = "*", a)]
fn get_args_and_required_keyword(&self, py: Python, args: &PyTuple, a: i32) -> PyObject {
(args, a).to_object(py)
}
#[args(a, b = 2, "*", c = 3)]
fn get_pos_arg_kw_sep1(&self, a: i32, b: i32, c: i32) -> i32 {
a + b + c
@ -365,6 +369,23 @@ fn meth_args() {
PyTypeError
);
py_run!(
py,
inst,
"assert inst.get_args_and_required_keyword(1, 2, a=3) == ((1, 2), 3)"
);
py_run!(
py,
inst,
"assert inst.get_args_and_required_keyword(a=1) == ((), 1)"
);
py_expect_exception!(
py,
inst,
"inst.get_args_and_required_keyword()",
PyTypeError
);
py_run!(py, inst, "assert inst.get_pos_arg_kw_sep1(1) == 6");
py_run!(py, inst, "assert inst.get_pos_arg_kw_sep1(1, 2) == 6");
py_run!(