Skip to content

Commit

Permalink
adjust path for GIL Refs deprecation warnings (#3968)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt authored Mar 19, 2024
1 parent cbed7c1 commit 02e188e
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 97 deletions.
27 changes: 12 additions & 15 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ impl<'a> FnSpec<'a> {
holders.pop().unwrap(); // does not actually use holder created by `self_arg`

quote! {{
#self_e = #pyo3_path::impl_::pymethods::Extractor::<()>::new();
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
let __guard = #pyo3_path::impl_::coroutine::RefGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
async move { function(&__guard, #(#args),*).await }
}}
Expand All @@ -549,20 +549,20 @@ impl<'a> FnSpec<'a> {
holders.pop().unwrap(); // does not actually use holder created by `self_arg`

quote! {{
#self_e = #pyo3_path::impl_::pymethods::Extractor::<()>::new();
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
let mut __guard = #pyo3_path::impl_::coroutine::RefMutGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
async move { function(&mut __guard, #(#args),*).await }
}}
}
_ => {
if self_arg.is_empty() {
quote! {{
#self_e = #pyo3_path::impl_::pymethods::Extractor::<()>::new();
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
function(#(#args),*)
}}
} else {
quote! { function({
let (self_arg, e) = #pyo3_path::impl_::pymethods::inspect_type(#self_arg);
let (self_arg, e) = #pyo3_path::impl_::deprecations::inspect_type(#self_arg);
#self_e = e;
self_arg
}, #(#args),*) }
Expand All @@ -588,13 +588,13 @@ impl<'a> FnSpec<'a> {
call
} else if self_arg.is_empty() {
quote! {{
#self_e = #pyo3_path::impl_::pymethods::Extractor::<()>::new();
#self_e = #pyo3_path::impl_::deprecations::GilRefs::<()>::new();
function(#(#args),*)
}}
} else {
quote! {
function({
let (self_arg, e) = #pyo3_path::impl_::pymethods::inspect_type(#self_arg);
let (self_arg, e) = #pyo3_path::impl_::deprecations::inspect_type(#self_arg);
#self_e = e;
self_arg
}, #(#args),*)
Expand Down Expand Up @@ -632,8 +632,7 @@ impl<'a> FnSpec<'a> {
})
.collect();
let (call, self_arg_span) = rust_call(args, &self_e, &mut holders);
let extract_gil_ref =
quote_spanned! { self_arg_span => #self_e.extract_gil_ref(); };
let function_arg = quote_spanned! { self_arg_span => #self_e.function_arg(); };

quote! {
unsafe fn #ident<'py>(
Expand All @@ -645,7 +644,7 @@ impl<'a> FnSpec<'a> {
let #self_e;
#( #holders )*
let result = #call;
#extract_gil_ref
#function_arg
result
}
}
Expand All @@ -654,8 +653,7 @@ impl<'a> FnSpec<'a> {
let mut holders = Vec::new();
let (arg_convert, args) = impl_arg_params(self, cls, true, &mut holders, ctx)?;
let (call, self_arg_span) = rust_call(args, &self_e, &mut holders);
let extract_gil_ref =
quote_spanned! { self_arg_span => #self_e.extract_gil_ref(); };
let function_arg = quote_spanned! { self_arg_span => #self_e.function_arg(); };

quote! {
unsafe fn #ident<'py>(
Expand All @@ -671,7 +669,7 @@ impl<'a> FnSpec<'a> {
#arg_convert
#( #holders )*
let result = #call;
#extract_gil_ref
#function_arg
result
}
}
Expand All @@ -680,8 +678,7 @@ impl<'a> FnSpec<'a> {
let mut holders = Vec::new();
let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx)?;
let (call, self_arg_span) = rust_call(args, &self_e, &mut holders);
let extract_gil_ref =
quote_spanned! { self_arg_span => #self_e.extract_gil_ref(); };
let function_arg = quote_spanned! { self_arg_span => #self_e.function_arg(); };

quote! {
unsafe fn #ident<'py>(
Expand All @@ -696,7 +693,7 @@ impl<'a> FnSpec<'a> {
#arg_convert
#( #holders )*
let result = #call;
#extract_gil_ref
#function_arg
result
}
}
Expand Down
7 changes: 4 additions & 3 deletions pyo3-macros-backend/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,8 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream>
if function.sig.inputs.len() == 2 {
module_args.push(quote!(module.py()));
}
module_args.push(quote!(::std::convert::Into::into(#pyo3_path::methods::BoundRef(module))));
module_args
.push(quote!(::std::convert::Into::into(#pyo3_path::impl_::pymethods::BoundRef(module))));

let extractors = function
.sig
Expand All @@ -306,8 +307,8 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream>
if let syn::Pat::Ident(pat_ident) = &*pat_type.pat {
let ident = &pat_ident.ident;
return Some([
parse_quote! { let (#ident, e) = #pyo3_path::impl_::pymethods::inspect_type(#ident); },
parse_quote_spanned! { pat_type.span() => e.extract_gil_ref(); },
parse_quote! { let (#ident, e) = #pyo3_path::impl_::deprecations::inspect_type(#ident); },
parse_quote_spanned! { pat_type.span() => e.function_arg(); },
]);
}
}
Expand Down
6 changes: 3 additions & 3 deletions pyo3-macros-backend/src/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ pub fn impl_arg_params(
let from_py_with_holder =
syn::Ident::new(&format!("from_py_with_{}", i), Span::call_site());
Some(quote_spanned! { from_py_with.span() =>
let e = #pyo3_path::impl_::pymethods::Extractor::new();
let #from_py_with_holder = #pyo3_path::impl_::pymethods::inspect_fn(#from_py_with, &e);
e.extract_from_py_with();
let e = #pyo3_path::impl_::deprecations::GilRefs::new();
let #from_py_with_holder = #pyo3_path::impl_::deprecations::inspect_fn(#from_py_with, &e);
e.from_py_with_arg();
})
})
.collect::<TokenStream>();
Expand Down
64 changes: 64 additions & 0 deletions src/impl_/deprecations.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,68 @@
//! Symbols used to denote deprecated usages of PyO3's proc macros.
use crate::{PyResult, Python};

#[deprecated(since = "0.20.0", note = "use `#[new]` instead of `#[__new__]`")]
pub const PYMETHODS_NEW_DEPRECATED_FORM: () = ();

pub fn inspect_type<T>(t: T) -> (T, GilRefs<T>) {
(t, GilRefs::new())
}

pub fn inspect_fn<A, T>(f: fn(A) -> PyResult<T>, _: &GilRefs<A>) -> fn(A) -> PyResult<T> {
f
}

pub struct GilRefs<T>(NotAGilRef<T>);
pub struct NotAGilRef<T>(std::marker::PhantomData<T>);

pub trait IsGilRef {}

impl<T: crate::PyNativeType> IsGilRef for &'_ T {}

impl<T> GilRefs<T> {
#[allow(clippy::new_without_default)]
pub fn new() -> Self {
GilRefs(NotAGilRef(std::marker::PhantomData))
}
}

impl GilRefs<Python<'_>> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(since = "0.21.0", note = "use `wrap_pyfunction_bound!` instead")
)]
pub fn is_python(&self) {}
}

impl<T: IsGilRef> GilRefs<T> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `&Bound<'_, T>` instead for this function argument"
)
)]
pub fn function_arg(&self) {}
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor"
)
)]
pub fn from_py_with_arg(&self) {}
}

impl<T> NotAGilRef<T> {
pub fn function_arg(&self) {}
pub fn from_py_with_arg(&self) {}
pub fn is_python(&self) {}
}

impl<T> std::ops::Deref for GilRefs<T> {
type Target = NotAGilRef<T>;
fn deref(&self) -> &Self::Target {
&self.0
}
}
62 changes: 0 additions & 62 deletions src/impl_/pymethods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,65 +580,3 @@ pub unsafe fn tp_new_impl<T: PyClass>(
.create_class_object_of_type(py, target_type)
.map(Bound::into_ptr)
}

pub fn inspect_type<T>(t: T) -> (T, Extractor<T>) {
(t, Extractor::new())
}

pub fn inspect_fn<A, T>(f: fn(A) -> PyResult<T>, _: &Extractor<A>) -> fn(A) -> PyResult<T> {
f
}

pub struct Extractor<T>(NotAGilRef<T>);
pub struct NotAGilRef<T>(std::marker::PhantomData<T>);

pub trait IsGilRef {}

impl<T: crate::PyNativeType> IsGilRef for &'_ T {}

impl<T> Extractor<T> {
#[allow(clippy::new_without_default)]
pub fn new() -> Self {
Extractor(NotAGilRef(std::marker::PhantomData))
}
}

impl Extractor<Python<'_>> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(since = "0.21.0", note = "use `wrap_pyfunction_bound!` instead")
)]
pub fn is_python(&self) {}
}

impl<T: IsGilRef> Extractor<T> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `&Bound<'_, T>` instead for this function argument"
)
)]
pub fn extract_gil_ref(&self) {}
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor"
)
)]
pub fn extract_from_py_with(&self) {}
}

impl<T> NotAGilRef<T> {
pub fn extract_gil_ref(&self) {}
pub fn extract_from_py_with(&self) {}
pub fn is_python(&self) {}
}

impl<T> std::ops::Deref for Extractor<T> {
type Target = NotAGilRef<T>;
fn deref(&self) -> &Self::Target {
&self.0
}
}
23 changes: 20 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,16 +346,23 @@ pub(crate) mod sealed;
/// For compatibility reasons this has not yet been removed, however will be done so
/// once <https://github.com/rust-lang/rust/issues/30827> is resolved.
pub mod class {
#[doc(hidden)]
pub use crate::impl_::pymethods as methods;

pub use self::gc::{PyTraverseError, PyVisit};

#[doc(hidden)]
pub use self::methods::{
PyClassAttributeDef, PyGetterDef, PyMethodDef, PyMethodDefType, PyMethodType, PySetterDef,
};

#[doc(hidden)]
pub mod methods {
// frozen with the contents of the `impl_::pymethods` module in 0.20,
// this should probably all be replaced with deprecated type aliases and removed.
pub use crate::impl_::pymethods::{
IPowModulo, PyClassAttributeDef, PyGetterDef, PyMethodDef, PyMethodDefType,
PyMethodType, PySetterDef,
};
}

/// Old module which contained some implementation details of the `#[pyproto]` module.
///
/// Prefer using the same content from `pyo3::pyclass`, e.g. `use pyo3::pyclass::CompareOp` instead
Expand Down Expand Up @@ -479,6 +486,16 @@ mod macros;
#[cfg(feature = "experimental-inspect")]
pub mod inspect;

/// Ths module only contains re-exports of pyo3 deprecation warnings and exists
/// purely to make compiler error messages nicer.
///
/// (The compiler uses this module in error messages, probably because it's a public
/// re-export at a shorter path than `pyo3::impl_::deprecations`.)
#[doc(hidden)]
pub mod deprecations {
pub use crate::impl_::deprecations::*;
}

/// Test readme and user guide
#[cfg(doctest)]
pub mod doc_test {
Expand Down
2 changes: 1 addition & 1 deletion src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ macro_rules! wrap_pyfunction {
};
($function:path, $py_or_module:expr) => {{
use $function as wrapped_pyfunction;
let (py_or_module, e) = $crate::impl_::pymethods::inspect_type($py_or_module);
let (py_or_module, e) = $crate::impl_::deprecations::inspect_type($py_or_module);
e.is_python();
$crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction(
py_or_module,
Expand Down
3 changes: 1 addition & 2 deletions src/types/function.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use crate::derive_utils::PyFunctionArguments;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::methods::PyMethodDefDestructor;
use crate::py_result_ext::PyResultExt;
use crate::types::capsule::PyCapsuleMethods;
use crate::types::module::PyModuleMethods;
use crate::{
ffi,
impl_::pymethods::{self, PyMethodDef},
impl_::pymethods::{self, PyMethodDef, PyMethodDefDestructor},
types::{PyCapsule, PyDict, PyModule, PyString, PyTuple},
};
use crate::{Bound, IntoPy, Py, PyAny, PyNativeType, PyResult, Python};
Expand Down
16 changes: 8 additions & 8 deletions tests/ui/deprecations.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: use of deprecated constant `pyo3::impl_::deprecations::PYMETHODS_NEW_DEPRECATED_FORM`: use `#[new]` instead of `#[__new__]`
error: use of deprecated constant `pyo3::deprecations::PYMETHODS_NEW_DEPRECATED_FORM`: use `#[new]` instead of `#[__new__]`
--> tests/ui/deprecations.rs:12:7
|
12 | #[__new__]
Expand All @@ -16,43 +16,43 @@ error: use of deprecated struct `pyo3::PyCell`: `PyCell` was merged into `Bound`
23 | fn method_gil_ref(_slf: &PyCell<Self>) {}
| ^^^^^^

error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
--> tests/ui/deprecations.rs:18:33
|
18 | fn cls_method_gil_ref(_cls: &PyType) {}
| ^

error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
--> tests/ui/deprecations.rs:23:29
|
23 | fn method_gil_ref(_slf: &PyCell<Self>) {}
| ^

error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
--> tests/ui/deprecations.rs:38:43
|
38 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> {
| ^

error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
--> tests/ui/deprecations.rs:48:19
|
48 | fn module_gil_ref(m: &PyModule) -> PyResult<()> {
| ^

error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`: use `&Bound<'_, T>` instead for this function argument
--> tests/ui/deprecations.rs:54:57
|
54 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
| ^

error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_from_py_with`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor
error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::from_py_with_arg`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor
--> tests/ui/deprecations.rs:87:27
|
87 | #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32,
| ^^^^^^^^^^^^^^^^^

error: use of deprecated method `pyo3::methods::Extractor::<pyo3::Python<'_>>::is_python`: use `wrap_pyfunction_bound!` instead
error: use of deprecated method `pyo3::deprecations::GilRefs::<pyo3::Python<'_>>::is_python`: use `wrap_pyfunction_bound!` instead
--> tests/ui/deprecations.rs:94:13
|
94 | let _ = wrap_pyfunction!(double, py);
Expand Down

0 comments on commit 02e188e

Please sign in to comment.