From 477fa6f67f53a6485c5874008186706418ff12ae Mon Sep 17 00:00:00 2001 From: jrudolph Date: Tue, 28 May 2024 15:57:55 -0500 Subject: [PATCH 01/13] Added `PyRef::as_super` and `PyRefMut::as_super` methods, including docstrings and tests. The implementation of these methods also required adding `#[repr(transparent)]` to the `PyRef` and `PyRefMut` structs. --- src/pycell.rs | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/src/pycell.rs b/src/pycell.rs index f15f5a54431..e394d74ec7c 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -611,6 +611,7 @@ impl fmt::Debug for PyCell { /// ``` /// /// See the [module-level documentation](self) for more information. +#[repr(transparent)] pub struct PyRef<'p, T: PyClass> { // TODO: once the GIL Ref API is removed, consider adding a lifetime parameter to `PyRef` to // store `Borrowed` here instead, avoiding reference counting overhead. @@ -742,6 +743,56 @@ where }, } } + /// Borrows a shared reference to `PyRef`. + /// + /// With the help of this method, you can access attributes and call methods + /// on the superclass without consuming the `PyRef`. + /// + /// # Examples + /// ``` + /// # use pyo3::prelude::*; + /// #[pyclass(subclass)] + /// struct Base { + /// base_name: &'static str, + /// } + /// #[pymethods] + /// impl Base { + /// fn base_name_len(&self) -> usize { + /// self.base_name.len() + /// } + /// } + /// + /// #[pyclass(extends=Base)] + /// struct Sub { + /// sub_name: &'static str, + /// } + /// + /// #[pymethods] + /// impl Sub { + /// #[new] + /// fn new() -> (Self, Base) { + /// (Self { sub_name: "sub_name" }, Base { base_name: "base_name" }) + /// } + /// fn sub_name_len(&self) -> usize { + /// self.sub_name.len() + /// } + /// fn format_name_lengths(slf: PyRef<'_, Self>) -> String { + /// format!("{} {}", slf.as_super().base_name_len(), slf.sub_name_len()) + /// } + /// } + /// # Python::with_gil(|py| { + /// # let sub = Py::new(py, Sub::new()).unwrap(); + /// # pyo3::py_run!(py, sub, "assert sub.format_name_lengths() == '9 8'") + /// # }); + /// ``` + pub fn as_super(&self) -> &PyRef<'p, U> { + unsafe { + // trait bound guarantees compatibility + let inner: &Bound<'p, U> = self.inner.as_any().downcast_unchecked(); + // `repr(transparent)` guarantees `Bound` has the same layout as `PyRef` + &*(inner as *const Bound<'p, U> as *const PyRef<'p, U>) + } + } } impl<'p, T: PyClass> Deref for PyRef<'p, T> { @@ -798,6 +849,7 @@ impl fmt::Debug for PyRef<'_, T> { /// A wrapper type for a mutably borrowed value from a [`Bound<'py, T>`]. /// /// See the [module-level documentation](self) for more information. +#[repr(transparent)] pub struct PyRefMut<'p, T: PyClass> { // TODO: once the GIL Ref API is removed, consider adding a lifetime parameter to `PyRef` to // store `Borrowed` here instead, avoiding reference counting overhead. @@ -890,6 +942,21 @@ where }, } } + /// Borrows a mutable reference to `PyRefMut`. + /// + /// With the help of this method, you can mutate attributes and call mutating + /// methods on the superclass without consuming the `PyRefMet`. + /// + /// See [`PyRefMut::as_super`] for more. + pub fn as_super(&mut self) -> &mut PyRefMut<'p, U> { + unsafe { + // trait bound guarantees compatibility + let inner = &mut self.inner as *mut Bound<'p, T> as *mut Bound<'p, U>; + // `repr(transparent)` guarantees `Bound` has the same layout as + // `PyRefMut`, and the mutable borrow on `self` prevent aliasing + &mut *(inner as *mut PyRefMut<'p, U>) + } + } } impl<'p, T: PyClass> Deref for PyRefMut<'p, T> { @@ -1139,4 +1206,53 @@ mod tests { unsafe { ffi::Py_DECREF(ptr) }; }) } + + #[crate::pyclass] + #[pyo3(crate = "crate", subclass)] + struct Base { + base_name: &'static str, + } + + #[crate::pyclass] + #[pyo3(crate = "crate", extends=Base)] + struct Sub { + sub_name: &'static str, + } + + #[crate::pymethods] + #[pyo3(crate = "crate")] + impl Sub { + #[new] + fn new(py: Python<'_>) -> crate::PyResult> { + let slf = Self { + sub_name: "sub_name", + }; + let spr = Base { + base_name: "base_name", + }; + crate::Py::new(py, (slf, spr)) + } + } + + #[test] + fn test_pyref_as_super() { + Python::with_gil(|py| { + let sub = Sub::new(py).unwrap().into_bound(py); + let subref = sub.borrow(); + assert_eq!(subref.as_super().base_name, "base_name"); + assert_eq!(subref.sub_name, "sub_name"); + }); + } + + #[test] + fn test_pyrefmut_as_super() { + Python::with_gil(|py| { + let sub = Sub::new(py).unwrap().into_bound(py); + let mut subref = sub.borrow_mut(); + assert_eq!(subref.as_super().base_name, "base_name"); + subref.as_super().base_name = "modified_name"; + assert_eq!(subref.as_super().base_name, "modified_name"); + assert_eq!(subref.sub_name, "sub_name"); + }); + } } From 6d27b2d4471545f302f1543b48c931a295028bc1 Mon Sep 17 00:00:00 2001 From: jrudolph Date: Tue, 28 May 2024 16:00:37 -0500 Subject: [PATCH 02/13] Added newsfragment entry. --- newsfragments/4215.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4215.added.md diff --git a/newsfragments/4215.added.md b/newsfragments/4215.added.md new file mode 100644 index 00000000000..48a7369951b --- /dev/null +++ b/newsfragments/4215.added.md @@ -0,0 +1 @@ +Added `as_super` methods to `PyRef` and `PyRefMut`. \ No newline at end of file From 42d7ca921f673b9a9d809ea11d05524852a7f46b Mon Sep 17 00:00:00 2001 From: jrudolph Date: Tue, 28 May 2024 18:10:23 -0500 Subject: [PATCH 03/13] Changed the `AsRef`/`AsMut` impls for `PyRef` and `PyRefMut` to use the new `as_super` methods. Added the `PyRefMut::downgrade` associated function for converting `&PyRefMut` to `&PyRef`. Updated tests and docstrings to better demonstrate the new functionality. --- src/pycell.rs | 76 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 27 deletions(-) diff --git a/src/pycell.rs b/src/pycell.rs index e394d74ec7c..6cd7cf99f22 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -631,7 +631,7 @@ where U: PyClass, { fn as_ref(&self) -> &T::BaseType { - unsafe { &*self.inner.get_class_object().ob_base.get_ptr() } + &*self.as_super() } } @@ -746,7 +746,8 @@ where /// Borrows a shared reference to `PyRef`. /// /// With the help of this method, you can access attributes and call methods - /// on the superclass without consuming the `PyRef`. + /// on the superclass without consuming the `PyRef`. This method can also + /// be chained to access the super-superclass (and so on). /// /// # Examples /// ``` @@ -869,7 +870,7 @@ where U: PyClass, { fn as_ref(&self) -> &T::BaseType { - unsafe { &*self.inner.get_class_object().ob_base.get_ptr() } + &*PyRefMut::downgrade(self).as_super() } } @@ -879,7 +880,7 @@ where U: PyClass, { fn as_mut(&mut self) -> &mut T::BaseType { - unsafe { &mut *self.inner.get_class_object().ob_base.get_ptr() } + &mut *self.as_super() } } @@ -921,6 +922,12 @@ impl<'py, T: PyClass> PyRefMut<'py, T> { .try_borrow_mut() .map(|_| Self { inner: obj.clone() }) } + + pub(crate) fn downgrade(slf: &Self) -> &PyRef<'py, T> { + // `PyRef` and `PyRefMut` have the same layout, and they are behind + // a reference so the difference in `Drop` behavior is irrelevant. + unsafe { &*(slf as *const Self as *const PyRef<'py, T>) } + } } impl<'p, T, U> PyRefMut<'p, T> @@ -945,15 +952,16 @@ where /// Borrows a mutable reference to `PyRefMut`. /// /// With the help of this method, you can mutate attributes and call mutating - /// methods on the superclass without consuming the `PyRefMet`. + /// methods on the superclass without consuming the `PyRefMet`. This method + /// can also be chained to access the super-superclass (and so on). /// - /// See [`PyRefMut::as_super`] for more. + /// See [`PyRef::as_super`] for more. pub fn as_super(&mut self) -> &mut PyRefMut<'p, U> { unsafe { // trait bound guarantees compatibility let inner = &mut self.inner as *mut Bound<'p, T> as *mut Bound<'p, U>; // `repr(transparent)` guarantees `Bound` has the same layout as - // `PyRefMut`, and the mutable borrow on `self` prevent aliasing + // `PyRefMut`, and the mutable borrow on `self` prevents aliasing &mut *(inner as *mut PyRefMut<'p, U>) } } @@ -1214,45 +1222,59 @@ mod tests { } #[crate::pyclass] - #[pyo3(crate = "crate", extends=Base)] + #[pyo3(crate = "crate", extends=Base, subclass)] struct Sub { sub_name: &'static str, } + #[crate::pyclass] + #[pyo3(crate = "crate", extends=Sub)] + struct SubSub { + subsub_name: &'static str, + } + #[crate::pymethods] #[pyo3(crate = "crate")] - impl Sub { + impl SubSub { #[new] - fn new(py: Python<'_>) -> crate::PyResult> { - let slf = Self { - sub_name: "sub_name", - }; - let spr = Base { - base_name: "base_name", - }; - crate::Py::new(py, (slf, spr)) + #[rustfmt::skip] + fn new(py: Python<'_>) -> crate::PyResult> { + let base = Base { base_name: "base_name" }; + let sub = Sub { sub_name: "sub_name" }; + let subsub = SubSub { subsub_name: "subsub_name"}; + let init = crate::PyClassInitializer::from(base) + .add_subclass(sub).add_subclass(subsub); + crate::Py::new(py, init) } } #[test] fn test_pyref_as_super() { Python::with_gil(|py| { - let sub = Sub::new(py).unwrap().into_bound(py); - let subref = sub.borrow(); - assert_eq!(subref.as_super().base_name, "base_name"); - assert_eq!(subref.sub_name, "sub_name"); + let subsub = SubSub::new(py).unwrap().into_bound(py); + let pyref = subsub.borrow(); + assert_eq!(pyref.as_super().as_super().base_name, "base_name"); + assert_eq!(pyref.as_super().sub_name, "sub_name"); + assert_eq!(pyref.subsub_name, "subsub_name"); + // `as_ref` still works the same + assert_eq!(pyref.as_ref().sub_name, "sub_name"); }); } #[test] fn test_pyrefmut_as_super() { Python::with_gil(|py| { - let sub = Sub::new(py).unwrap().into_bound(py); - let mut subref = sub.borrow_mut(); - assert_eq!(subref.as_super().base_name, "base_name"); - subref.as_super().base_name = "modified_name"; - assert_eq!(subref.as_super().base_name, "modified_name"); - assert_eq!(subref.sub_name, "sub_name"); + let subsub = SubSub::new(py).unwrap().into_bound(py); + let mut pyrefmut = subsub.borrow_mut(); + pyrefmut.as_super().as_super().base_name = "base_name2"; + pyrefmut.as_super().sub_name = "sub_name2"; + pyrefmut.subsub_name = "subsub_name2"; + assert_eq!(pyrefmut.as_super().as_super().base_name, "base_name2"); + assert_eq!(pyrefmut.as_super().sub_name, "sub_name2"); + assert_eq!(pyrefmut.subsub_name, "subsub_name2"); + // `as_ref`/`as_mut` still work the same + pyrefmut.as_mut().sub_name = "sub_name3"; + assert_eq!(pyrefmut.as_ref().sub_name, "sub_name3"); }); } } From 189d92ed160f8e9ddd7815f4faa6810ae67135e2 Mon Sep 17 00:00:00 2001 From: jrudolph Date: Tue, 28 May 2024 20:16:54 -0500 Subject: [PATCH 04/13] Fixed newsfragment filename. --- newsfragments/{4215.added.md => 4219.added.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{4215.added.md => 4219.added.md} (100%) diff --git a/newsfragments/4215.added.md b/newsfragments/4219.added.md similarity index 100% rename from newsfragments/4215.added.md rename to newsfragments/4219.added.md From 6596cd60bc8f718b545e2199449e96839b288a2d Mon Sep 17 00:00:00 2001 From: jrudolph Date: Tue, 28 May 2024 21:41:43 -0500 Subject: [PATCH 05/13] Removed unnecessary re-borrows flagged by clippy. --- src/pycell.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pycell.rs b/src/pycell.rs index 6cd7cf99f22..d8559cbc202 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -631,7 +631,7 @@ where U: PyClass, { fn as_ref(&self) -> &T::BaseType { - &*self.as_super() + self.as_super() } } @@ -870,7 +870,7 @@ where U: PyClass, { fn as_ref(&self) -> &T::BaseType { - &*PyRefMut::downgrade(self).as_super() + PyRefMut::downgrade(self).as_super() } } @@ -880,7 +880,7 @@ where U: PyClass, { fn as_mut(&mut self) -> &mut T::BaseType { - &mut *self.as_super() + self.as_super() } } From 8c44d28914975766ad00c8f6ef58958efd5b97af Mon Sep 17 00:00:00 2001 From: jrudolph Date: Tue, 28 May 2024 23:59:56 -0500 Subject: [PATCH 06/13] retrigger checks From 23c7836f6eaf9cb776f577544d1772725043c329 Mon Sep 17 00:00:00 2001 From: jrudolph Date: Sat, 8 Jun 2024 09:46:48 -0500 Subject: [PATCH 07/13] Updated `PyRef::as_super`, `PyRefMut::as_super`, and `PyRefMut::downgrade` to use `.cast()` instead of `as _` pointer casts. Fixed typo. --- src/pycell.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/pycell.rs b/src/pycell.rs index d8559cbc202..44ea59e124d 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -743,6 +743,7 @@ where }, } } + /// Borrows a shared reference to `PyRef`. /// /// With the help of this method, you can access attributes and call methods @@ -787,12 +788,12 @@ where /// # }); /// ``` pub fn as_super(&self) -> &PyRef<'p, U> { - unsafe { - // trait bound guarantees compatibility - let inner: &Bound<'p, U> = self.inner.as_any().downcast_unchecked(); - // `repr(transparent)` guarantees `Bound` has the same layout as `PyRef` - &*(inner as *const Bound<'p, U> as *const PyRef<'p, U>) - } + let ptr = (&self.inner as *const Bound<'p, T>) + // `Bound` has the same layout as `Bound` + .cast::>() + // `Bound` has the same layout as `PyRef` + .cast::>(); + unsafe { &*ptr } } } @@ -926,7 +927,7 @@ impl<'py, T: PyClass> PyRefMut<'py, T> { pub(crate) fn downgrade(slf: &Self) -> &PyRef<'py, T> { // `PyRef` and `PyRefMut` have the same layout, and they are behind // a reference so the difference in `Drop` behavior is irrelevant. - unsafe { &*(slf as *const Self as *const PyRef<'py, T>) } + unsafe { &*(slf as *const Self).cast::>() } } } @@ -949,21 +950,22 @@ where }, } } + /// Borrows a mutable reference to `PyRefMut`. /// /// With the help of this method, you can mutate attributes and call mutating - /// methods on the superclass without consuming the `PyRefMet`. This method + /// methods on the superclass without consuming the `PyRefMut`. This method /// can also be chained to access the super-superclass (and so on). /// /// See [`PyRef::as_super`] for more. pub fn as_super(&mut self) -> &mut PyRefMut<'p, U> { - unsafe { - // trait bound guarantees compatibility - let inner = &mut self.inner as *mut Bound<'p, T> as *mut Bound<'p, U>; - // `repr(transparent)` guarantees `Bound` has the same layout as - // `PyRefMut`, and the mutable borrow on `self` prevents aliasing - &mut *(inner as *mut PyRefMut<'p, U>) - } + let ptr = (&mut self.inner as *mut Bound<'p, T>) + // `Bound` has the same layout as `Bound` + .cast::>() + // `Bound` has the same layout as `PyRefMut`, + // and the mutable borrow on `self` prevents aliasing + .cast::>(); + unsafe { &mut *ptr } } } From 4e345f31c18c6b176efbe868ab93b0977c853a49 Mon Sep 17 00:00:00 2001 From: jrudolph Date: Sat, 8 Jun 2024 10:32:39 -0500 Subject: [PATCH 08/13] Updated `PyRef::as_super` and `PyRefMut::downgrade` to use `ptr_from_ref` for the initial cast to `*const _` instead of `as _` casts. --- src/pycell.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/pycell.rs b/src/pycell.rs index fb4d2b82832..8d4636af2b7 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -196,13 +196,13 @@ use crate::conversion::AsPyPointer; use crate::exceptions::PyRuntimeError; use crate::ffi_ptr_ext::FfiPtrExt; +use crate::internal_tricks::ptr_from_ref; use crate::pyclass::{boolean_struct::False, PyClass}; use crate::types::any::PyAnyMethods; #[cfg(feature = "gil-refs")] use crate::{ conversion::ToPyObject, impl_::pyclass::PyClassImpl, - internal_tricks::ptr_from_ref, pyclass::boolean_struct::True, pyclass_init::PyClassInitializer, type_object::{PyLayout, PySizedLayout}, @@ -789,7 +789,7 @@ where /// # }); /// ``` pub fn as_super(&self) -> &PyRef<'p, U> { - let ptr = (&self.inner as *const Bound<'p, T>) + let ptr = ptr_from_ref::>(&self.inner) // `Bound` has the same layout as `Bound` .cast::>() // `Bound` has the same layout as `PyRef` @@ -926,9 +926,8 @@ impl<'py, T: PyClass> PyRefMut<'py, T> { } pub(crate) fn downgrade(slf: &Self) -> &PyRef<'py, T> { - // `PyRef` and `PyRefMut` have the same layout, and they are behind - // a reference so the difference in `Drop` behavior is irrelevant. - unsafe { &*(slf as *const Self).cast::>() } + // `PyRefMut` and `PyRef` have the same layout + unsafe { &*ptr_from_ref(slf).cast() } } } From 9c32d77f2d2f2ec1562bcf56a89f17a3cf3fb5cf Mon Sep 17 00:00:00 2001 From: jrudolph Date: Sat, 8 Jun 2024 11:43:55 -0500 Subject: [PATCH 09/13] Added `pyo3::internal_tricks::ptr_from_mut` function alongside the `ptr_from_ref` added in PR #4240. Updated `PyRefMut::as_super` to use this method instead of `as *mut _`. --- src/internal_tricks.rs | 6 ++++++ src/pycell.rs | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/internal_tricks.rs b/src/internal_tricks.rs index 62ec0d02166..a8873dda007 100644 --- a/src/internal_tricks.rs +++ b/src/internal_tricks.rs @@ -223,3 +223,9 @@ pub(crate) fn extract_c_string( pub(crate) const fn ptr_from_ref(t: &T) -> *const T { t as *const T } + +// TODO: use ptr::from_mut on MSRV 1.76 +#[inline] +pub(crate) fn ptr_from_mut(t: &mut T) -> *mut T { + t as *mut T +} diff --git a/src/pycell.rs b/src/pycell.rs index 8d4636af2b7..88be4fe78f9 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -196,7 +196,7 @@ use crate::conversion::AsPyPointer; use crate::exceptions::PyRuntimeError; use crate::ffi_ptr_ext::FfiPtrExt; -use crate::internal_tricks::ptr_from_ref; +use crate::internal_tricks::{ptr_from_mut, ptr_from_ref}; use crate::pyclass::{boolean_struct::False, PyClass}; use crate::types::any::PyAnyMethods; #[cfg(feature = "gil-refs")] @@ -959,7 +959,7 @@ where /// /// See [`PyRef::as_super`] for more. pub fn as_super(&mut self) -> &mut PyRefMut<'p, U> { - let ptr = (&mut self.inner as *mut Bound<'p, T>) + let ptr = ptr_from_mut::>(&mut self.inner) // `Bound` has the same layout as `Bound` .cast::>() // `Bound` has the same layout as `PyRefMut`, From 1d3e41a9e1e4a0897bd880138164a6bd6058b37c Mon Sep 17 00:00:00 2001 From: jrudolph Date: Sat, 8 Jun 2024 14:50:32 -0500 Subject: [PATCH 10/13] Updated the user guide to recommend `as_super` for accessing the base class instead of `as_ref`, and updated the subsequent example/doctest to demonstrate this functionality. --- guide/src/class.md | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index 2bcfe75911e..ab0c82fc88b 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -327,8 +327,12 @@ explicitly. To get a parent class from a child, use [`PyRef`] instead of `&self` for methods, or [`PyRefMut`] instead of `&mut self`. -Then you can access a parent class by `self_.as_ref()` as `&Self::BaseClass`, -or by `self_.into_super()` as `PyRef`. +Then you can access a parent class by `self_.as_super()` as `&PyRef`, +or by `self_.into_super()` as `PyRef` (and similar for the `PyRefMut` +case). For convenience, `self_.as_ref()` can also be used to get `&Self::BaseClass` +directly; however, this approach does not let you access base clases higher in the +inheritance hierarchy, for which you would need to chain multiple `as_super` or +`into_super` calls. ```rust # use pyo3::prelude::*; @@ -345,7 +349,7 @@ impl BaseClass { BaseClass { val1: 10 } } - pub fn method(&self) -> PyResult { + pub fn method1(&self) -> PyResult { Ok(self.val1) } } @@ -363,8 +367,8 @@ impl SubClass { } fn method2(self_: PyRef<'_, Self>) -> PyResult { - let super_ = self_.as_ref(); // Get &BaseClass - super_.method().map(|x| x * self_.val2) + let super_ = self_.as_super(); // Get &PyRef + super_.method1().map(|x| x * self_.val2) } } @@ -381,11 +385,28 @@ impl SubSubClass { } fn method3(self_: PyRef<'_, Self>) -> PyResult { + let base = self_.as_super().as_super(); // Get &PyRef<'_, BaseClass> + base.method1().map(|x| x * self_.val3) + } + + fn method4(self_: PyRef<'_, Self>) -> PyResult { let v = self_.val3; let super_ = self_.into_super(); // Get PyRef<'_, SubClass> SubClass::method2(super_).map(|x| x * v) } + fn get_values(self_: PyRef<'_, Self>) -> (usize, usize, usize) { + let val1 = self_.as_super().as_super().val1; + let val2 = self_.as_super().val2; + (val1, val2, self_.val3) + } + + fn double_values(mut self_: PyRefMut<'_, Self>) { + self_.as_super().as_super().val1 *= 2; + self_.as_super().val2 *= 2; + self_.val3 *= 2; + } + #[staticmethod] fn factory_method(py: Python<'_>, val: usize) -> PyResult { let base = PyClassInitializer::from(BaseClass::new()); @@ -400,7 +421,13 @@ impl SubSubClass { } # Python::with_gil(|py| { # let subsub = pyo3::Py::new(py, SubSubClass::new()).unwrap(); -# pyo3::py_run!(py, subsub, "assert subsub.method3() == 3000"); +# pyo3::py_run!(py, subsub, "assert subsub.method1() == 10"); +# pyo3::py_run!(py, subsub, "assert subsub.method2() == 150"); +# pyo3::py_run!(py, subsub, "assert subsub.method3() == 200"); +# pyo3::py_run!(py, subsub, "assert subsub.method4() == 3000"); +# pyo3::py_run!(py, subsub, "assert subsub.get_values() == (10, 15, 20)"); +# pyo3::py_run!(py, subsub, "assert subsub.double_values() == None"); +# pyo3::py_run!(py, subsub, "assert subsub.get_values() == (20, 30, 40)"); # let subsub = SubSubClass::factory_method(py, 2).unwrap(); # let subsubsub = SubSubClass::factory_method(py, 3).unwrap(); # let cls = py.get_type_bound::(); From bf4e87216815e36a9099c8d756d6421f8cf6734c Mon Sep 17 00:00:00 2001 From: jrudolph Date: Sat, 8 Jun 2024 14:58:39 -0500 Subject: [PATCH 11/13] Improved tests for the `as_super` methods. --- src/pycell.rs | 91 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/src/pycell.rs b/src/pycell.rs index 88be4fe78f9..864399984da 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -1219,64 +1219,85 @@ mod tests { #[crate::pyclass] #[pyo3(crate = "crate", subclass)] - struct Base { - base_name: &'static str, + struct BaseClass { + val1: usize, } #[crate::pyclass] - #[pyo3(crate = "crate", extends=Base, subclass)] - struct Sub { - sub_name: &'static str, + #[pyo3(crate = "crate", extends=BaseClass, subclass)] + struct SubClass { + val2: usize, } #[crate::pyclass] - #[pyo3(crate = "crate", extends=Sub)] - struct SubSub { - subsub_name: &'static str, + #[pyo3(crate = "crate", extends=SubClass)] + struct SubSubClass { + val3: usize, } #[crate::pymethods] #[pyo3(crate = "crate")] - impl SubSub { + impl SubSubClass { #[new] - #[rustfmt::skip] - fn new(py: Python<'_>) -> crate::PyResult> { - let base = Base { base_name: "base_name" }; - let sub = Sub { sub_name: "sub_name" }; - let subsub = SubSub { subsub_name: "subsub_name"}; - let init = crate::PyClassInitializer::from(base) - .add_subclass(sub).add_subclass(subsub); - crate::Py::new(py, init) + fn new(py: Python<'_>) -> crate::Py { + let init = crate::PyClassInitializer::from(BaseClass { val1: 10 }) + .add_subclass(SubClass { val2: 15 }) + .add_subclass(SubSubClass { val3: 20 }); + crate::Py::new(py, init).expect("allocation error") + } + + fn get_values(self_: PyRef<'_, Self>) -> (usize, usize, usize) { + let val1 = self_.as_super().as_super().val1; + let val2 = self_.as_super().val2; + (val1, val2, self_.val3) + } + + fn double_values(mut self_: PyRefMut<'_, Self>) { + self_.as_super().as_super().val1 *= 2; + self_.as_super().val2 *= 2; + self_.val3 *= 2; } } #[test] fn test_pyref_as_super() { Python::with_gil(|py| { - let subsub = SubSub::new(py).unwrap().into_bound(py); - let pyref = subsub.borrow(); - assert_eq!(pyref.as_super().as_super().base_name, "base_name"); - assert_eq!(pyref.as_super().sub_name, "sub_name"); - assert_eq!(pyref.subsub_name, "subsub_name"); - // `as_ref` still works the same - assert_eq!(pyref.as_ref().sub_name, "sub_name"); + let obj = SubSubClass::new(py).into_bound(py); + let pyref = obj.borrow(); + assert_eq!(pyref.as_super().as_super().val1, 10); + assert_eq!(pyref.as_super().val2, 15); + assert_eq!(pyref.as_ref().val2, 15); // `as_ref` also works + assert_eq!(pyref.val3, 20); + assert_eq!(SubSubClass::get_values(pyref), (10, 15, 20)); }); } #[test] fn test_pyrefmut_as_super() { Python::with_gil(|py| { - let subsub = SubSub::new(py).unwrap().into_bound(py); - let mut pyrefmut = subsub.borrow_mut(); - pyrefmut.as_super().as_super().base_name = "base_name2"; - pyrefmut.as_super().sub_name = "sub_name2"; - pyrefmut.subsub_name = "subsub_name2"; - assert_eq!(pyrefmut.as_super().as_super().base_name, "base_name2"); - assert_eq!(pyrefmut.as_super().sub_name, "sub_name2"); - assert_eq!(pyrefmut.subsub_name, "subsub_name2"); - // `as_ref`/`as_mut` still work the same - pyrefmut.as_mut().sub_name = "sub_name3"; - assert_eq!(pyrefmut.as_ref().sub_name, "sub_name3"); + let obj = SubSubClass::new(py).into_bound(py); + assert_eq!(SubSubClass::get_values(obj.borrow()), (10, 15, 20)); + { + let mut pyrefmut = obj.borrow_mut(); + assert_eq!(pyrefmut.as_super().as_ref().val1, 10); + pyrefmut.as_super().as_super().val1 -= 5; + pyrefmut.as_super().val2 -= 3; + pyrefmut.as_mut().val2 -= 2; // `as_mut` also works + pyrefmut.val3 -= 5; + } + assert_eq!(SubSubClass::get_values(obj.borrow()), (5, 10, 15)); + SubSubClass::double_values(obj.borrow_mut()); + assert_eq!(SubSubClass::get_values(obj.borrow()), (10, 20, 30)); + }); + } + + #[test] + fn test_pyrefs_in_python() { + Python::with_gil(|py| { + let obj = SubSubClass::new(py); + crate::py_run!(py, obj, "assert obj.get_values() == (10, 15, 20)"); + crate::py_run!(py, obj, "assert obj.double_values() is None"); + crate::py_run!(py, obj, "assert obj.get_values() == (20, 30, 40)"); }); } } From 0fc914394ee278333f31c6e098a5f32502545a0c Mon Sep 17 00:00:00 2001 From: jrudolph Date: Sat, 8 Jun 2024 15:46:12 -0500 Subject: [PATCH 12/13] Updated newsfragment to include additional changes. --- newsfragments/4219.added.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/newsfragments/4219.added.md b/newsfragments/4219.added.md index 48a7369951b..cea8fa1c314 100644 --- a/newsfragments/4219.added.md +++ b/newsfragments/4219.added.md @@ -1 +1,3 @@ -Added `as_super` methods to `PyRef` and `PyRefMut`. \ No newline at end of file +- Added `as_super` methods to `PyRef` and `PyRefMut` for accesing the base class by reference +- Updated user guide to recommend `as_super` for referencing the base class instead of `as_ref` +- Added `pyo3::internal_tricks::ptr_from_mut` function for casting `&mut T` to `*mut T` \ No newline at end of file From f85c83a372bd5d2bae1efb625f557d167a9b7008 Mon Sep 17 00:00:00 2001 From: jrudolph Date: Sat, 8 Jun 2024 16:04:18 -0500 Subject: [PATCH 13/13] Fixed formatting. --- src/pycell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pycell.rs b/src/pycell.rs index 864399984da..9ed6c8aca7d 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -1245,7 +1245,7 @@ mod tests { .add_subclass(SubSubClass { val3: 20 }); crate::Py::new(py, init).expect("allocation error") } - + fn get_values(self_: PyRef<'_, Self>) -> (usize, usize, usize) { let val1 = self_.as_super().as_super().val1; let val2 = self_.as_super().val2;