changeset 52608:d85514a88706

rust-pyo3-sharedref: reworked constructors We had previously duplicated the `new` associated function on `PySharedRef` with a method on `PySharedRefCell`: in `rust-cpython`, the former was hidden by the accessor defined by the `py_class!` macro, which we did not port yet. On `PySharedRefCell` itself, replacing the `new` associated function by the `From` trait carries all the needed semantics, and has the advantage of less repetititons of the type name, which will help with further refactorings and renamings.
author Georges Racinet <georges.racinet@cloudcrane.io>
date Sun, 15 Dec 2024 14:42:53 +0100
parents a7d2529ed6dd
children d1e304025b90
files rust/pyo3-sharedref/src/lib.rs rust/pyo3-sharedref/tests/test_sharedref.rs
diffstat 2 files changed, 28 insertions(+), 50 deletions(-) [+]
line wrap: on
line diff
--- a/rust/pyo3-sharedref/src/lib.rs	Sat Dec 14 18:21:56 2024 +0100
+++ b/rust/pyo3-sharedref/src/lib.rs	Sun Dec 15 14:42:53 2024 +0100
@@ -83,9 +83,7 @@
 ///     fn new(values: &Bound<'_, PyTuple>) -> PyResult<Self> {
 ///         let as_vec = values.extract::<Vec<i32>>()?;
 ///         let s: HashSet<_> = as_vec.iter().copied().collect();
-///         Ok(Self {
-///             rust_set: PySharedRefCell::new(s),
-///         })
+///         Ok(Self { rust_set: s.into() })
 ///     }
 ///
 ///     fn __iter__(slf: &Bound<'_, Self>) -> SetIterator {
@@ -94,7 +92,7 @@
 ///
 ///     fn add(slf: &Bound<'_, Self>, i: i32) -> PyResult<()> {
 ///         let rust_set = &slf.borrow().rust_set;
-///         let shared_ref = unsafe { rust_set.borrow(slf) };
+///         let shared_ref = unsafe { rust_set.borrow_with_owner(slf) };
 ///         let mut set_ref = shared_ref.borrow_mut();
 ///         set_ref.insert(i);
 ///         Ok(())
@@ -112,7 +110,7 @@
 ///     fn new(s: &Bound<'_, Set>) -> Self {
 ///         let py = s.py();
 ///         let rust_set = &s.borrow().rust_set;
-///         let shared_ref = unsafe { rust_set.borrow(s) };
+///         let shared_ref = unsafe { rust_set.borrow_with_owner(s) };
 ///         let leaked_set = shared_ref.leak_immutable();
 ///         let iter = unsafe { leaked_set.map(py, |o| o.iter()) };
 ///         Self {
@@ -176,14 +174,6 @@
 }
 
 impl<T> PySharedRefCell<T> {
-    /// Creates a new `PySharedRefCell` containing `value`.
-    pub fn new(value: T) -> PySharedRefCell<T> {
-        Self {
-            state: PySharedState::new(),
-            data: value.into(),
-        }
-    }
-
     /// Borrows the shared data and its state, keeping a reference
     /// on the owner Python object.
     ///
@@ -191,7 +181,7 @@
     ///
     /// The `data` must be owned by the `owner`. Otherwise, calling
     /// `leak_immutable()` on the shared ref would create an invalid reference.
-    pub unsafe fn borrow<'py>(
+    pub unsafe fn borrow_with_owner<'py>(
         &'py self,
         owner: &'py Bound<'py, PyAny>,
     ) -> PySharedRef<'py, T> {
@@ -203,6 +193,15 @@
     }
 }
 
+impl<T> From<T> for PySharedRefCell<T> {
+    fn from(value: T) -> Self {
+        Self {
+            state: PySharedState::new(),
+            data: value.into(),
+        }
+    }
+}
+
 /// Errors that can happen in `leak_immutable()`
 #[derive(Debug, PartialEq, Eq)]
 pub enum TryLeakError {
@@ -231,25 +230,6 @@
 }
 
 impl<'py, T: ?Sized> PySharedRef<'py, T> {
-    /// Creates a reference to the given `PySharedRefCell` owned by the
-    /// given `PyObject`.
-    ///
-    /// # Safety
-    ///
-    /// The `data` must be owned by the `owner`. Otherwise, `leak_immutable()`
-    /// would create an invalid reference.
-    #[doc(hidden)]
-    pub unsafe fn new(
-        owner: &'py Bound<'py, PyAny>,
-        data: &'py PySharedRefCell<T>,
-    ) -> Self {
-        Self {
-            owner,
-            state: &data.state,
-            data: &data.data,
-        }
-    }
-
     /// Immutably borrows the wrapped value.
     ///
     /// # Panics
--- a/rust/pyo3-sharedref/tests/test_sharedref.rs	Sat Dec 14 18:21:56 2024 +0100
+++ b/rust/pyo3-sharedref/tests/test_sharedref.rs	Sun Dec 15 14:42:53 2024 +0100
@@ -10,9 +10,7 @@
 impl Owner {
     #[new]
     fn new(s: String) -> Self {
-        Self {
-            string: PySharedRefCell::new(s),
-        }
+        Self { string: s.into() }
     }
 }
 
@@ -30,7 +28,7 @@
 /// taking care of all the boilerplate
 fn leak_string(owner: &Bound<'_, Owner>) -> UnsafePyLeaked<&'static String> {
     let cell = &owner.borrow().string;
-    let shared_ref = unsafe { cell.borrow(owner) };
+    let shared_ref = unsafe { cell.borrow_with_owner(owner) };
     shared_ref.leak_immutable()
 }
 
@@ -38,7 +36,7 @@
     owner: &Bound<'_, Owner>,
 ) -> Result<UnsafePyLeaked<&'static String>, TryLeakError> {
     let cell = &owner.borrow().string;
-    let shared_ref = unsafe { cell.borrow(owner) };
+    let shared_ref = unsafe { cell.borrow_with_owner(owner) };
     shared_ref.try_leak_immutable()
 }
 
@@ -54,7 +52,7 @@
     f: impl FnOnce(&mut String),
 ) -> () {
     let cell = &owner.borrow_mut().string;
-    let shared_ref = unsafe { cell.borrow(owner) };
+    let shared_ref = unsafe { cell.borrow_with_owner(owner) };
     f(&mut shared_ref.borrow_mut());
 }
 
@@ -123,19 +121,19 @@
 /// returning a reference to data owned by the function
 fn assert_try_borrow_string_mut_ok(owner: &Bound<'_, Owner>) {
     let cell = &owner.borrow().string;
-    let shared_ref = unsafe { cell.borrow(owner) };
+    let shared_ref = unsafe { cell.borrow_with_owner(owner) };
     assert!(shared_ref.try_borrow_mut().is_ok());
 }
 
 fn assert_try_borrow_string_mut_err(owner: &Bound<'_, Owner>) {
     let cell = &owner.borrow().string;
-    let shared_ref = unsafe { cell.borrow(owner) };
+    let shared_ref = unsafe { cell.borrow_with_owner(owner) };
     assert!(shared_ref.try_borrow_mut().is_err());
 }
 
 fn assert_try_borrow_string_err(owner: &Bound<'_, Owner>) {
     let cell = &owner.borrow().string;
-    let shared_ref = unsafe { cell.borrow(owner) };
+    let shared_ref = unsafe { cell.borrow_with_owner(owner) };
     assert!(shared_ref.try_borrow().is_err());
 }
 
@@ -178,7 +176,7 @@
 fn test_try_leak_while_borrow_mut() -> PyResult<()> {
     with_setup(|_py, owner| {
         let cell = &owner.borrow().string;
-        let shared_ref = unsafe { cell.borrow(owner) };
+        let shared_ref = unsafe { cell.borrow_with_owner(owner) };
         let _mut_ref = shared_ref.borrow_mut();
 
         assert!(try_leak_string(owner).is_err());
@@ -191,7 +189,7 @@
 fn test_leak_while_borrow_mut() {
     with_setup(|_py, owner| {
         let cell = &owner.borrow().string;
-        let shared_ref = unsafe { cell.borrow(owner) };
+        let shared_ref = unsafe { cell.borrow_with_owner(owner) };
         let _mut_ref = shared_ref.borrow_mut();
 
         leak_string(owner);
@@ -204,7 +202,7 @@
 fn test_try_borrow_mut_while_borrow() -> PyResult<()> {
     with_setup(|_py, owner| {
         let cell = &owner.borrow().string;
-        let shared_ref = unsafe { cell.borrow(owner) };
+        let shared_ref = unsafe { cell.borrow_with_owner(owner) };
         let _ref = shared_ref.borrow();
 
         assert_try_borrow_string_mut_err(owner);
@@ -217,10 +215,10 @@
 fn test_borrow_mut_while_borrow() {
     with_setup(|_py, owner| {
         let cell = &owner.borrow().string;
-        let shared_ref = unsafe { cell.borrow(owner) };
+        let shared_ref = unsafe { cell.borrow_with_owner(owner) };
         let _ref = shared_ref.borrow();
 
-        let shared_ref2 = unsafe { cell.borrow(owner) };
+        let shared_ref2 = unsafe { cell.borrow_with_owner(owner) };
         let _mut_ref = shared_ref2.borrow_mut();
         Ok(())
     })
@@ -231,7 +229,7 @@
 fn test_try_borrow_while_borrow_mut() -> PyResult<()> {
     with_setup(|_py, owner| {
         let cell = &owner.borrow().string;
-        let shared_ref = unsafe { cell.borrow(owner) };
+        let shared_ref = unsafe { cell.borrow_with_owner(owner) };
         let _mut_ref = shared_ref.borrow_mut();
 
         assert_try_borrow_string_err(owner);
@@ -244,10 +242,10 @@
 fn test_borrow_while_borrow_mut() {
     with_setup(|_py, owner| {
         let cell = &owner.borrow().string;
-        let shared_ref = unsafe { cell.borrow(owner) };
+        let shared_ref = unsafe { cell.borrow_with_owner(owner) };
         let _mut_ref = shared_ref.borrow_mut();
 
-        let shared_ref2 = unsafe { cell.borrow(owner) };
+        let shared_ref2 = unsafe { cell.borrow_with_owner(owner) };
         let _ref = shared_ref2.borrow();
         Ok(())
     })