From 02e188e4b4151afb42c52866bd7e3a5f77dfb87d Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 19 Mar 2024 21:08:20 +0000 Subject: [PATCH] adjust path for GIL Refs deprecation warnings (#3968) --- pyo3-macros-backend/src/method.rs | 27 ++++++------- pyo3-macros-backend/src/module.rs | 7 ++-- pyo3-macros-backend/src/params.rs | 6 +-- src/impl_/deprecations.rs | 64 +++++++++++++++++++++++++++++++ src/impl_/pymethods.rs | 62 ------------------------------ src/lib.rs | 23 +++++++++-- src/macros.rs | 2 +- src/types/function.rs | 3 +- tests/ui/deprecations.stderr | 16 ++++---- 9 files changed, 113 insertions(+), 97 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 3cc2a96e..93a54395 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -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 } }} @@ -549,7 +549,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 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 } }} @@ -557,12 +557,12 @@ impl<'a> FnSpec<'a> { _ => { 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),*) } @@ -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),*) @@ -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>( @@ -645,7 +644,7 @@ impl<'a> FnSpec<'a> { let #self_e; #( #holders )* let result = #call; - #extract_gil_ref + #function_arg result } } @@ -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>( @@ -671,7 +669,7 @@ impl<'a> FnSpec<'a> { #arg_convert #( #holders )* let result = #call; - #extract_gil_ref + #function_arg result } } @@ -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>( @@ -696,7 +693,7 @@ impl<'a> FnSpec<'a> { #arg_convert #( #holders )* let result = #call; - #extract_gil_ref + #function_arg result } } diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 1cc3e836..2515d519 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -295,7 +295,8 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result 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 @@ -306,8 +307,8 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result 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(); }, ]); } } diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs index 74b3eba4..ae1661fe 100644 --- a/pyo3-macros-backend/src/params.rs +++ b/pyo3-macros-backend/src/params.rs @@ -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::(); diff --git a/src/impl_/deprecations.rs b/src/impl_/deprecations.rs index 6b9930ac..0c749ff9 100644 --- a/src/impl_/deprecations.rs +++ b/src/impl_/deprecations.rs @@ -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, GilRefs) { + (t, GilRefs::new()) +} + +pub fn inspect_fn(f: fn(A) -> PyResult, _: &GilRefs) -> fn(A) -> PyResult { + f +} + +pub struct GilRefs(NotAGilRef); +pub struct NotAGilRef(std::marker::PhantomData); + +pub trait IsGilRef {} + +impl IsGilRef for &'_ T {} + +impl GilRefs { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + GilRefs(NotAGilRef(std::marker::PhantomData)) + } +} + +impl GilRefs> { + #[cfg_attr( + not(feature = "gil-refs"), + deprecated(since = "0.21.0", note = "use `wrap_pyfunction_bound!` instead") + )] + pub fn is_python(&self) {} +} + +impl GilRefs { + #[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 NotAGilRef { + pub fn function_arg(&self) {} + pub fn from_py_with_arg(&self) {} + pub fn is_python(&self) {} +} + +impl std::ops::Deref for GilRefs { + type Target = NotAGilRef; + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index ca3f0a06..df89dba7 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -580,65 +580,3 @@ pub unsafe fn tp_new_impl( .create_class_object_of_type(py, target_type) .map(Bound::into_ptr) } - -pub fn inspect_type(t: T) -> (T, Extractor) { - (t, Extractor::new()) -} - -pub fn inspect_fn(f: fn(A) -> PyResult, _: &Extractor) -> fn(A) -> PyResult { - f -} - -pub struct Extractor(NotAGilRef); -pub struct NotAGilRef(std::marker::PhantomData); - -pub trait IsGilRef {} - -impl IsGilRef for &'_ T {} - -impl Extractor { - #[allow(clippy::new_without_default)] - pub fn new() -> Self { - Extractor(NotAGilRef(std::marker::PhantomData)) - } -} - -impl Extractor> { - #[cfg_attr( - not(feature = "gil-refs"), - deprecated(since = "0.21.0", note = "use `wrap_pyfunction_bound!` instead") - )] - pub fn is_python(&self) {} -} - -impl Extractor { - #[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 NotAGilRef { - pub fn extract_gil_ref(&self) {} - pub fn extract_from_py_with(&self) {} - pub fn is_python(&self) {} -} - -impl std::ops::Deref for Extractor { - type Target = NotAGilRef; - fn deref(&self) -> &Self::Target { - &self.0 - } -} diff --git a/src/lib.rs b/src/lib.rs index dab48fbe..fd5a520f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -346,9 +346,6 @@ pub(crate) mod sealed; /// For compatibility reasons this has not yet been removed, however will be done so /// once is resolved. pub mod class { - #[doc(hidden)] - pub use crate::impl_::pymethods as methods; - pub use self::gc::{PyTraverseError, PyVisit}; #[doc(hidden)] @@ -356,6 +353,16 @@ pub mod class { 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 @@ -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 { diff --git a/src/macros.rs b/src/macros.rs index bd0bd814..db409515 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -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, diff --git a/src/types/function.rs b/src/types/function.rs index ea8201fb..f5305e31 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -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}; diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index 10a21d3a..8bd14508 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -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__] @@ -16,43 +16,43 @@ error: use of deprecated struct `pyo3::PyCell`: `PyCell` was merged into `Bound` 23 | fn method_gil_ref(_slf: &PyCell) {} | ^^^^^^ -error: use of deprecated method `pyo3::methods::Extractor::::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument +error: use of deprecated method `pyo3::deprecations::GilRefs::::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::::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument +error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument --> tests/ui/deprecations.rs:23:29 | 23 | fn method_gil_ref(_slf: &PyCell) {} | ^ -error: use of deprecated method `pyo3::methods::Extractor::::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument +error: use of deprecated method `pyo3::deprecations::GilRefs::::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::::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument +error: use of deprecated method `pyo3::deprecations::GilRefs::::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::::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument +error: use of deprecated method `pyo3::deprecations::GilRefs::::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::::extract_from_py_with`: use `&Bound<'_, PyAny>` as the argument for this `from_py_with` extractor +error: use of deprecated method `pyo3::deprecations::GilRefs::::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::>::is_python`: use `wrap_pyfunction_bound!` instead +error: use of deprecated method `pyo3::deprecations::GilRefs::>::is_python`: use `wrap_pyfunction_bound!` instead --> tests/ui/deprecations.rs:94:13 | 94 | let _ = wrap_pyfunction!(double, py);