Skip to content

Commit

Permalink
Merge branch 'main' into gil-ref-deprecation
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Mar 20, 2024
2 parents ae74999 + cedac43 commit 094787e
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 46 deletions.
168 changes: 128 additions & 40 deletions pyo3-macros-backend/src/frompyobject.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::attributes::{self, get_pyo3_options, CrateAttribute, FromPyWithAttribute};
use crate::utils::Ctx;
use proc_macro2::TokenStream;
use quote::{format_ident, quote};
use quote::{format_ident, quote, quote_spanned};
use syn::{
parenthesized,
parse::{Parse, ParseStream},
Expand Down Expand Up @@ -44,13 +44,16 @@ impl<'a> Enum<'a> {
}

/// Build derivation body for enums.
fn build(&self, ctx: &Ctx) -> TokenStream {
fn build(&self, ctx: &Ctx) -> (TokenStream, TokenStream) {
let Ctx { pyo3_path } = ctx;
let mut var_extracts = Vec::new();
let mut variant_names = Vec::new();
let mut error_names = Vec::new();

let mut deprecations = TokenStream::new();
for var in &self.variants {
let struct_derive = var.build(ctx);
let (struct_derive, dep) = var.build(ctx);
deprecations.extend(dep);
let ext = quote!({
let maybe_ret = || -> #pyo3_path::PyResult<Self> {
#struct_derive
Expand All @@ -67,19 +70,22 @@ impl<'a> Enum<'a> {
error_names.push(&var.err_name);
}
let ty_name = self.enum_ident.to_string();
quote!(
let errors = [
#(#var_extracts),*
];
::std::result::Result::Err(
#pyo3_path::impl_::frompyobject::failed_to_extract_enum(
obj.py(),
#ty_name,
&[#(#variant_names),*],
&[#(#error_names),*],
&errors
(
quote!(
let errors = [
#(#var_extracts),*
];
::std::result::Result::Err(
#pyo3_path::impl_::frompyobject::failed_to_extract_enum(
obj.py(),
#ty_name,
&[#(#variant_names),*],
&[#(#error_names),*],
&errors
)
)
)
),
deprecations,
)
}
}
Expand Down Expand Up @@ -238,7 +244,7 @@ impl<'a> Container<'a> {
}

/// Build derivation body for a struct.
fn build(&self, ctx: &Ctx) -> TokenStream {
fn build(&self, ctx: &Ctx) -> (TokenStream, TokenStream) {
match &self.ty {
ContainerType::StructNewtype(ident, from_py_with) => {
self.build_newtype_struct(Some(ident), from_py_with, ctx)
Expand All @@ -256,41 +262,73 @@ impl<'a> Container<'a> {
field_ident: Option<&Ident>,
from_py_with: &Option<FromPyWithAttribute>,
ctx: &Ctx,
) -> TokenStream {
) -> (TokenStream, TokenStream) {
let Ctx { pyo3_path } = ctx;
let self_ty = &self.path;
let struct_name = self.name();
if let Some(ident) = field_ident {
let field_name = ident.to_string();
match from_py_with {
None => quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
})
},
None => (
quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)?
})
},
TokenStream::new(),
),
Some(FromPyWithAttribute {
value: expr_path, ..
}) => quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
})
},
}) => (
quote! {
Ok(#self_ty {
#ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
})
},
quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
},
),
}
} else {
match from_py_with {
None => quote!(
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
None => (
quote!(
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty)
),
TokenStream::new(),
),
Some(FromPyWithAttribute {
value: expr_path, ..
}) => quote! (
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
}) => (
quote! (
#pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
),
quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
},
),
}
}
}

fn build_tuple_struct(&self, struct_fields: &[TupleStructField], ctx: &Ctx) -> TokenStream {
fn build_tuple_struct(
&self,
struct_fields: &[TupleStructField],
ctx: &Ctx,
) -> (TokenStream, TokenStream) {
let Ctx { pyo3_path } = ctx;
let self_ty = &self.path;
let struct_name = &self.name();
Expand All @@ -309,15 +347,41 @@ impl<'a> Container<'a> {
),
}
});
quote!(
match #pyo3_path::types::PyAnyMethods::extract(obj) {
::std::result::Result::Ok((#(#field_idents),*)) => ::std::result::Result::Ok(#self_ty(#(#fields),*)),
::std::result::Result::Err(err) => ::std::result::Result::Err(err),
}

let deprecations = struct_fields
.iter()
.filter_map(|field| {
let FromPyWithAttribute {
value: expr_path, ..
} = field.from_py_with.as_ref()?;
Some(quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
})
})
.collect::<TokenStream>();

(
quote!(
match #pyo3_path::types::PyAnyMethods::extract(obj) {
::std::result::Result::Ok((#(#field_idents),*)) => ::std::result::Result::Ok(#self_ty(#(#fields),*)),
::std::result::Result::Err(err) => ::std::result::Result::Err(err),
}
),
deprecations,
)
}

fn build_struct(&self, struct_fields: &[NamedStructField<'_>], ctx: &Ctx) -> TokenStream {
fn build_struct(
&self,
struct_fields: &[NamedStructField<'_>],
ctx: &Ctx,
) -> (TokenStream, TokenStream) {
let Ctx { pyo3_path } = ctx;
let self_ty = &self.path;
let struct_name = &self.name();
Expand Down Expand Up @@ -355,7 +419,29 @@ impl<'a> Container<'a> {

fields.push(quote!(#ident: #extractor));
}
quote!(::std::result::Result::Ok(#self_ty{#fields}))

let deprecations = struct_fields
.iter()
.filter_map(|field| {
let FromPyWithAttribute {
value: expr_path, ..
} = field.from_py_with.as_ref()?;
Some(quote_spanned! { expr_path.span() =>
const _: () = {
fn check_from_py_with() {
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
#pyo3_path::impl_::deprecations::inspect_fn(#expr_path, &e);
e.from_py_with_arg();
}
};
})
})
.collect::<TokenStream>();

(
quote!(::std::result::Result::Ok(#self_ty{#fields})),
deprecations,
)
}
}

Expand Down Expand Up @@ -587,7 +673,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
let ctx = &Ctx::new(&options.krate);
let Ctx { pyo3_path } = &ctx;

let derives = match &tokens.data {
let (derives, from_py_with_deprecations) = match &tokens.data {
syn::Data::Enum(en) => {
if options.transparent || options.annotation.is_some() {
bail_spanned!(tokens.span() => "`transparent` or `annotation` is not supported \
Expand Down Expand Up @@ -617,5 +703,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result<TokenStream> {
#derives
}
}

#from_py_with_deprecations
))
}
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Holders {
}

pub fn check_gil_refs(&self) -> TokenStream {
self.gil_refs_checkers
self.gil_refs_checkers
.iter()
.map(|e| quote_spanned! { e.span() => #e.function_arg(); })
.collect()
Expand Down
27 changes: 24 additions & 3 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,13 @@ impl<'py, T> Bound<'py, T> {
unsafe { Py::from_non_null(non_null) }
}

/// Removes the connection for this `Bound<T>` from the GIL, allowing
/// it to cross thread boundaries, without transferring ownership.
#[inline]
pub fn as_unbound(&self) -> &Py<T> {
&self.1
}

/// Casts this `Bound<T>` as the corresponding "GIL Ref" type.
///
/// This is a helper to be used for migration from the deprecated "GIL Refs" API.
Expand Down Expand Up @@ -521,11 +528,11 @@ impl<'py, T> Borrowed<'_, 'py, T> {
/// # fn main() -> PyResult<()> {
/// Python::with_gil(|py| -> PyResult<()> {
/// let tuple = PyTuple::new_bound(py, [1, 2, 3]);
///
///
/// // borrows from `tuple`, so can only be
/// // used while `tuple` stays alive
/// let borrowed = tuple.get_borrowed_item(0)?;
///
///
/// // creates a new owned reference, which
/// // can be used indendently of `tuple`
/// let bound = borrowed.to_owned();
Expand Down Expand Up @@ -1960,8 +1967,8 @@ impl PyObject {
mod tests {
use super::{Bound, Py, PyObject};
use crate::types::any::PyAnyMethods;
use crate::types::PyCapsule;
use crate::types::{dict::IntoPyDict, PyDict, PyString};
use crate::types::{PyCapsule, PyStringMethods};
use crate::{ffi, Borrowed, PyAny, PyNativeType, PyResult, Python, ToPyObject};

#[test]
Expand Down Expand Up @@ -2161,6 +2168,20 @@ a = A()
});
}

#[test]
fn test_bound_py_conversions() {
Python::with_gil(|py| {
let obj: Bound<'_, PyString> = PyString::new_bound(py, "hello world");
let obj_unbound: &Py<PyString> = obj.as_unbound();
let _: &Bound<'_, PyString> = obj_unbound.bind(py);

let obj_unbound: Py<PyString> = obj.unbind();
let obj: Bound<'_, PyString> = obj_unbound.into_bound(py);

assert_eq!(obj.to_cow().unwrap(), "hello world");
});
}

#[test]
fn bound_from_borrowed_ptr_constructors() {
// More detailed tests of the underlying semantics in pycell.rs
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,45 @@ fn pyfunction_from_py_with(
#[pyfunction]
fn pyfunction_gil_ref(_any: &PyAny) {}

#[derive(Debug, FromPyObject)]
pub struct Zap {
#[pyo3(item)]
name: String,

#[pyo3(from_py_with = "PyAny::len", item("my_object"))]
some_object_length: usize,

#[pyo3(from_py_with = "extract_bound")]
some_number: i32,
}

#[derive(Debug, FromPyObject)]
pub struct ZapTuple(
String,
#[pyo3(from_py_with = "PyAny::len")] usize,
#[pyo3(from_py_with = "extract_bound")] i32,
);

#[derive(Debug, FromPyObject, PartialEq, Eq)]
pub enum ZapEnum {
Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32),
Zap(String, #[pyo3(from_py_with = "extract_bound")] i32),
}

#[derive(Debug, FromPyObject, PartialEq, Eq)]
#[pyo3(transparent)]
pub struct TransparentFromPyWithGilRef {
#[pyo3(from_py_with = "extract_gil_ref")]
len: i32,
}

#[derive(Debug, FromPyObject, PartialEq, Eq)]
#[pyo3(transparent)]
pub struct TransparentFromPyWithBound {
#[pyo3(from_py_with = "extract_bound")]
len: i32,
}

fn test_wrap_pyfunction(py: Python<'_>, m: &Bound<'_, PyModule>) {
// should lint
let _ = wrap_pyfunction!(double, py);
Expand Down
Loading

0 comments on commit 094787e

Please sign in to comment.