Patchwork [5,of,7] rust-cpython: inline PySharedState::leak_immutable() and PyLeaked::new()

login
register
mail settings
Submitter Yuya Nishihara
Date Oct. 22, 2019, 8:11 a.m.
Message ID <253a5f9f8ed37f696073.1571731916@mimosa>
Download mbox | patch
Permalink /patch/42526/
State New
Headers show

Comments

Yuya Nishihara - Oct. 22, 2019, 8:11 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1571471314 -32400
#      Sat Oct 19 16:48:34 2019 +0900
# Node ID 253a5f9f8ed37f696073bcfc9be87bf5fddbfd57
# Parent  e842b9628b92f587a8be0cfb03bb9be5ac095494
rust-cpython: inline PySharedState::leak_immutable() and PyLeaked::new()

For the same reason as the previous patch. The unsafe stuff can be better
documented if these functions are inlined.

Patch

diff --git a/rust/hg-cpython/src/ref_sharing.rs b/rust/hg-cpython/src/ref_sharing.rs
--- a/rust/hg-cpython/src/ref_sharing.rs
+++ b/rust/hg-cpython/src/ref_sharing.rs
@@ -57,24 +57,6 @@  struct PySharedState {
 }
 
 impl PySharedState {
-    /// Return a reference to the wrapped data and its state with an
-    /// artificial static lifetime.
-    /// We need to be protected by the GIL for thread-safety.
-    ///
-    /// # Safety
-    ///
-    /// This is highly unsafe since the lifetime of the given data can be
-    /// extended. Do not call this function directly.
-    unsafe fn leak_immutable<T>(
-        &self,
-        _py: Python,
-        data: Ref<T>,
-    ) -> (&'static T, &'static PySharedState) {
-        let ptr: *const T = &*data;
-        let state_ptr: *const PySharedState = self;
-        (&*ptr, &*state_ptr)
-    }
-
     fn current_borrow_count(&self, _py: Python) -> usize {
         self.borrow_count.load(Ordering::Relaxed)
     }
@@ -223,10 +205,16 @@  impl<'a, T> PySharedRef<'a, T> {
         // make sure self.data isn't mutably borrowed; otherwise the
         // generation number can't be trusted.
         let data_ref = self.borrow();
-        unsafe {
-            let (static_ref, static_state_ref) =
-                state.leak_immutable(self.py, data_ref);
-            PyLeaked::new(self.py, self.owner, static_ref, static_state_ref)
+
+        // &'static cast is safe because data_ptr and state_ptr are owned
+        // by self.owner, and we do have the GIL for thread safety.
+        let data_ptr: *const T = &*data_ref;
+        let state_ptr: *const PySharedState = state;
+        PyLeaked::<&'static T> {
+            inner: self.owner.clone_ref(self.py),
+            data: unsafe { &*data_ptr },
+            py_shared_state: unsafe { &*state_ptr },
+            generation: state.current_generation(self.py),
         }
     }
 }
@@ -304,23 +292,6 @@  pub struct PyLeaked<T> {
 // is invalid if generation != py_shared_state.generation.
 
 impl<T> PyLeaked<T> {
-    /// # Safety
-    ///
-    /// The `py_shared_state` must be owned by the `inner` Python object.
-    fn new(
-        py: Python,
-        inner: &PyObject,
-        data: T,
-        py_shared_state: &'static PySharedState,
-    ) -> Self {
-        Self {
-            inner: inner.clone_ref(py),
-            data: data,
-            py_shared_state,
-            generation: py_shared_state.current_generation(py),
-        }
-    }
-
     /// Immutably borrows the wrapped value.
     ///
     /// Borrowing fails if the underlying reference has been invalidated.