Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add second lifetime to FromPyObject #4390

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,123 @@
This guide can help you upgrade code through breaking changes from one PyO3 version to the next.
For a detailed list of all changes, see the [CHANGELOG](changelog.md).

## from 0.23.* to 0.24

### `FromPyObject` gains additional lifetime
<details open>
<summary><small>Click to expand</small></summary>

With the removal of the `gil-ref` API it is now possible to fully split the Python GIL lifetime
`'py` and the input lifetime `'a`. This allows borrowing from the input data without extending the
GIL lifetime.

`FromPyObject` now takes an additional lifetime `'a` describing the input lifetime. The argument
type of the `extract` method changed from `&Bound<'py, PyAny>` to `Borrowed<'a, 'py, PyAny>`. This was
done because `&'a Bound<'py, PyAny>` would have an implicit restriction `'py: 'a` due to the reference type. `extract_bound` with its
old signature is deprecated, but still available during migration.

This new form was partly implemented already in 0.22 using the internal `FromPyObjectBound` trait and
is now extended to all types.

Most implementations can just add an elided lifetime to migrate.

Before:
```rust,ignore
impl<'py> FromPyObject<'py> for IpAddr {
fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult<Self> {
...
}
}
```

After
```rust,ignore
impl<'py> FromPyObject<'_, 'py> for IpAddr {
fn extract(obj: Borrowed<'_, 'py, PyAny>) -> PyResult<Self> {
...
// since `Borrowed` derefs to `&Bound`, the body often
// needs no changes, or adding an occasional `&`
}
}
```

Occasionally, more steps are necessary. For generic types, the bounds need to be adjusted. The
correct bound depends on how the type is used.

For simple wrapper types usually it's possible to just forward the bound.

Before:
```rust,ignore
struct MyWrapper<T>(T);

impl<'py, T> FromPyObject<'py> for MyWrapper<T>
where
T: FromPyObject<'py>
{
fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult<Self> {
ob.extract().map(MyWrapper)
}
}
```

After:
```rust
# use pyo3::prelude::*;
# pub struct MyWrapper<T>(T);
impl<'a, 'py, T> FromPyObject<'a, 'py> for MyWrapper<T>
where
T: FromPyObject<'a, 'py>
{
fn extract(obj: Borrowed<'a, 'py, PyAny>) -> PyResult<Self> {
obj.extract().map(MyWrapper)
}
}
```

Container types that need to create temporary Python references during extraction, for example
extracing from a `PyList`, require a stronger bound. For these the `FromPyObjectOwned` trait was
introduced. It is automatically implemented for any type that implements `FromPyObject` and does not
borrow from the input. It is intended to be used as a trait bound in these situations.

Before:
```rust,ignore
struct MyVec<T>(Vec<T>);
impl<'py, T> FromPyObject<'py> for Vec<T>
where
T: FromPyObject<'py>,
{
fn extract_bound(obj: &Bound<'py, PyAny>) -> PyResult<Self> {
let mut v = MyVec(Vec::new());
for item in obj.try_iter()? {
v.0.push(item?.extract::<T>()?);
}
Ok(v)
}
}
```

After:
```rust
# use pyo3::prelude::*;
# pub struct MyVec<T>(Vec<T>);
impl<'py, T> FromPyObject<'_, 'py> for MyVec<T>
where
T: FromPyObjectOwned<'py> // 👈 can only extract owned values, because each `item` below
// is a temporary short lived owned reference
{
fn extract(obj: Borrowed<'_, 'py, PyAny>) -> PyResult<Self> {
let mut v = MyVec(Vec::new());
for item in obj.try_iter()? {
v.0.push(item?.extract::<T>()?);
}
Ok(v)
}
}
```

This is very similar to `serde`s `Deserialize` and `DeserializeOwned` traits.
</details>

## from 0.22.* to 0.23
<details open>
<summary><small>Click to expand</small></summary>
Expand Down
2 changes: 2 additions & 0 deletions newsfragments/4390.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
added `FromPyObjectOwned` as more convenient trait bound
added `Borrowed::extract`, same as `PyAnyMethods::extract`, but does not restrict the lifetime by deref
3 changes: 3 additions & 0 deletions newsfragments/4390.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
added second lifetime to `FromPyObject`
reintroduced `extract` method
deprecated `extract_bound` method
1 change: 1 addition & 0 deletions newsfragments/4390.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
removed `FromPyObjectBound`
7 changes: 4 additions & 3 deletions pyo3-macros-backend/src/frompyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
let gen_ident = &param.ident;
where_clause
.predicates
.push(parse_quote!(#gen_ident: #pyo3_path::FromPyObject<'py>))
.push(parse_quote!(#gen_ident: #pyo3_path::conversion::FromPyObjectOwned<#lt_param>))
}

let derives = match &tokens.data {
Expand Down Expand Up @@ -619,8 +619,9 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
let ident = &tokens.ident;
Ok(quote!(
#[automatically_derived]
impl #impl_generics #pyo3_path::FromPyObject<#lt_param> for #ident #ty_generics #where_clause {
fn extract_bound(obj: &#pyo3_path::Bound<#lt_param, #pyo3_path::PyAny>) -> #pyo3_path::PyResult<Self> {
impl #impl_generics #pyo3_path::FromPyObject<'_, #lt_param> for #ident #ty_generics #where_clause {
fn extract(obj: #pyo3_path::Borrowed<'_, #lt_param, #pyo3_path::PyAny>) -> #pyo3_path::PyResult<Self> {
let obj: &#pyo3_path::Bound<'_, _> = &*obj;
#derives
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
// DEALINGS IN THE SOFTWARE.

//! `PyBuffer` implementation
use crate::Bound;
use crate::{err, exceptions::PyBufferError, ffi, FromPyObject, PyAny, PyResult, Python};
use crate::{Borrowed, Bound};
use std::marker::PhantomData;
use std::os::raw;
use std::pin::Pin;
Expand Down Expand Up @@ -182,9 +182,9 @@ pub unsafe trait Element: Copy {
fn is_compatible_format(format: &CStr) -> bool;
}

impl<T: Element> FromPyObject<'_> for PyBuffer<T> {
fn extract_bound(obj: &Bound<'_, PyAny>) -> PyResult<PyBuffer<T>> {
Self::get(obj)
impl<T: Element> FromPyObject<'_, '_> for PyBuffer<T> {
fn extract(obj: Borrowed<'_, '_, PyAny>) -> PyResult<PyBuffer<T>> {
Self::get(&obj)
}
}

Expand Down
Loading
Loading