Skip to content

Commit

Permalink
update extract_argument to use Bound APIs (#3708)
Browse files Browse the repository at this point in the history 
* update `extract_argument` to use `Bound` APIs

* tidy up borrow in macros expression

* update `trybuild` output

* more concise form for `DowncastError::new`

Co-authored-by: Lily Foote <code@lilyf.org>

* use `Borrowed` instead of newtype

* use `Borrowed::from_ptr` methods in extract_argument

* update UI tests

* avoid double-negative `#[cfg]` clauses

Co-authored-by: Lily Foote <code@lilyf.org>

* review: LilyFoote, Icxolu feedback

---------

Co-authored-by: Lily Foote <code@lilyf.org>
  • Loading branch information
@davidhewitt @LilyFoote
davidhewitt and LilyFoote authored Feb 28, 2024
1 parent a15e4b1 commit 8a12970
Show file tree
Hide file tree
Showing 12 changed files with 274 additions and 144 deletions.
4 changes: 2 additions & 2 deletions guide/src/class.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1254,7 +1254,7 @@ impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a
type Holder = ::std::option::Option<pyo3::PyRef<'py, MyClass>>;

#[inline]
fn extract(obj: &'py pyo3::PyAny, holder: &'a mut Self::Holder) -> pyo3::PyResult<Self> {
fn extract(obj: &'a pyo3::Bound<'py, PyAny>, holder: &'a mut Self::Holder) -> pyo3::PyResult<Self> {
pyo3::impl_::extract_argument::extract_pyclass_ref(obj, holder)
}
}
Expand All @@ -1264,7 +1264,7 @@ impl<'a, 'py> pyo3::impl_::extract_argument::PyFunctionArgument<'a, 'py> for &'a
type Holder = ::std::option::Option<pyo3::PyRefMut<'py, MyClass>>;

#[inline]
fn extract(obj: &'py pyo3::PyAny, holder: &'a mut Self::Holder) -> pyo3::PyResult<Self> {
fn extract(obj: &'a pyo3::Bound<'py, PyAny>, holder: &'a mut Self::Holder) -> pyo3::PyResult<Self> {
pyo3::impl_::extract_argument::extract_pyclass_ref_mut(obj, holder)
}
}
Expand Down
8 changes: 4 additions & 4 deletions pyo3-benches/benches/bench_comparisons.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,17 @@ impl OrderedRichcmp {

fn bench_ordered_dunder_methods(b: &mut Bencher<'_>) {
Python::with_gil(|py| {
let obj1 = Py::new(py, OrderedDunderMethods(0)).unwrap().into_ref(py);
let obj2 = Py::new(py, OrderedDunderMethods(1)).unwrap().into_ref(py);
let obj1 = &Bound::new(py, OrderedDunderMethods(0)).unwrap().into_any();
let obj2 = &Bound::new(py, OrderedDunderMethods(1)).unwrap().into_any();

b.iter(|| obj2.gt(obj1).unwrap());
});
}

fn bench_ordered_richcmp(b: &mut Bencher<'_>) {
Python::with_gil(|py| {
let obj1 = Py::new(py, OrderedRichcmp(0)).unwrap().into_ref(py);
let obj2 = Py::new(py, OrderedRichcmp(1)).unwrap().into_ref(py);
let obj1 = &Bound::new(py, OrderedRichcmp(0)).unwrap().into_any();
let obj2 = &Bound::new(py, OrderedRichcmp(1)).unwrap().into_any();

b.iter(|| obj2.gt(obj1).unwrap());
});
Expand Down
12 changes: 8 additions & 4 deletions pyo3-macros-backend/src/method.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,11 @@ impl SelfType {
holders.push(quote_spanned! { *span =>
#[allow(clippy::let_unit_value)]
let mut #holder = _pyo3::impl_::extract_argument::FunctionArgumentHolder::INIT;
let mut #slf = _pyo3::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf);
});
error_mode.handle_error(quote_spanned! { *span =>
_pyo3::impl_::extract_argument::#method::<#cls>(
#py.from_borrowed_ptr::<_pyo3::PyAny>(#slf),
&#slf,
&mut #holder,
)
})
Expand Down Expand Up @@ -582,7 +583,8 @@ impl<'a> FnSpec<'a> {
) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> {
let function = #rust_name; // Shadow the function name to avoid #3017
#( #holders )*
#call
let result = #call;
result
}
}
}
Expand All @@ -601,7 +603,8 @@ impl<'a> FnSpec<'a> {
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#( #holders )*
#call
let result = #call;
result
}
}
}
Expand All @@ -619,7 +622,8 @@ impl<'a> FnSpec<'a> {
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#( #holders )*
#call
let result = #call;
result
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions pyo3-macros-backend/src/params.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ pub fn impl_arg_params(
.collect::<Result<_>>()?;
return Ok((
quote! {
let _args = py.from_borrowed_ptr::<_pyo3::types::PyTuple>(_args);
let _kwargs: ::std::option::Option<&_pyo3::types::PyDict> = py.from_borrowed_ptr_or_opt(_kwargs);
let _args = _pyo3::impl_::pymethods::BoundRef::ref_from_ptr(py, &_args);
let _kwargs = _pyo3::impl_::pymethods::BoundRef::ref_from_ptr_or_opt(py, &_kwargs);
},
arg_convert,
));
Expand Down Expand Up @@ -180,7 +180,7 @@ fn impl_arg_param(
let holder = push_holder();
return Ok(quote_arg_span! {
_pyo3::impl_::extract_argument::extract_argument(
_args,
&_args,
&mut #holder,
#name_str
)?
Expand All @@ -193,7 +193,7 @@ fn impl_arg_param(
let holder = push_holder();
return Ok(quote_arg_span! {
_pyo3::impl_::extract_argument::extract_optional_argument(
_kwargs.map(::std::convert::AsRef::as_ref),
_kwargs.as_deref(),
&mut #holder,
#name_str,
|| ::std::option::Option::None
Expand All @@ -217,7 +217,7 @@ fn impl_arg_param(
quote_arg_span! {
#[allow(clippy::redundant_closure)]
_pyo3::impl_::extract_argument::from_py_with_with_default(
#arg_value.map(_pyo3::PyNativeType::as_borrowed).as_deref(),
#arg_value.as_deref(),
#name_str,
#expr_path as fn(_) -> _,
|| #default
Expand All @@ -226,7 +226,7 @@ fn impl_arg_param(
} else {
quote_arg_span! {
_pyo3::impl_::extract_argument::from_py_with(
&_pyo3::impl_::extract_argument::unwrap_required_argument(#arg_value).as_borrowed(),
&_pyo3::impl_::extract_argument::unwrap_required_argument(#arg_value),
#name_str,
#expr_path as fn(_) -> _,
)?
Expand All @@ -237,7 +237,7 @@ fn impl_arg_param(
quote_arg_span! {
#[allow(clippy::redundant_closure)]
_pyo3::impl_::extract_argument::extract_optional_argument(
#arg_value,
#arg_value.as_deref(),
&mut #holder,
#name_str,
|| #default
Expand All @@ -248,7 +248,7 @@ fn impl_arg_param(
quote_arg_span! {
#[allow(clippy::redundant_closure)]
_pyo3::impl_::extract_argument::extract_argument_with_default(
#arg_value,
#arg_value.as_deref(),
&mut #holder,
#name_str,
|| #default
Expand All @@ -258,7 +258,7 @@ fn impl_arg_param(
let holder = push_holder();
quote_arg_span! {
_pyo3::impl_::extract_argument::extract_argument(
_pyo3::impl_::extract_argument::unwrap_required_argument(#arg_value),
&_pyo3::impl_::extract_argument::unwrap_required_argument(#arg_value),
&mut #holder,
#name_str
)?
Expand Down
6 changes: 3 additions & 3 deletions pyo3-macros-backend/src/pyclass.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1378,7 +1378,7 @@ impl<'a> PyClassImplsBuilder<'a> {
type Holder = ::std::option::Option<_pyo3::PyRef<'py, #cls>>;

#[inline]
fn extract(obj: &'py _pyo3::PyAny, holder: &'a mut Self::Holder) -> _pyo3::PyResult<Self> {
fn extract(obj: &'a _pyo3::Bound<'py, _pyo3::PyAny>, holder: &'a mut Self::Holder) -> _pyo3::PyResult<Self> {
_pyo3::impl_::extract_argument::extract_pyclass_ref(obj, holder)
}
}
Expand All @@ -1390,7 +1390,7 @@ impl<'a> PyClassImplsBuilder<'a> {
type Holder = ::std::option::Option<_pyo3::PyRef<'py, #cls>>;

#[inline]
fn extract(obj: &'py _pyo3::PyAny, holder: &'a mut Self::Holder) -> _pyo3::PyResult<Self> {
fn extract(obj: &'a _pyo3::Bound<'py, _pyo3::PyAny>, holder: &'a mut Self::Holder) -> _pyo3::PyResult<Self> {
_pyo3::impl_::extract_argument::extract_pyclass_ref(obj, holder)
}
}
Expand All @@ -1400,7 +1400,7 @@ impl<'a> PyClassImplsBuilder<'a> {
type Holder = ::std::option::Option<_pyo3::PyRefMut<'py, #cls>>;

#[inline]
fn extract(obj: &'py _pyo3::PyAny, holder: &'a mut Self::Holder) -> _pyo3::PyResult<Self> {
fn extract(obj: &'a _pyo3::Bound<'py, _pyo3::PyAny>, holder: &'a mut Self::Holder) -> _pyo3::PyResult<Self> {
_pyo3::impl_::extract_argument::extract_pyclass_ref_mut(obj, holder)
}
}
Expand Down
31 changes: 12 additions & 19 deletions pyo3-macros-backend/src/pymethod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,8 @@ pub fn impl_py_getter_def(
_slf: *mut _pyo3::ffi::PyObject
) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> {
#( #holders )*
#body
let result = #body;
result
}
};

Expand Down Expand Up @@ -930,39 +931,31 @@ impl Ty {
extract_error_mode,
holders,
&name_str,
quote! {
py.from_borrowed_ptr::<_pyo3::PyAny>(#ident)
},
quote! { #ident },
),
Ty::MaybeNullObject => extract_object(
extract_error_mode,
holders,
&name_str,
quote! {
py.from_borrowed_ptr::<_pyo3::PyAny>(
if #ident.is_null() {
_pyo3::ffi::Py_None()
} else {
#ident
}
)
if #ident.is_null() {
_pyo3::ffi::Py_None()
} else {
#ident
}
},
),
Ty::NonNullObject => extract_object(
extract_error_mode,
holders,
&name_str,
quote! {
py.from_borrowed_ptr::<_pyo3::PyAny>(#ident.as_ptr())
},
quote! { #ident.as_ptr() },
),
Ty::IPowModulo => extract_object(
extract_error_mode,
holders,
&name_str,
quote! {
#ident.to_borrowed_any(py)
},
quote! { #ident.as_ptr() },
),
Ty::CompareOp => extract_error_mode.handle_error(
quote! {
Expand All @@ -988,7 +981,7 @@ fn extract_object(
extract_error_mode: ExtractErrorMode,
holders: &mut Vec<TokenStream>,
name: &str,
source: TokenStream,
source_ptr: TokenStream,
) -> TokenStream {
let holder = syn::Ident::new(&format!("holder_{}", holders.len()), Span::call_site());
holders.push(quote! {
Expand All @@ -997,7 +990,7 @@ fn extract_object(
});
extract_error_mode.handle_error(quote! {
_pyo3::impl_::extract_argument::extract_argument(
#source,
&_pyo3::impl_::pymethods::BoundRef::ref_from_ptr(py, &#source_ptr),
&mut #holder,
#name
)
Expand Down
74 changes: 39 additions & 35 deletions pytests/src/pyfunctions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,62 +4,66 @@ use pyo3::types::{PyDict, PyTuple};
#[pyfunction(signature = ())]
fn none() {}

type Any<'py> = Bound<'py, PyAny>;
type Dict<'py> = Bound<'py, PyDict>;
type Tuple<'py> = Bound<'py, PyTuple>;

#[pyfunction(signature = (a, b = None, *, c = None))]
fn simple<'a>(
a: &'a PyAny,
b: Option<&'a PyAny>,
c: Option<&'a PyAny>,
) -> (&'a PyAny, Option<&'a PyAny>, Option<&'a PyAny>) {
fn simple<'py>(
a: Any<'py>,
b: Option<Any<'py>>,
c: Option<Any<'py>>,
) -> (Any<'py>, Option<Any<'py>>, Option<Any<'py>>) {
(a, b, c)
}

#[pyfunction(signature = (a, b = None, *args, c = None))]
fn simple_args<'a>(
a: &'a PyAny,
b: Option<&'a PyAny>,
args: &'a PyTuple,
c: Option<&'a PyAny>,
) -> (&'a PyAny, Option<&'a PyAny>, &'a PyTuple, Option<&'a PyAny>) {
fn simple_args<'py>(
a: Any<'py>,
b: Option<Any<'py>>,
args: Tuple<'py>,
c: Option<Any<'py>>,
) -> (Any<'py>, Option<Any<'py>>, Tuple<'py>, Option<Any<'py>>) {
(a, b, args, c)
}

#[pyfunction(signature = (a, b = None, c = None, **kwargs))]
fn simple_kwargs<'a>(
a: &'a PyAny,
b: Option<&'a PyAny>,
c: Option<&'a PyAny>,
kwargs: Option<&'a PyDict>,
fn simple_kwargs<'py>(
a: Any<'py>,
b: Option<Any<'py>>,
c: Option<Any<'py>>,
kwargs: Option<Dict<'py>>,
) -> (
&'a PyAny,
Option<&'a PyAny>,
Option<&'a PyAny>,
Option<&'a PyDict>,
Any<'py>,
Option<Any<'py>>,
Option<Any<'py>>,
Option<Dict<'py>>,
) {
(a, b, c, kwargs)
}

#[pyfunction(signature = (a, b = None, *args, c = None, **kwargs))]
fn simple_args_kwargs<'a>(
a: &'a PyAny,
b: Option<&'a PyAny>,
args: &'a PyTuple,
c: Option<&'a PyAny>,
kwargs: Option<&'a PyDict>,
fn simple_args_kwargs<'py>(
a: Any<'py>,
b: Option<Any<'py>>,
args: Tuple<'py>,
c: Option<Any<'py>>,
kwargs: Option<Dict<'py>>,
) -> (
&'a PyAny,
Option<&'a PyAny>,
&'a PyTuple,
Option<&'a PyAny>,
Option<&'a PyDict>,
Any<'py>,
Option<Any<'py>>,
Tuple<'py>,
Option<Any<'py>>,
Option<Dict<'py>>,
) {
(a, b, args, c, kwargs)
}

#[pyfunction(signature = (*args, **kwargs))]
fn args_kwargs<'a>(
args: &'a PyTuple,
kwargs: Option<&'a PyDict>,
) -> (&'a PyTuple, Option<&'a PyDict>) {
fn args_kwargs<'py>(
args: Tuple<'py>,
kwargs: Option<Dict<'py>>,
) -> (Tuple<'py>, Option<Dict<'py>>) {
(args, kwargs)
}

Expand Down
Loading

0 comments on commit 8a12970

Please sign in to comment.