From 779ee45b3abc0e5322ac76eaed8ae648f3d2e3ff Mon Sep 17 00:00:00 2001
From: Jack O'Connor <oconnor663@gmail.com>
Date: Thu, 16 Nov 2023 11:01:32 -0800
Subject: [PATCH] add &ReadOnlyCell<T> conversions from &T and &Cell<T>

Is there any practical use for these conversions? I'm not sure :) But
they're spiritually similar to the `Cell::from_mut` conversion in the
standard library. This makes it possible to write a function taking
`&ReadOnlyCell<T>` if you only need to copy the `T` and you don't need
the guarantee that it won't change from one read to the next (which it
could, if other `&Cell<T>` references exist pointing to the same value).

With these conversions in place, the relationships between the different
reference types look like this:

```
                     &mut T
                    /      \
                coerce   Cell::from_mut
                  /          \
                 &T       &Cell<T>
                  \          /
ReadOnlyCell::from_ref    ReadOnlyCell::from_cell
                    \      /
                 &ReadOnlyCell<T>
```

This sort of thing wasn't supported by Miri until recently, but the new
Tree Borrows model supports it. The new test currently fails Miri by
default:

```
$ cargo +nightly miri test test_read_only_cell_conversions
...
error: Undefined Behavior: trying to retag from <747397> for SharedReadWrite permission at
alloc222352[0x0], but that tag only grants SharedReadOnly permission for this location
```

But it passes with `-Zmiri-tree-borrows`:

```
$ MIRIFLAGS="-Zmiri-tree-borrows" cargo +nightly miri test test_read_only_cell_conversions
...
test buffer::tests::test_read_only_cell_conversions ... ok
```
---
 src/buffer.rs | 57 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 9 deletions(-)

diff --git a/src/buffer.rs b/src/buffer.rs
index 4ff2c1d97eb..c951f7dff6b 100644
--- a/src/buffer.rs
+++ b/src/buffer.rs
@@ -641,20 +641,38 @@ impl<T> Drop for PyBuffer<T> {
 ///  The data cannot be modified through the reference, but other references may
 ///  be modifying the data.
 #[repr(transparent)]
-pub struct ReadOnlyCell<T: Element>(cell::UnsafeCell<T>);
-
-impl<T: Element> ReadOnlyCell<T> {
-    /// Returns a copy of the current value.
-    #[inline]
-    pub fn get(&self) -> T {
-        unsafe { *self.0.get() }
-    }
+pub struct ReadOnlyCell<T: ?Sized>(cell::UnsafeCell<T>);
 
+impl<T: ?Sized> ReadOnlyCell<T> {
     /// Returns a pointer to the current value.
     #[inline]
     pub fn as_ptr(&self) -> *const T {
         self.0.get()
     }
+
+    /// Converts `&T` to `&ReadOnlyCell<T>`.
+    #[inline]
+    pub fn from_ref(t: &T) -> &ReadOnlyCell<T> {
+        // Safety: ReadOnlyCell is repr(transparent), and &ReadOnlyCell<T> is strictly less capable
+        // than &T.
+        unsafe { &*(t as *const T as *const ReadOnlyCell<T>) }
+    }
+
+    /// Converts `&Cell<T>` to `&ReadOnlyCell<T>`.
+    #[inline]
+    pub fn from_cell(t: &cell::Cell<T>) -> &ReadOnlyCell<T> {
+        // Safety: Cell and ReadOnlyCell are both repr(transparent), and &ReadOnlyCell<T> is
+        // strictly less capable than &Cell<T>.
+        unsafe { &*(t as *const cell::Cell<T> as *const ReadOnlyCell<T>) }
+    }
+}
+
+impl<T: Copy> ReadOnlyCell<T> {
+    /// Returns a copy of the current value.
+    #[inline]
+    pub fn get(&self) -> T {
+        unsafe { *self.0.get() }
+    }
 }
 
 macro_rules! impl_element(
@@ -686,7 +704,7 @@ impl_element!(f64, Float);
 
 #[cfg(test)]
 mod tests {
-    use super::PyBuffer;
+    use super::{PyBuffer, ReadOnlyCell};
     use crate::ffi;
     use crate::Python;
 
@@ -942,4 +960,25 @@ mod tests {
             assert_eq!(buffer.to_fortran_vec(py).unwrap(), [10.0, 11.0, 12.0, 13.0]);
         });
     }
+
+    #[test]
+    fn test_read_only_cell_conversions() {
+        let mut x = 42;
+        let x_ref_mut = &mut x;
+
+        let x_ref = &*x_ref_mut;
+        let x_rocell1 = ReadOnlyCell::from_ref(x_ref);
+        let x_rocell2 = ReadOnlyCell::from_ref(x_ref);
+        assert_eq!(x_rocell1.get(), x_rocell2.get());
+        assert_eq!(x_rocell1.get(), 42);
+
+        let x_cell = std::cell::Cell::from_mut(x_ref_mut);
+        let x_rocell3 = ReadOnlyCell::from_cell(x_cell);
+        let x_rocell4 = ReadOnlyCell::from_cell(x_cell);
+        assert_eq!(x_rocell3.get(), x_rocell4.get());
+        assert_eq!(x_rocell3.get(), 42);
+
+        // Importantly, this doesn't compile, because x_ref and x_cell cannot coexist.
+        // _ = *x_ref;
+    }
 }