Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proof of Concept: making Lazy<T, F> covariant in F #230

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/covariant_cell_option.rs
@@ -0,0 +1,59 @@
//! A `Cell<Option<F>>`, but covariant, at the cost of a very reduced API.
//!
//! (the idea being: it starts as `Some(F)`, and the only `&`-based operation
//! is `take()`ing it. This guarantees covariance to be fine, since the `F`
//! value is never overwritten).

use ::core::{cell::Cell, mem::ManuallyDrop};

pub struct CovariantCellOption<F> {
/// Invariant: if this is `true` then `value` must contain a non-dropped `F`;
is_some: Cell<bool>,
value: ManuallyDrop<F>,
}

impl<F> CovariantCellOption<F> {
pub const fn some(value: F) -> Self {
Self { is_some: Cell::new(true), value: ManuallyDrop::new(value) }
}

pub fn into_inner(self) -> Option<F> {
// Small optimization: disable drop glue so as not to have to overwrite `is_some`.
let mut this = ManuallyDrop::new(self);
let is_some = this.is_some.get_mut();
is_some.then(|| unsafe {
// SAFETY: as per the invariant, we can use `value`. We can also *consume* it by doing:
// *is_some = false;
// but we actually don't even need to do it since we don't use `this` anymore.
ManuallyDrop::take(&mut this.value)
})
}

pub fn take(&self) -> Option<F> {
self.is_some.get().then(|| unsafe {
// SAFETY: as per the invariant, we can use `value`.
// Clearing the `is_some` flag also lets us *consume* it.
self.is_some.set(false);
// `ManuallyDrop::take_by_ref`, morally.
<*const F>::read(&*self.value)
})
}
}

impl<F> Drop for CovariantCellOption<F> {
fn drop(&mut self) {
if *self.is_some.get_mut() {
unsafe {
// SAFETY: as per the invariant, we can use `value`.
ManuallyDrop::drop(&mut self.value)
}
}
}
}

#[cfg(test)]
fn _assert_covariance<'short>(
long: *const (CovariantCellOption<&'static ()>,),
) -> *const (CovariantCellOption<&'short ()>,) {
long
}
21 changes: 13 additions & 8 deletions src/lib.rs
Expand Up @@ -369,6 +369,8 @@
#[cfg(feature = "alloc")]
extern crate alloc;

mod covariant_cell_option;

#[cfg(all(feature = "critical-section", not(feature = "std")))]
#[path = "imp_cs.rs"]
mod imp;
Expand All @@ -384,13 +386,14 @@ mod imp;
/// Single-threaded version of `OnceCell`.
pub mod unsync {
use core::{
cell::{Cell, UnsafeCell},
cell::UnsafeCell,
fmt, mem,
ops::{Deref, DerefMut},
panic::{RefUnwindSafe, UnwindSafe},
};

use super::unwrap_unchecked;
use crate::covariant_cell_option::CovariantCellOption;

/// A cell which can be written to only once. It is not thread safe.
///
Expand Down Expand Up @@ -717,7 +720,7 @@ pub mod unsync {
/// ```
pub struct Lazy<T, F = fn() -> T> {
cell: OnceCell<T>,
init: Cell<Option<F>>,
init: CovariantCellOption<F>,
}

impl<T, F: RefUnwindSafe> RefUnwindSafe for Lazy<T, F> where OnceCell<T>: RefUnwindSafe {}
Expand All @@ -744,7 +747,7 @@ pub mod unsync {
/// # }
/// ```
pub const fn new(init: F) -> Lazy<T, F> {
Lazy { cell: OnceCell::new(), init: Cell::new(Some(init)) }
Lazy { cell: OnceCell::new(), init: CovariantCellOption::some(init) }
}

/// Consumes this `Lazy` returning the stored value.
Expand All @@ -754,7 +757,8 @@ pub mod unsync {
let cell = this.cell;
let init = this.init;
cell.into_inner().ok_or_else(|| {
init.take().unwrap_or_else(|| panic!("Lazy instance has previously been poisoned"))
init.into_inner()
.unwrap_or_else(|| panic!("Lazy instance has previously been poisoned"))
})
}
}
Expand Down Expand Up @@ -861,13 +865,13 @@ pub mod unsync {
#[cfg(any(feature = "std", feature = "critical-section"))]
pub mod sync {
use core::{
cell::Cell,
fmt, mem,
ops::{Deref, DerefMut},
panic::RefUnwindSafe,
};

use super::{imp::OnceCell as Imp, unwrap_unchecked};
use crate::covariant_cell_option::CovariantCellOption;

/// A thread-safe cell which can be written to only once.
///
Expand Down Expand Up @@ -1253,7 +1257,7 @@ pub mod sync {
/// ```
pub struct Lazy<T, F = fn() -> T> {
cell: OnceCell<T>,
init: Cell<Option<F>>,
init: CovariantCellOption<F>,
}

impl<T: fmt::Debug, F> fmt::Debug for Lazy<T, F> {
Expand All @@ -1275,7 +1279,7 @@ pub mod sync {
/// Creates a new lazy value with the given initializing
/// function.
pub const fn new(f: F) -> Lazy<T, F> {
Lazy { cell: OnceCell::new(), init: Cell::new(Some(f)) }
Lazy { cell: OnceCell::new(), init: CovariantCellOption::some(f) }
}

/// Consumes this `Lazy` returning the stored value.
Expand All @@ -1285,7 +1289,8 @@ pub mod sync {
let cell = this.cell;
let init = this.init;
cell.into_inner().ok_or_else(|| {
init.take().unwrap_or_else(|| panic!("Lazy instance has previously been poisoned"))
init.into_inner()
.unwrap_or_else(|| panic!("Lazy instance has previously been poisoned"))
})
}
}
Expand Down
19 changes: 19 additions & 0 deletions tests/it.rs
@@ -1,3 +1,18 @@
/// Put here any code relying on duck-typed `Lazy` and `OnceCell`, oblivious to
/// their exact `sync` or `unsync` nature.
macro_rules! tests_for_both {() => (
/// <https://github.com/matklad/once_cell/issues/167>
#[test]
fn assert_lazy_is_covariant_in_the_ctor() {
#[allow(dead_code)]
type AnyLazy<'f, T> = Lazy<T, Box<dyn 'f + FnOnce() -> T>>;

fn _for<'short, T>(it: *const (AnyLazy<'static, T>, )) -> *const (AnyLazy<'short, T>, ) {
it
}
}
)}

mod unsync {
use core::{
cell::Cell,
Expand All @@ -6,6 +21,8 @@ mod unsync {

use once_cell::unsync::{Lazy, OnceCell};

tests_for_both!();

#[test]
fn once_cell() {
let c = OnceCell::new();
Expand Down Expand Up @@ -263,6 +280,8 @@ mod sync {

use once_cell::sync::{Lazy, OnceCell};

tests_for_both!();

#[test]
fn once_cell() {
let c = OnceCell::new();
Expand Down