From patchwork Tue Jan 28 23:50:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [5, of, 7, RESEND] rust-cpython: inline PySharedState::leak_immutable() and PyLeaked::new() From: Yuya Nishihara X-Patchwork-Id: 44726 Message-Id: <8439565986801068475a.1580255431@mimosa> To: mercurial-devel@mercurial-scm.org Date: Wed, 29 Jan 2020 08:50:31 +0900 # HG changeset patch # User Yuya Nishihara # Date 1571471314 -32400 # Sat Oct 19 16:48:34 2019 +0900 # Node ID 8439565986801068475aa58004ab98488d6bde2d # Parent 1dc8005a3aa88e5f277cf11a04c537644b8a0b99 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. 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( - &self, - _py: Python, - data: Ref, - ) -> (&'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 { // is invalid if generation != py_shared_state.generation. impl PyLeaked { - /// # 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.