deprecate optional GIL Ref in function argument (#3975)

This commit is contained in:
David Hewitt 2024-03-21 07:24:40 +00:00 committed by GitHub
parent 870a4bb20d
commit 351c6a0a49
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 50 additions and 21 deletions

View File

@ -301,7 +301,7 @@ The expectation is that in 0.22 `extract_bound` will have the default implementa
Despite a large amount of deprecations warnings produced by PyO3 to aid with the transition from GIL Refs to the Bound API, there are a few cases where PyO3 cannot automatically warn on uses of GIL Refs. It is worth checking for these cases manually after the deprecation warnings have all been addressed:
- Individual implementations of the `FromPyObject` trait cannot be deprecated, so PyO3 cannot warn about uses of code patterns like `.extract<&PyAny>()` which produce a GIL Ref.
- GIL Refs in `#[pyfunction]` arguments emit a warning, but if the GIL Ref is wrapped inside another container such as `Option<&PyAny>` or `Vec<&PyAny>` then PyO3 cannot warn against this.
- GIL Refs in `#[pyfunction]` arguments emit a warning, but if the GIL Ref is wrapped inside another container such as `Vec<&PyAny>` then PyO3 cannot warn against this.
- The `wrap_pyfunction!(function)(py)` deferred argument form of the `wrap_pyfunction` macro taking `py: Python<'py>` produces a GIL Ref, and due to limitations in type inference PyO3 cannot warn against this specific case.
</details>

View File

@ -292,7 +292,7 @@ impl<Tz: TimeZone + for<'py> FromPyObject<'py>> FromPyObject<'_> for DateTime<Tz
#[cfg(not(Py_LIMITED_API))]
let tzinfo = dt.get_tzinfo_bound();
#[cfg(Py_LIMITED_API)]
let tzinfo: Option<&PyAny> = dt.getattr(intern!(dt.py(), "tzinfo"))?.extract()?;
let tzinfo: Option<Bound<'_, PyAny>> = dt.getattr(intern!(dt.py(), "tzinfo"))?.extract()?;
let tz = if let Some(tzinfo) = tzinfo {
tzinfo.extract()?

View File

@ -13,7 +13,8 @@ pub fn inspect_fn<A, T>(f: fn(A) -> PyResult<T>, _: &GilRefs<A>) -> fn(A) -> PyR
f
}
pub struct GilRefs<T>(NotAGilRef<T>);
pub struct GilRefs<T>(OptionGilRefs<T>);
pub struct OptionGilRefs<T>(NotAGilRef<T>);
pub struct NotAGilRef<T>(std::marker::PhantomData<T>);
pub trait IsGilRef {}
@ -23,7 +24,7 @@ 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))
GilRefs(OptionGilRefs(NotAGilRef(std::marker::PhantomData)))
}
}
@ -54,6 +55,17 @@ impl<T: IsGilRef> GilRefs<T> {
pub fn from_py_with_arg(&self) {}
}
impl<T: IsGilRef> OptionGilRefs<Option<T>> {
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Option<&Bound<'_, T>>` instead for this function argument"
)
)]
pub fn function_arg(&self) {}
}
impl<T> NotAGilRef<T> {
pub fn function_arg(&self) {}
pub fn from_py_with_arg(&self) {}
@ -61,6 +73,13 @@ impl<T> NotAGilRef<T> {
}
impl<T> std::ops::Deref for GilRefs<T> {
type Target = OptionGilRefs<T>;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl<T> std::ops::Deref for OptionGilRefs<T> {
type Target = NotAGilRef<T>;
fn deref(&self) -> &Self::Target {
&self.0

View File

@ -289,7 +289,7 @@ impl RhsArithmetic {
format!("{:?} | RA", other)
}
fn __rpow__(&self, other: &Bound<'_, PyAny>, _mod: Option<&PyAny>) -> String {
fn __rpow__(&self, other: &Bound<'_, PyAny>, _mod: Option<&Bound<'_, PyAny>>) -> String {
format!("{:?} ** RA", other)
}
}
@ -406,7 +406,7 @@ impl LhsAndRhs {
format!("{:?} | RA", other)
}
fn __rpow__(&self, other: &Bound<'_, PyAny>, _mod: Option<&PyAny>) -> String {
fn __rpow__(&self, other: &Bound<'_, PyAny>, _mod: Option<&Bound<'_, PyAny>>) -> String {
format!("{:?} ** RA", other)
}

View File

@ -21,11 +21,11 @@ struct Mapping {
#[pymethods]
impl Mapping {
#[new]
fn new(elements: Option<&PyList>) -> PyResult<Self> {
fn new(elements: Option<&Bound<'_, PyList>>) -> PyResult<Self> {
if let Some(pylist) = elements {
let mut elems = HashMap::with_capacity(pylist.len());
for (i, pyelem) in pylist.into_iter().enumerate() {
let elem = String::extract(pyelem)?;
let elem = pyelem.extract()?;
elems.insert(elem, i);
}
Ok(Self { index: elems })

View File

@ -2,6 +2,7 @@
use pyo3::prelude::*;
use pyo3::py_run;
use pyo3::types::PySequence;
use pyo3::types::{IntoPyDict, PyDict, PyList, PySet, PyString, PyTuple, PyType};
#[path = "../src/tests/common.rs"]
@ -857,7 +858,7 @@ struct FromSequence {
#[pymethods]
impl FromSequence {
#[new]
fn new(seq: Option<&pyo3::types::PySequence>) -> PyResult<Self> {
fn new(seq: Option<&Bound<'_, PySequence>>) -> PyResult<Self> {
if let Some(seq) = seq {
Ok(FromSequence {
numbers: seq.as_ref().extract::<Vec<_>>()?,

View File

@ -17,11 +17,11 @@ struct ByteSequence {
#[pymethods]
impl ByteSequence {
#[new]
fn new(elements: Option<&PyList>) -> PyResult<Self> {
fn new(elements: Option<&Bound<'_, PyList>>) -> PyResult<Self> {
if let Some(pylist) = elements {
let mut elems = Vec::with_capacity(pylist.len());
for pyelem in pylist {
let elem = u8::extract(pyelem)?;
let elem = pyelem.extract()?;
elems.push(elem);
}
Ok(Self { elements: elems })

View File

@ -95,6 +95,9 @@ fn pyfunction_from_py_with(
#[pyfunction]
fn pyfunction_gil_ref(_any: &PyAny) {}
#[pyfunction]
fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {}
#[derive(Debug, FromPyObject)]
pub struct Zap {
#[pyo3(item)]

View File

@ -64,34 +64,40 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::<T>::function_arg`
96 | fn pyfunction_gil_ref(_any: &PyAny) {}
| ^
error: use of deprecated method `pyo3::deprecations::OptionGilRefs::<std::option::Option<T>>::function_arg`: use `Option<&Bound<'_, T>>` instead for this function argument
--> tests/ui/deprecations.rs:99:36
|
99 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {}
| ^^^^^^
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:103:27
--> tests/ui/deprecations.rs:106:27
|
103 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))]
106 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))]
| ^^^^^^^^^^^^
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:113:27
--> tests/ui/deprecations.rs:116:27
|
113 | #[pyo3(from_py_with = "PyAny::len")] usize,
116 | #[pyo3(from_py_with = "PyAny::len")] usize,
| ^^^^^^^^^^^^
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:119:31
--> tests/ui/deprecations.rs:122:31
|
119 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32),
122 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32),
| ^^^^^^^^^^^^^^^^^
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:126:27
--> tests/ui/deprecations.rs:129:27
|
126 | #[pyo3(from_py_with = "extract_gil_ref")]
129 | #[pyo3(from_py_with = "extract_gil_ref")]
| ^^^^^^^^^^^^^^^^^
error: use of deprecated method `pyo3::deprecations::GilRefs::<pyo3::Python<'_>>::is_python`: use `wrap_pyfunction_bound!` instead
--> tests/ui/deprecations.rs:139:13
--> tests/ui/deprecations.rs:142:13
|
139 | let _ = wrap_pyfunction!(double, py);
142 | let _ = wrap_pyfunction!(double, py);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this error originates in the macro `wrap_pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)