Skip to content

Commit

Permalink
add &ReadOnlyCell<T> conversions from &T and &Cell<T>
Browse files Browse the repository at this point in the history
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:
rust-lang/unsafe-code-guidelines#303.

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
```
  • Loading branch information
oconnor663 committed Nov 16, 2023
1 parent d89c388 commit a37bd63
Showing 1 changed file with 40 additions and 3 deletions.
43 changes: 40 additions & 3 deletions src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,9 +641,9 @@ 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>);
pub struct ReadOnlyCell<T: Copy>(cell::UnsafeCell<T>);

impl<T: Element> ReadOnlyCell<T> {
impl<T: Copy> ReadOnlyCell<T> {
/// Returns a copy of the current value.
#[inline]
pub fn get(&self) -> T {
Expand All @@ -655,6 +655,22 @@ impl<T: Element> ReadOnlyCell<T> {
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>) }
}
}

macro_rules! impl_element(
Expand Down Expand Up @@ -686,7 +702,7 @@ impl_element!(f64, Float);

#[cfg(test)]
mod tests {
use super::PyBuffer;
use super::{PyBuffer, ReadOnlyCell};
use crate::ffi;
use crate::Python;

Expand Down Expand Up @@ -942,4 +958,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;
}
}

0 comments on commit a37bd63

Please sign in to comment.