From dd1710256d00ac2538f2fb8068fe63254b9f469c Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 27 Mar 2024 16:41:04 +0100 Subject: [PATCH] use `extract_argument` for `#[setter]` extraction (#3998) * use `extract_argument` for `#[setter]` extraction * add newsfragment --- newsfragments/3998.changed.md | 1 + newsfragments/3998.fixed.md | 1 + pyo3-macros-backend/src/pymethod.rs | 19 ++++++++-- tests/test_getter_setter.rs | 9 +++++ tests/ui/deprecations.rs | 6 ++++ tests/ui/deprecations.stderr | 54 ++++++++++++++++------------- 6 files changed, 64 insertions(+), 26 deletions(-) create mode 100644 newsfragments/3998.changed.md create mode 100644 newsfragments/3998.fixed.md diff --git a/newsfragments/3998.changed.md b/newsfragments/3998.changed.md new file mode 100644 index 00000000..c02c6546 --- /dev/null +++ b/newsfragments/3998.changed.md @@ -0,0 +1 @@ +Warn on uses of GIL Refs for `#[setter]` function arguments. \ No newline at end of file diff --git a/newsfragments/3998.fixed.md b/newsfragments/3998.fixed.md new file mode 100644 index 00000000..11f5d006 --- /dev/null +++ b/newsfragments/3998.fixed.md @@ -0,0 +1 @@ +Allow extraction of `&Bound` in `#[setter]` methods. \ No newline at end of file diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 1e476ca2..22802a01 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -586,6 +586,8 @@ pub fn impl_py_setter_def( } }; + // TODO: rework this to make use of `impl_::params::impl_arg_param` which + // handles all these cases already. let extract = if let PropertyType::Function { spec, .. } = &property_type { Some(spec) } else { @@ -609,8 +611,21 @@ pub fn impl_py_setter_def( }) }) .unwrap_or_else(|| { + let (span, name) = match &property_type { + PropertyType::Descriptor { field, .. } => (field.ty.span(), field.ident.as_ref().map(|i|i.to_string()).unwrap_or_default()), + PropertyType::Function { spec, .. } => { + let (_, args) = split_off_python_arg(&spec.signature.arguments); + (args[0].ty.span(), args[0].name.to_string()) + } + }; + + let holder = holders.push_holder(span); + let gil_refs_checker = holders.push_gil_refs_checker(span); quote! { - let _val = #pyo3_path::FromPyObject::extract_bound(_value.into())?; + let _val = #pyo3_path::impl_::deprecations::inspect_type( + #pyo3_path::impl_::extract_argument::extract_argument(_value.into(), &mut #holder, #name)?, + &#gil_refs_checker + ); } }); @@ -639,8 +654,8 @@ pub fn impl_py_setter_def( .ok_or_else(|| { #pyo3_path::exceptions::PyAttributeError::new_err("can't delete attribute") })?; - #extract #init_holders + #extract let result = #setter_impl; #check_gil_refs #pyo3_path::callback::convert(py, result) diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index 5f886639..b0b15d78 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -46,6 +46,12 @@ impl ClassWithProperties { self.num = value; } + #[setter] + fn set_from_any(&mut self, value: &Bound<'_, PyAny>) -> PyResult<()> { + self.num = value.extract()?; + Ok(()) + } + #[getter] fn get_data_list<'py>(&self, py: Python<'py>) -> Bound<'py, PyList> { PyList::new_bound(py, [self.num]) @@ -76,6 +82,9 @@ fn class_with_properties() { py_run!(py, inst, "inst.from_len = [0, 0, 0]"); py_run!(py, inst, "assert inst.get_num() == 3"); + py_run!(py, inst, "inst.from_any = 15"); + py_run!(py, inst, "assert inst.get_num() == 15"); + let d = [("C", py.get_type_bound::())].into_py_dict_bound(py); py_assert!(py, *d, "C.DATA.__doc__ == 'a getter for data'"); }); diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs index 0ed4d09e..dcc9b7b1 100644 --- a/tests/ui/deprecations.rs +++ b/tests/ui/deprecations.rs @@ -32,6 +32,12 @@ impl MyClass { #[setter] fn set_foo_bound(&self, #[pyo3(from_py_with = "extract_bound")] _value: i32) {} + + #[setter] + fn set_bar_gil_ref(&self, _value: &PyAny) {} + + #[setter] + fn set_bar_bound(&self, _value: &Bound<'_, PyAny>) {} } fn main() {} diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index e2d9cf36..e692702f 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -41,69 +41,75 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::::from_py_with_ | ^^^^^^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:47:43 + --> tests/ui/deprecations.rs:37:39 | -47 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> { +37 | fn set_bar_gil_ref(&self, _value: &PyAny) {} + | ^ + +error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument + --> tests/ui/deprecations.rs:53:43 + | +53 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> { | ^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:57:19 + --> tests/ui/deprecations.rs:63:19 | -57 | fn module_gil_ref(m: &PyModule) -> PyResult<()> { +63 | fn module_gil_ref(m: &PyModule) -> PyResult<()> { | ^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:63:57 + --> tests/ui/deprecations.rs:69:57 | -63 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> { +69 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> { | ^ 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:96:27 - | -96 | #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32, - | ^^^^^^^^^^^^^^^^^ + --> tests/ui/deprecations.rs:102:27 + | +102 | #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32, + | ^^^^^^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:102:29 + --> tests/ui/deprecations.rs:108:29 | -102 | fn pyfunction_gil_ref(_any: &PyAny) {} +108 | fn pyfunction_gil_ref(_any: &PyAny) {} | ^ error: use of deprecated method `pyo3::deprecations::OptionGilRefs::>::function_arg`: use `Option<&Bound<'_, T>>` instead for this function argument - --> tests/ui/deprecations.rs:105:36 + --> tests/ui/deprecations.rs:111:36 | -105 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {} +111 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {} | ^^^^^^ 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:112:27 + --> tests/ui/deprecations.rs:118:27 | -112 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))] +118 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))] | ^^^^^^^^^^^^ 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:122:27 + --> tests/ui/deprecations.rs:128:27 | -122 | #[pyo3(from_py_with = "PyAny::len")] usize, +128 | #[pyo3(from_py_with = "PyAny::len")] usize, | ^^^^^^^^^^^^ 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:128:31 + --> tests/ui/deprecations.rs:134:31 | -128 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32), +134 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32), | ^^^^^^^^^^^^^^^^^ 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:135:27 + --> tests/ui/deprecations.rs:141:27 | -135 | #[pyo3(from_py_with = "extract_gil_ref")] +141 | #[pyo3(from_py_with = "extract_gil_ref")] | ^^^^^^^^^^^^^^^^^ error: use of deprecated method `pyo3::deprecations::GilRefs::>::is_python`: use `wrap_pyfunction_bound!` instead - --> tests/ui/deprecations.rs:148:13 + --> tests/ui/deprecations.rs:154:13 | -148 | let _ = wrap_pyfunction!(double, py); +154 | 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)