Skip to content

Commit

Permalink
Merge pull request #1661 from PyO3/optimize_forwarding
Browse files Browse the repository at this point in the history
Optimize f(*args, **kwds) function argument "parsing"
  • Loading branch information
birkenfeld authored Jun 6, 2021
2 parents 4d2c173 + bd6788b commit c57afeb
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 26 deletions.
27 changes: 6 additions & 21 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2017-present PyO3 Project and Contributors

use crate::attributes::TextSignatureAttribute;
use crate::params::impl_arg_params;
use crate::params::{impl_arg_params, is_forwarded_args};
use crate::pyfunction::PyFunctionOptions;
use crate::pyfunction::{PyFunctionArgPyO3Attributes, PyFunctionSignature};
use crate::utils;
Expand Down Expand Up @@ -170,9 +170,12 @@ impl CallingConvention {
///
/// Different other slots (tp_call, tp_new) can have other requirements
/// and are set manually (see `parse_fn_type` below).
pub fn from_args(args: &[FnArg<'_>]) -> Self {
pub fn from_args(args: &[FnArg<'_>], attrs: &[Argument]) -> Self {
if args.is_empty() {
Self::Noargs
} else if is_forwarded_args(args, attrs) {
// for f(*args, **kwds), always prefer varargs
Self::Varargs
} else if cfg!(all(Py_3_7, not(Py_LIMITED_API))) {
Self::Fastcall
} else {
Expand Down Expand Up @@ -291,7 +294,7 @@ impl<'a> FnSpec<'a> {
};

let convention =
fixed_convention.unwrap_or_else(|| CallingConvention::from_args(&arguments));
fixed_convention.unwrap_or_else(|| CallingConvention::from_args(&arguments, &fn_attrs));

Ok(FnSpec {
tp: fn_type,
Expand Down Expand Up @@ -411,24 +414,6 @@ impl<'a> FnSpec<'a> {
Ok((fn_type, skip_first_arg, fixed_convention))
}

pub fn is_args(&self, name: &syn::Ident) -> bool {
for s in self.attrs.iter() {
if let Argument::VarArgs(path) = s {
return path.is_ident(name);
}
}
false
}

pub fn is_kwargs(&self, name: &syn::Ident) -> bool {
for s in self.attrs.iter() {
if let Argument::KeywordArgs(path) = s {
return path.is_ident(name);
}
}
false
}

pub fn default_value(&self, name: &syn::Ident) -> Option<TokenStream> {
for s in self.attrs.iter() {
match s {
Expand Down
55 changes: 51 additions & 4 deletions pyo3-macros-backend/src/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,37 @@
use crate::{
attributes::FromPyWithAttribute,
method::{FnArg, FnSpec},
pyfunction::Argument,
};
use proc_macro2::{Span, TokenStream};
use quote::{quote, quote_spanned};
use syn::ext::IdentExt;
use syn::spanned::Spanned;
use syn::Result;

/// Return true if the argument list is simply (*args, **kwds).
pub fn is_forwarded_args(args: &[FnArg<'_>], attrs: &[Argument]) -> bool {
args.len() == 2 && is_args(attrs, &args[0].name) && is_kwargs(attrs, &args[1].name)
}

fn is_args(attrs: &[Argument], name: &syn::Ident) -> bool {
for s in attrs.iter() {
if let Argument::VarArgs(path) = s {
return path.is_ident(name);
}
}
false
}

fn is_kwargs(attrs: &[Argument], name: &syn::Ident) -> bool {
for s in attrs.iter() {
if let Argument::KeywordArgs(path) = s {
return path.is_ident(name);
}
}
false
}

pub fn impl_arg_params(
spec: &FnSpec<'_>,
self_: Option<&syn::Type>,
Expand All @@ -21,12 +45,36 @@ pub fn impl_arg_params(
return Ok(body);
}

let args_array = syn::Ident::new("output", Span::call_site());

if !fastcall && is_forwarded_args(&spec.args, &spec.attrs) {
// In the varargs convention, we can just pass though if the signature
// is (*args, **kwds).
let mut arg_convert = vec![];
for (i, arg) in spec.args.iter().enumerate() {
arg_convert.push(impl_arg_param(
arg,
&spec,
i,
None,
&mut 0,
py,
&args_array,
)?);
}
return Ok(quote! {{
let _args = Some(_args);
#(#arg_convert)*
#body
}});
};

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) {
if arg.py || is_args(&spec.attrs, &arg.name) || is_kwargs(&spec.attrs, &arg.name) {
continue;
}
let name = arg.name.unraw().to_string();
Expand All @@ -49,7 +97,6 @@ pub fn impl_arg_params(
}

let num_params = positional_parameter_names.len() + keyword_only_parameters.len();
let args_array = syn::Ident::new("output", Span::call_site());

let mut param_conversion = Vec::new();
let mut option_pos = 0;
Expand Down Expand Up @@ -149,15 +196,15 @@ fn impl_arg_param(
|e| pyo3::derive_utils::argument_extraction_error(#py, stringify!(#name), e)
};

if spec.is_args(&name) {
if is_args(&spec.attrs, &name) {
ensure_spanned!(
arg.optional.is_none(),
arg.name.span() => "args cannot be optional"
);
return Ok(quote_arg_span! {
let #arg_name = _args.unwrap().extract().map_err(#transform_error)?;
});
} else if spec.is_kwargs(&name) {
} else if is_kwargs(&spec.attrs, &name) {
ensure_spanned!(
arg.optional.is_some(),
arg.name.span() => "kwargs must be Option<_>"
Expand Down
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/pyfunction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ pub fn impl_wrap_pyfunction(
method::FnType::FnStatic
},
name: &func.sig.ident,
convention: CallingConvention::from_args(&arguments),
convention: CallingConvention::from_args(&arguments, &signature.arguments),
python_name,
attrs: signature.arguments,
args: arguments,
Expand Down

0 comments on commit c57afeb

Please sign in to comment.