Mark SelfCell invariant over owner lifetime if dependent is not_covar…

…iant

src/lib.rs

@@ -318,9 +318,7 @@ macro_rules! self_cell {
             $Dependent<'static>
         >,
 
-        // marker to ensure that contravariant owners don't imply covariance
-        // over the dependent. See issue #18
-        owner_marker: core::marker::PhantomData<$(&$OwnerLifetime)* ()>,
+        $(owner_marker: $crate::_covariant_owner_marker!($Covariance, $OwnerLifetime) ,)*
     }
 
     impl $(<$OwnerLifetime>)* $StructName $(<$OwnerLifetime>)* {
@@ -373,7 +371,7 @@ macro_rules! self_cell {
                     unsafe_self_cell: $crate::unsafe_self_cell::UnsafeSelfCell::new(
                         joined_void_ptr,
                     ),
-                    owner_marker: core::marker::PhantomData,
+                    $(owner_marker: $crate::_covariant_owner_marker_ctor!($OwnerLifetime) ,)*
                 }
             }
         }
@@ -419,7 +417,7 @@ macro_rules! self_cell {
                             unsafe_self_cell: $crate::unsafe_self_cell::UnsafeSelfCell::new(
                                 joined_void_ptr,
                             ),
-                            owner_marker: core::marker::PhantomData,
+                            $(owner_marker: $crate::_covariant_owner_marker_ctor!($OwnerLifetime) ,)*
                         })
                     }
                     Err(err) => Err(err)
@@ -468,7 +466,7 @@ macro_rules! self_cell {
                             unsafe_self_cell: $crate::unsafe_self_cell::UnsafeSelfCell::new(
                                 joined_void_ptr,
                             ),
-                            owner_marker: core::marker::PhantomData,
+                            $(owner_marker: $crate::_covariant_owner_marker_ctor!($OwnerLifetime) ,)*
                         })
                     }
                     Err(err) => {
@@ -571,6 +569,35 @@ macro_rules! _covariant_access {
     };
 }
 
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _covariant_owner_marker {
+    (covariant, $OwnerLifetime:lifetime) => {
+        // Ensure that contravariant owners don't imply covariance
+        // over the dependent. See issue https://github.com/Voultapher/self_cell/issues/18
+        core::marker::PhantomData<&$OwnerLifetime ()>
+    };
+    (not_covariant, $OwnerLifetime:lifetime) => {
+        // See the discussion in https://github.com/Voultapher/self_cell/pull/29
+        //
+        // If the dependent is non_covariant, mark the owner as invariant over it's
+        // lifetime. Otherwise unsound use is possible.
+        core::marker::PhantomData<core::cell::UnsafeCell<&$OwnerLifetime ()>>
+    };
+    ($x:ident, $OwnerLifetime:lifetime) => {
+        compile_error!("This macro only accepts `covariant` or `not_covariant`");
+    };
+}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _covariant_owner_marker_ctor {
+    ($OwnerLifetime:lifetime) => {
+        // Helper to optionally expand into PhantomData for construction.
+        core::marker::PhantomData
+    };
+}
+
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _impl_automatic_derive {

tests/invalid/covariant_owner_non_covariant_dependent.rs

@@ -0,0 +1,23 @@
+use std::{cell::Cell, marker::PhantomData};
+
+use self_cell::self_cell;
+
+self_cell! {
+    struct Foo<'a> {
+        owner: PhantomData<&'a ()>,
+        #[not_covariant]
+        dependent: Dependent,
+    }
+}
+
+type Dependent<'q> = Cell<&'q str>;
+
+fn main() {
+    let foo = Foo::new(PhantomData, |_| Cell::new(""));
+    let s: String = "Hello World".into();
+    foo.with_dependent(|_, d| {
+        d.set(&s);
+    });
+    drop(s);
+    foo.with_dependent(|_, d| println!("{}", d.get()));
+}

tests/invalid/covariant_owner_non_covariant_dependent.stderr

@@ -0,0 +1,28 @@
+error[E0597]: `s` does not live long enough
+  --> $DIR/covariant_owner_non_covariant_dependent.rs:19:16
+   |
+18 |     foo.with_dependent(|_, d| {
+   |                        ------ value captured here
+19 |         d.set(&s);
+   |                ^ borrowed value does not live long enough
+...
+23 | }
+   | -
+   | |
+   | `s` dropped here while still borrowed
+   | borrow might be used here, when `foo` is dropped and runs the `Drop` code for type `Foo`
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
+
+error[E0505]: cannot move out of `s` because it is borrowed
+  --> $DIR/covariant_owner_non_covariant_dependent.rs:21:10
+   |
+18 |     foo.with_dependent(|_, d| {
+   |                        ------ borrow of `s` occurs here
+19 |         d.set(&s);
+   |                - borrow occurs due to use in closure
+20 |     });
+21 |     drop(s);
+   |          ^ move out of `s` occurs here
+22 |     foo.with_dependent(|_, d| println!("{}", d.get()));
+   |     --- borrow later used here

tests/invalid/escape_dependent.rs

@@ -0,0 +1,28 @@
+use std::{cell::Cell, marker::PhantomData};
+
+use self_cell::self_cell;
+
+type NotCovariant<'a> = Cell<&'a str>;
+
+struct Owner<'a>(String, PhantomData<&'a ()>);
+
+self_cell!(
+    struct NoCov<'a> {
+        owner: Owner<'a>,
+
+        #[not_covariant]
+        dependent: NotCovariant,
+    }
+);
+
+fn main() {
+    let cell = NoCov::new(Owner("hi this is no good".into(), PhantomData), |owner| {
+        Cell::new(&owner.0)
+    });
+    let leaked_ref = cell.with_dependent(|_, dependent| dependent);
+    leaked_ref.set(&String::from("temporary"));
+
+    cell.with_dependent(|_, dep| {
+        println!("{}", dep.replace(""));
+    });
+}

tests/invalid/escape_dependent.stderr

@@ -0,0 +1,24 @@
+error[E0597]: `cell` does not live long enough
+  --> $DIR/escape_dependent.rs:22:22
+   |
+22 |     let leaked_ref = cell.with_dependent(|_, dependent| dependent);
+   |                      ^^^^ borrowed value does not live long enough
+...
+28 | }
+   | -
+   | |
+   | `cell` dropped here while still borrowed
+   | borrow might be used here, when `cell` is dropped and runs the `Drop` code for type `NoCov`
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/escape_dependent.rs:23:21
+   |
+23 |     leaked_ref.set(&String::from("temporary"));
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^ - temporary value is freed at the end of this statement
+   |                     |
+   |                     creates a temporary which is freed while still in use
+...
+28 | }
+   | - borrow might be used here, when `cell` is dropped and runs the `Drop` code for type `NoCov`
+   |
+   = note: consider using a `let` binding to create a longer lived value

tests/invalid/leak_dependent.stderr

@@ -1,8 +1,9 @@
-error: lifetime may not live long enough
-  --> $DIR/leak_dependent.rs:18:58
+error[E0597]: `cell` does not live long enough
+  --> $DIR/leak_dependent.rs:18:23
    |
 18 |     let _leaked_ref = cell.with_dependent(|_, dependent| dependent);
-   |                                            -           - ^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
-   |                                            |           |
-   |                                            |           return type of closure is &Cell<&'2 String>
-   |                                            has type `&'1 String`
+   |                       ^^^^                               --------- returning this value requires that `cell` is borrowed for `'static`
+   |                       |
+   |                       borrowed value does not live long enough
+19 | }
+   | - `cell` dropped here while still borrowed