Skip to content

Commit d85a02d

Browse files
Icxoludavidhewitt
Icxolu
and
davidhewitt
authored
split PyFunctionArgument to specialize Option (#5002)
* split `PyFunctionArgument` to specialize `Option` Adds const generic parameter to `PyFunctionArgument` to allow specialization. This allows `Option` wrapped extraction of some types that can't implement `FromPyObject(Bound)` because they require a `holder` to borrow from, most notably `&T` for `T: PyClass`. Closes #4965 * tests and changelog * clippy --------- Co-authored-by: David Hewitt <mail@davidhewitt.dev>
1 parent c37a50a commit d85a02d

File tree

10 files changed

+194
-56
lines changed

10 files changed

+194
-56
lines changed

‎guide/src/class.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,7 +1390,7 @@ impl pyo3::PyClass for MyClass {
13901390
type Frozen = pyo3::pyclass::boolean_struct::False;
13911391
}
13921392

1393-
impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a MyClass
1393+
impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py, false> for &'a MyClass
13941394
{
13951395
type Holder = ::std::option::Option<pyo3::PyRef<'py, MyClass>>;
13961396

@@ -1400,7 +1400,7 @@ impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a
14001400
}
14011401
}
14021402

1403-
impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a mut MyClass
1403+
impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py, false> for &'a mut MyClass
14041404
{
14051405
type Holder = ::std::option::Option<pyo3::PyRefMut<'py, MyClass>>;
14061406

‎newsfragments/5002.fixed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix compile failure with required `#[pyfunction]` arguments taking `Option<&str>` and `Option<&T>` (for `#[pyclass]` types).

‎pyo3-macros-backend/src/params.rs

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::utils::{deprecated_from_py_with, Ctx};
1+
use crate::utils::{deprecated_from_py_with, Ctx, TypeExt as _};
22
use crate::{
33
attributes::FromPyWithAttribute,
44
method::{FnArg, FnSpec, RegularArg},
@@ -200,7 +200,7 @@ fn impl_arg_param(
200200
let holder = holders.push_holder(arg.name.span());
201201
let name_str = arg.name.to_string();
202202
quote_spanned! { arg.name.span() =>
203-
#pyo3_path::impl_::extract_argument::extract_argument(
203+
#pyo3_path::impl_::extract_argument::extract_argument::<_, false>(
204204
&_args,
205205
&mut #holder,
206206
#name_str
@@ -211,7 +211,7 @@ fn impl_arg_param(
211211
let holder = holders.push_holder(arg.name.span());
212212
let name_str = arg.name.to_string();
213213
quote_spanned! { arg.name.span() =>
214-
#pyo3_path::impl_::extract_argument::extract_optional_argument(
214+
#pyo3_path::impl_::extract_argument::extract_optional_argument::<_, false>(
215215
_kwargs.as_deref(),
216216
&mut #holder,
217217
#name_str,
@@ -238,8 +238,9 @@ pub(crate) fn impl_regular_arg_param(
238238

239239
// Use this macro inside this function, to ensure that all code generated here is associated
240240
// with the function argument
241+
let use_probe = quote!(use #pyo3_path::impl_::pyclass::Probe as _;);
241242
macro_rules! quote_arg_span {
242-
($($tokens:tt)*) => { quote_spanned!(arg.ty.span() => $($tokens)*) }
243+
($($tokens:tt)*) => { quote_spanned!(arg.ty.span() => { #use_probe $($tokens)* }) }
243244
}
244245

245246
let name_str = arg.name.to_string();
@@ -251,6 +252,7 @@ pub(crate) fn impl_regular_arg_param(
251252
default = default.map(|tokens| some_wrap(tokens, ctx));
252253
}
253254

255+
let arg_ty = arg.ty.clone().elide_lifetimes();
254256
if let Some(FromPyWithAttribute { kw, .. }) = arg.from_py_with {
255257
let extractor = quote_spanned! { kw.span =>
256258
{ let from_py_with: fn(_) -> _ = #from_py_with; from_py_with }
@@ -279,9 +281,13 @@ pub(crate) fn impl_regular_arg_param(
279281
}
280282
} else if let Some(default) = default {
281283
let holder = holders.push_holder(arg.name.span());
282-
if arg.option_wrapped_type.is_some() {
284+
if let Some(arg_ty) = arg.option_wrapped_type {
285+
let arg_ty = arg_ty.clone().elide_lifetimes();
283286
quote_arg_span! {
284-
#pyo3_path::impl_::extract_argument::extract_optional_argument(
287+
#pyo3_path::impl_::extract_argument::extract_optional_argument::<
288+
_,
289+
{ #pyo3_path::impl_::pyclass::IsOption::<#arg_ty>::VALUE }
290+
>(
285291
#arg_value,
286292
&mut #holder,
287293
#name_str,
@@ -293,22 +299,28 @@ pub(crate) fn impl_regular_arg_param(
293299
}
294300
} else {
295301
quote_arg_span! {
296-
#pyo3_path::impl_::extract_argument::extract_argument_with_default(
297-
#arg_value,
298-
&mut #holder,
299-
#name_str,
300-
#[allow(clippy::redundant_closure)]
301-
{
302-
|| #default
303-
}
304-
)?
302+
#pyo3_path::impl_::extract_argument::extract_argument_with_default::<
303+
_,
304+
{ #pyo3_path::impl_::pyclass::IsOption::<#arg_ty>::VALUE }
305+
>(
306+
#arg_value,
307+
&mut #holder,
308+
#name_str,
309+
#[allow(clippy::redundant_closure)]
310+
{
311+
|| #default
312+
}
313+
)?
305314
}
306315
}
307316
} else {
308317
let holder = holders.push_holder(arg.name.span());
309318
let unwrap = quote! {unsafe { #pyo3_path::impl_::extract_argument::unwrap_required_argument(#arg_value) }};
310319
quote_arg_span! {
311-
#pyo3_path::impl_::extract_argument::extract_argument(
320+
#pyo3_path::impl_::extract_argument::extract_argument::<
321+
_,
322+
{ #pyo3_path::impl_::pyclass::IsOption::<#arg_ty>::VALUE }
323+
>(
312324
#unwrap,
313325
&mut #holder,
314326
#name_str

‎pyo3-macros-backend/src/pyclass.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2097,7 +2097,7 @@ impl<'a> PyClassImplsBuilder<'a> {
20972097
let cls = self.cls;
20982098
if self.attr.options.frozen.is_some() {
20992099
quote! {
2100-
impl<'a, 'py> #pyo3_path::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a #cls
2100+
impl<'a, 'py> #pyo3_path::impl_::extract_argument::PyFunctionArgument<'a, 'py, false> for &'a #cls
21012101
{
21022102
type Holder = ::std::option::Option<#pyo3_path::PyRef<'py, #cls>>;
21032103

@@ -2109,7 +2109,7 @@ impl<'a> PyClassImplsBuilder<'a> {
21092109
}
21102110
} else {
21112111
quote! {
2112-
impl<'a, 'py> #pyo3_path::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a #cls
2112+
impl<'a, 'py> #pyo3_path::impl_::extract_argument::PyFunctionArgument<'a, 'py, false> for &'a #cls
21132113
{
21142114
type Holder = ::std::option::Option<#pyo3_path::PyRef<'py, #cls>>;
21152115

@@ -2119,7 +2119,7 @@ impl<'a> PyClassImplsBuilder<'a> {
21192119
}
21202120
}
21212121

2122-
impl<'a, 'py> #pyo3_path::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a mut #cls
2122+
impl<'a, 'py> #pyo3_path::impl_::extract_argument::PyFunctionArgument<'a, 'py, false> for &'a mut #cls
21232123
{
21242124
type Holder = ::std::option::Option<#pyo3_path::PyRefMut<'py, #cls>>;
21252125

‎pyo3-macros-backend/src/pymethod.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::ffi::CString;
44
use crate::attributes::{FromPyWithAttribute, NameAttribute, RenamingRule};
55
use crate::method::{CallingConvention, ExtractErrorMode, PyArg};
66
use crate::params::{impl_regular_arg_param, Holders};
7-
use crate::utils::{deprecated_from_py_with, PythonDoc};
7+
use crate::utils::{deprecated_from_py_with, PythonDoc, TypeExt as _};
88
use crate::utils::{Ctx, LitCStr};
99
use crate::{
1010
method::{FnArg, FnSpec, FnType, SelfType},
@@ -699,8 +699,13 @@ pub fn impl_py_setter_def(
699699
.unwrap_or_default();
700700

701701
let holder = holders.push_holder(span);
702+
let ty = field.ty.clone().elide_lifetimes();
702703
quote! {
703-
let _val = #pyo3_path::impl_::extract_argument::extract_argument(_value.into(), &mut #holder, #name)?;
704+
use #pyo3_path::impl_::pyclass::Probe as _;
705+
let _val = #pyo3_path::impl_::extract_argument::extract_argument::<
706+
_,
707+
{ #pyo3_path::impl_::pyclass::IsOption::<#ty>::VALUE }
708+
>(_value.into(), &mut #holder, #name)?;
704709
}
705710
}
706711
};
@@ -1198,13 +1203,18 @@ fn extract_object(
11981203
}
11991204
} else {
12001205
let holder = holders.push_holder(Span::call_site());
1201-
quote! {
1202-
#pyo3_path::impl_::extract_argument::extract_argument(
1206+
let ty = arg.ty().clone().elide_lifetimes();
1207+
quote! {{
1208+
use #pyo3_path::impl_::pyclass::Probe as _;
1209+
#pyo3_path::impl_::extract_argument::extract_argument::<
1210+
_,
1211+
{ #pyo3_path::impl_::pyclass::IsOption::<#ty>::VALUE }
1212+
>(
12031213
unsafe { #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &#source_ptr).0 },
12041214
&mut #holder,
12051215
#name
12061216
)
1207-
}
1217+
}}
12081218
};
12091219

12101220
let extracted = extract_error_mode.handle_error(extract, ctx);

‎pyo3-macros-backend/src/utils.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,3 +337,73 @@ pub(crate) fn deprecated_from_py_with(expr_path: &ExprPathWrap) -> Option<TokenS
337337
}
338338
})
339339
}
340+
341+
pub(crate) trait TypeExt {
342+
/// Replaces all explicit lifetimes in `self` with elided (`'_`) lifetimes
343+
///
344+
/// This is useful if `Self` is used in `const` context, where explicit
345+
/// lifetimes are not allowed (yet).
346+
fn elide_lifetimes(self) -> Self;
347+
}
348+
349+
impl TypeExt for syn::Type {
350+
fn elide_lifetimes(mut self) -> Self {
351+
fn elide_lifetimes(ty: &mut syn::Type) {
352+
match ty {
353+
syn::Type::Path(type_path) => {
354+
if let Some(qself) = &mut type_path.qself {
355+
elide_lifetimes(&mut qself.ty)
356+
}
357+
for seg in &mut type_path.path.segments {
358+
if let syn::PathArguments::AngleBracketed(args) = &mut seg.arguments {
359+
for generic_arg in &mut args.args {
360+
match generic_arg {
361+
syn::GenericArgument::Lifetime(lt) => {
362+
*lt = syn::Lifetime::new("'_", lt.span());
363+
}
364+
syn::GenericArgument::Type(ty) => elide_lifetimes(ty),
365+
syn::GenericArgument::AssocType(assoc) => {
366+
elide_lifetimes(&mut assoc.ty)
367+
}
368+
369+
syn::GenericArgument::Const(_)
370+
| syn::GenericArgument::AssocConst(_)
371+
| syn::GenericArgument::Constraint(_)
372+
| _ => {}
373+
}
374+
}
375+
}
376+
}
377+
}
378+
syn::Type::Reference(type_ref) => {
379+
if let Some(lt) = type_ref.lifetime.as_mut() {
380+
*lt = syn::Lifetime::new("'_", lt.span());
381+
}
382+
elide_lifetimes(&mut type_ref.elem);
383+
}
384+
syn::Type::Tuple(type_tuple) => {
385+
for ty in &mut type_tuple.elems {
386+
elide_lifetimes(ty);
387+
}
388+
}
389+
syn::Type::Array(type_array) => elide_lifetimes(&mut type_array.elem),
390+
syn::Type::Slice(ty) => elide_lifetimes(&mut ty.elem),
391+
syn::Type::Group(ty) => elide_lifetimes(&mut ty.elem),
392+
syn::Type::Paren(ty) => elide_lifetimes(&mut ty.elem),
393+
syn::Type::Ptr(ty) => elide_lifetimes(&mut ty.elem),
394+
395+
syn::Type::BareFn(_)
396+
| syn::Type::ImplTrait(_)
397+
| syn::Type::Infer(_)
398+
| syn::Type::Macro(_)
399+
| syn::Type::Never(_)
400+
| syn::Type::TraitObject(_)
401+
| syn::Type::Verbatim(_)
402+
| _ => {}
403+
}
404+
}
405+
406+
elide_lifetimes(&mut self);
407+
self
408+
}
409+
}

‎src/impl_/extract_argument.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ type PyArg<'py> = Borrowed<'py, 'py, PyAny>;
2020
/// will be dropped as soon as the pyfunction call ends.
2121
///
2222
/// There exists a trivial blanket implementation for `T: FromPyObject` with `Holder = ()`.
23-
pub trait PyFunctionArgument<'a, 'py>: Sized + 'a {
23+
pub trait PyFunctionArgument<'a, 'py, const IS_OPTION: bool>: Sized + 'a {
2424
type Holder: FunctionArgumentHolder;
2525
fn extract(obj: &'a Bound<'py, PyAny>, holder: &'a mut Self::Holder) -> PyResult<Self>;
2626
}
2727

28-
impl<'a, 'py, T> PyFunctionArgument<'a, 'py> for T
28+
impl<'a, 'py, T> PyFunctionArgument<'a, 'py, false> for T
2929
where
3030
T: FromPyObjectBound<'a, 'py> + 'a,
3131
{
@@ -37,7 +37,7 @@ where
3737
}
3838
}
3939

40-
impl<'a, 'py, T: 'py> PyFunctionArgument<'a, 'py> for &'a Bound<'py, T>
40+
impl<'a, 'py, T: 'py> PyFunctionArgument<'a, 'py, false> for &'a Bound<'py, T>
4141
where
4242
T: PyTypeCheck,
4343
{
@@ -49,24 +49,24 @@ where
4949
}
5050
}
5151

52-
impl<'a, 'py, T: 'py> PyFunctionArgument<'a, 'py> for Option<&'a Bound<'py, T>>
52+
impl<'a, 'py, T> PyFunctionArgument<'a, 'py, true> for Option<T>
5353
where
54-
T: PyTypeCheck,
54+
T: PyFunctionArgument<'a, 'py, false>, // inner `Option`s will use `FromPyObject`
5555
{
56-
type Holder = ();
56+
type Holder = T::Holder;
5757

5858
#[inline]
59-
fn extract(obj: &'a Bound<'py, PyAny>, _: &'a mut ()) -> PyResult<Self> {
59+
fn extract(obj: &'a Bound<'py, PyAny>, holder: &'a mut T::Holder) -> PyResult<Self> {
6060
if obj.is_none() {
6161
Ok(None)
6262
} else {
63-
Ok(Some(obj.downcast()?))
63+
Ok(Some(T::extract(obj, holder)?))
6464
}
6565
}
6666
}
6767

6868
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
69-
impl<'a> PyFunctionArgument<'a, '_> for &'a str {
69+
impl<'a> PyFunctionArgument<'a, '_, false> for &'a str {
7070
type Holder = Option<std::borrow::Cow<'a, str>>;
7171

7272
#[inline]
@@ -110,13 +110,13 @@ pub fn extract_pyclass_ref_mut<'a, 'py: 'a, T: PyClass<Frozen = False>>(
110110

111111
/// The standard implementation of how PyO3 extracts a `#[pyfunction]` or `#[pymethod]` function argument.
112112
#[doc(hidden)]
113-
pub fn extract_argument<'a, 'py, T>(
113+
pub fn extract_argument<'a, 'py, T, const IS_OPTION: bool>(
114114
obj: &'a Bound<'py, PyAny>,
115115
holder: &'a mut T::Holder,
116116
arg_name: &str,
117117
) -> PyResult<T>
118118
where
119-
T: PyFunctionArgument<'a, 'py>,
119+
T: PyFunctionArgument<'a, 'py, IS_OPTION>,
120120
{
121121
match PyFunctionArgument::extract(obj, holder) {
122122
Ok(value) => Ok(value),
@@ -127,14 +127,14 @@ where
127127
/// Alternative to [`extract_argument`] used for `Option<T>` arguments. This is necessary because Option<&T>
128128
/// does not implement `PyFunctionArgument` for `T: PyClass`.
129129
#[doc(hidden)]
130-
pub fn extract_optional_argument<'a, 'py, T>(
130+
pub fn extract_optional_argument<'a, 'py, T, const IS_OPTION: bool>(
131131
obj: Option<&'a Bound<'py, PyAny>>,
132132
holder: &'a mut T::Holder,
133133
arg_name: &str,
134134
default: fn() -> Option<T>,
135135
) -> PyResult<Option<T>>
136136
where
137-
T: PyFunctionArgument<'a, 'py>,
137+
T: PyFunctionArgument<'a, 'py, IS_OPTION>,
138138
{
139139
match obj {
140140
Some(obj) => {
@@ -151,14 +151,14 @@ where
151151

152152
/// Alternative to [`extract_argument`] used when the argument has a default value provided by an annotation.
153153
#[doc(hidden)]
154-
pub fn extract_argument_with_default<'a, 'py, T>(
154+
pub fn extract_argument_with_default<'a, 'py, T, const IS_OPTION: bool>(
155155
obj: Option<&'a Bound<'py, PyAny>>,
156156
holder: &'a mut T::Holder,
157157
arg_name: &str,
158158
default: fn() -> T,
159159
) -> PyResult<T>
160160
where
161-
T: PyFunctionArgument<'a, 'py>,
161+
T: PyFunctionArgument<'a, 'py, IS_OPTION>,
162162
{
163163
match obj {
164164
Some(obj) => extract_argument(obj, holder, arg_name),

‎src/impl_/pyclass/probes.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,9 @@ probe!(IsSync);
7070
impl<T: Sync> IsSync<T> {
7171
pub const VALUE: bool = true;
7272
}
73+
74+
probe!(IsOption);
75+
76+
impl<T> IsOption<Option<T>> {
77+
pub const VALUE: bool = true;
78+
}

0 commit comments

Comments
 (0)