From 35faeff6f1db5bc4aa672355ae6174f8fe154fc4 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Tue, 26 Mar 2024 19:53:11 +0100 Subject: [PATCH] handle `#[pyo3(from_py_with = "")]` in `#[setter]` methods (#3995) * handle `#[pyo3(from_py_with = "")]` in `#[setter]` methods * add newsfragment --- newsfragments/3995.fixed.md | 1 + pyo3-macros-backend/src/pymethod.rs | 32 ++++++++++++++- tests/test_getter_setter.rs | 12 ++++++ tests/ui/deprecations.rs | 6 +++ tests/ui/deprecations.stderr | 60 ++++++++++++++++------------- 5 files changed, 82 insertions(+), 29 deletions(-) create mode 100644 newsfragments/3995.fixed.md diff --git a/newsfragments/3995.fixed.md b/newsfragments/3995.fixed.md new file mode 100644 index 00000000..e47a71b7 --- /dev/null +++ b/newsfragments/3995.fixed.md @@ -0,0 +1 @@ +handle `#[pyo3(from_py_with = "")]` 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 4e6f46d9..1e476ca2 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -11,7 +11,7 @@ use crate::{ }; use crate::{quotes, utils}; use proc_macro2::{Span, TokenStream}; -use quote::{format_ident, quote, ToTokens}; +use quote::{format_ident, quote, quote_spanned, ToTokens}; use syn::{ext::IdentExt, spanned::Spanned, Result}; /// Generated code for a single pymethod item. @@ -586,6 +586,34 @@ pub fn impl_py_setter_def( } }; + let extract = if let PropertyType::Function { spec, .. } = &property_type { + Some(spec) + } else { + None + } + .and_then(|spec| { + let (_, args) = split_off_python_arg(&spec.signature.arguments); + let value_arg = &args[0]; + let from_py_with = &value_arg.attrs.from_py_with.as_ref()?.value; + let name = value_arg.name.to_string(); + + Some(quote_spanned! { from_py_with.span() => + let e = #pyo3_path::impl_::deprecations::GilRefs::new(); + let from_py_with = #pyo3_path::impl_::deprecations::inspect_fn(#from_py_with, &e); + e.from_py_with_arg(); + let _val = #pyo3_path::impl_::extract_argument::from_py_with( + &_value.into(), + #name, + from_py_with as fn(_) -> _, + )?; + }) + }) + .unwrap_or_else(|| { + quote! { + let _val = #pyo3_path::FromPyObject::extract_bound(_value.into())?; + } + }); + let mut cfg_attrs = TokenStream::new(); if let PropertyType::Descriptor { field, .. } = &property_type { for attr in field @@ -611,7 +639,7 @@ pub fn impl_py_setter_def( .ok_or_else(|| { #pyo3_path::exceptions::PyAttributeError::new_err("can't delete attribute") })?; - let _val = #pyo3_path::FromPyObject::extract_bound(_value.into())?; + #extract #init_holders let result = #setter_impl; #check_gil_refs diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index 1009ce1b..5f886639 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -41,12 +41,21 @@ impl ClassWithProperties { self.num = value; } + #[setter] + fn set_from_len(&mut self, #[pyo3(from_py_with = "extract_len")] value: i32) { + self.num = value; + } + #[getter] fn get_data_list<'py>(&self, py: Python<'py>) -> Bound<'py, PyList> { PyList::new_bound(py, [self.num]) } } +fn extract_len(any: &Bound<'_, PyAny>) -> PyResult { + any.len().map(|len| len as i32) +} + #[test] fn class_with_properties() { Python::with_gil(|py| { @@ -64,6 +73,9 @@ fn class_with_properties() { py_run!(py, inst, "assert inst.get_num() == inst.unwrapped == 42"); py_run!(py, inst, "assert inst.data_list == [42]"); + py_run!(py, inst, "inst.from_len = [0, 0, 0]"); + py_run!(py, inst, "assert inst.get_num() == 3"); + 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 6f3a1fed..0ed4d09e 100644 --- a/tests/ui/deprecations.rs +++ b/tests/ui/deprecations.rs @@ -26,6 +26,12 @@ impl MyClass { #[staticmethod] fn static_method_gil_ref(_any: &PyAny) {} + + #[setter] + fn set_foo_gil_ref(&self, #[pyo3(from_py_with = "extract_gil_ref")] _value: i32) {} + + #[setter] + fn set_foo_bound(&self, #[pyo3(from_py_with = "extract_bound")] _value: i32) {} } fn main() {} diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr index 1d87f31c..e2d9cf36 100644 --- a/tests/ui/deprecations.stderr +++ b/tests/ui/deprecations.stderr @@ -34,70 +34,76 @@ error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg` 28 | fn static_method_gil_ref(_any: &PyAny) {} | ^ -error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument - --> tests/ui/deprecations.rs:41:43 +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:31:53 | -41 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> { +31 | fn set_foo_gil_ref(&self, #[pyo3(from_py_with = "extract_gil_ref")] _value: i32) {} + | ^^^^^^^^^^^^^^^^^ + +error: use of deprecated method `pyo3::deprecations::GilRefs::::function_arg`: use `&Bound<'_, T>` instead for this function argument + --> tests/ui/deprecations.rs:47:43 + | +47 | 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:51:19 + --> tests/ui/deprecations.rs:57:19 | -51 | fn module_gil_ref(m: &PyModule) -> PyResult<()> { +57 | 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:57:57 + --> tests/ui/deprecations.rs:63:57 | -57 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> { +63 | 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:90:27 + --> tests/ui/deprecations.rs:96:27 | -90 | #[pyo3(from_py_with = "extract_gil_ref")] _gil_ref: i32, +96 | #[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:96:29 - | -96 | fn pyfunction_gil_ref(_any: &PyAny) {} - | ^ + --> tests/ui/deprecations.rs:102:29 + | +102 | 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:99:36 - | -99 | fn pyfunction_option_gil_ref(_any: Option<&PyAny>) {} - | ^^^^^^ + --> tests/ui/deprecations.rs:105:36 + | +105 | 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:106:27 + --> tests/ui/deprecations.rs:112:27 | -106 | #[pyo3(from_py_with = "PyAny::len", item("my_object"))] +112 | #[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:116:27 + --> tests/ui/deprecations.rs:122:27 | -116 | #[pyo3(from_py_with = "PyAny::len")] usize, +122 | #[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:122:31 + --> tests/ui/deprecations.rs:128:31 | -122 | Zip(#[pyo3(from_py_with = "extract_gil_ref")] i32), +128 | 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:129:27 + --> tests/ui/deprecations.rs:135:27 | -129 | #[pyo3(from_py_with = "extract_gil_ref")] +135 | #[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:142:13 + --> tests/ui/deprecations.rs:148:13 | -142 | let _ = wrap_pyfunction!(double, py); +148 | 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)