Mercurial > public > mercurial-scm > hg
changeset 52414:233707101dae
rust-pyo3: simplified API to get core Index references
Given the amount of conversion and arcane internal detail, it
is easier for the caller and makes a better separation of concerns
to just introduce a new unsafe helper. It is actually possible that
it would be safe, and we can decide about that later.
Actually the reason given in the `cpython` crate for unsafety of the
`try_borrow()` method is the following:
```
// try_borrow() and try_borrow_mut() are unsafe because self.data may
// have a function returning the inner &'static reference.
// If T is &'static U, its lifetime can be easily coerced to &'a U, but
// how could we do that for Whatever<'static> in general?
```
Given that we coerce the Index reference to the GIL lifetime and that
it is unlikely that the inner data would contain a function returning the
a `'static` reference, it is possible that it is actually even safe.
| author | Georges Racinet <georges.racinet@cloudcrane.io> |
|---|---|
| date | Fri, 29 Nov 2024 23:35:31 +0100 |
| parents | 20fe0bf9a9a5 |
| children | 4eec920bbb37 |
| files | rust/hg-pyo3/src/convert_cpython.rs rust/hg-pyo3/src/dagops.rs |
| diffstat | 2 files changed, 18 insertions(+), 11 deletions(-) [+] |
line wrap: on
line diff
--- a/rust/hg-pyo3/src/convert_cpython.rs Sat Nov 30 20:58:09 2024 +0100 +++ b/rust/hg-pyo3/src/convert_cpython.rs Fri Nov 29 23:35:31 2024 +0100 @@ -16,6 +16,7 @@ use cpython::PythonObject; use lazy_static::lazy_static; +use hg::revlog::index::Index as CoreIndex; use rusthg::revlog::{InnerRevlog, PySharedIndex}; /// Force cpython's GIL handle with the appropriate lifetime @@ -165,9 +166,21 @@ Ok(unsafe { leaked.map(py, |idx| PySharedIndex { inner: &idx.index }) }) } -pub(crate) fn proxy_index_extract<'py>( +/// Full extraction of the proxy index object as received in PyO3 to a +/// [`CoreIndex`] reference. +/// +/// The safety invariants to maintain are those of the underlying +/// [`UnsafePyLeaked::try_borrow`]: the caller must not leak the inner +/// reference. +pub(crate) unsafe fn proxy_index_extract<'py>( index_proxy: &Bound<'py, PyAny>, -) -> PyResult<(cpython::Python<'py>, cpython::UnsafePyLeaked<PySharedIndex>)> { +) -> PyResult<&'py CoreIndex> { let (py, idx_proxy) = to_cpython_py_object(index_proxy); - Ok((py, py_rust_index_to_graph(py, idx_proxy)?)) + let py_leaked = py_rust_index_to_graph(py, idx_proxy)?; + let py_shared = &*unsafe { + py_leaked + .try_borrow(py) + .map_err(|e| from_cpython_pyerr(py, e))? + }; + Ok(py_shared.inner) }
--- a/rust/hg-pyo3/src/dagops.rs Sat Nov 30 20:58:09 2024 +0100 +++ b/rust/hg-pyo3/src/dagops.rs Fri Nov 29 23:35:31 2024 +0100 @@ -15,7 +15,7 @@ use hg::{dagops, Revision}; -use crate::convert_cpython::{from_cpython_pyerr, proxy_index_extract}; +use crate::convert_cpython::proxy_index_extract; use crate::exceptions::GraphError; use crate::revision::{rev_pyiter_collect, PyRevision}; use crate::util::new_submodule; @@ -29,14 +29,8 @@ index_proxy: &Bound<'_, PyAny>, revs: &Bound<'_, PyAny>, ) -> PyResult<HashSet<PyRevision>> { - let (py, py_leaked) = proxy_index_extract(index_proxy)?; // Safety: we don't leak the "faked" reference out of `UnsafePyLeaked` - let index = &*unsafe { - py_leaked - .try_borrow(py) - .map_err(|e| from_cpython_pyerr(py, e))? - }; - + let index = unsafe { proxy_index_extract(index_proxy)? }; let mut as_set: HashSet<Revision> = rev_pyiter_collect(revs, index)?; dagops::retain_heads(index, &mut as_set).map_err(GraphError::from_hg)?; Ok(as_set.into_iter().map(Into::into).collect())