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

Fix insufficient covariance detection #50

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ pub mod unsafe_self_cell;
///
/// * **covariant**: This generates the direct reference accessor function
/// `borrow_dependent`. This is only safe to do if this compiles `fn
/// _assert_covariance<'x: 'y, 'y>(x: $Dependent<'x>) -> $Dependent<'y>
/// _assert_covariance<'x: 'y, 'y>(x: &'y $Dependent<'x>) -> &'y $Dependent<'y>
/// {x}`. Otherwise you could choose a lifetime that is too short for types
/// with interior mutability like `Cell`, which can lead to UB in safe code.
/// Which would violate the promise of this library that it is safe-to-use.
Expand Down Expand Up @@ -577,7 +577,7 @@ macro_rules! _covariant_access {
(covariant, $Vis:vis, $Dependent:ident) => {
/// Borrows dependent.
$Vis fn borrow_dependent<'_q>(&'_q self) -> &'_q $Dependent<'_q> {
fn _assert_covariance<'x: 'y, 'y>(x: $Dependent<'x>) -> $Dependent<'y> {
fn _assert_covariance<'x: 'y, 'y>(x: &'y $Dependent<'x>) -> &'y $Dependent<'y> {
// This function only compiles for covariant types.
x // Change the macro invocation to not_covariant.
}
Expand Down
31 changes: 5 additions & 26 deletions tests-extra/invalid/wrong_covariance.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0308]: mismatched types
error[E0623]: lifetime mismatch
--> $DIR/wrong_covariance.rs:7:1
|
7 | / self_cell!(
Expand All @@ -8,30 +8,9 @@ error[E0308]: mismatched types
... |
13 | | }
14 | | );
| |__^ lifetime mismatch
| | ^
| | |
| |__these two types are declared with different lifetimes...
| ...but data from `x` flows into `x` here
|
= note: expected struct `Cell<&'y String>`
found struct `Cell<&'x String>`
note: the lifetime `'y` as defined on the function body at 580:43...
--> $DIR/wrong_covariance.rs:7:1
|
7 | / self_cell!(
8 | | struct NoCov {
9 | | owner: String,
10 | |
... |
13 | | }
14 | | );
| |__^
note: ...does not necessarily outlive the lifetime `'x` as defined on the function body at 580:35
--> $DIR/wrong_covariance.rs:7:1
|
7 | / self_cell!(
8 | | struct NoCov {
9 | | owner: String,
10 | |
... |
13 | | }
14 | | );
| |__^
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
32 changes: 32 additions & 0 deletions tests-extra/invalid/wrong_covariance_unsize_coercion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use std::cell::RefCell;
use std::fmt;

use self_cell::self_cell;

self_cell! {
struct WrongVarianceExample {
owner: (),

#[covariant]
dependent: Dependent,
}
}

// this type is not covariant
type Dependent<'a> = RefCell<Box<dyn fmt::Display + 'a>>;

fn main() {
let cell = WrongVarianceExample::new((), |_| RefCell::new(Box::new("")));
let s = String::from("Hello World");

// borrow_dependent unsound due to incorrectly checked variance
*cell.borrow_dependent().borrow_mut() = Box::new(s.as_str());

// s still exists
cell.with_dependent(|_, d| println!("{}", d.borrow()));

drop(s);

// s is gone
cell.with_dependent(|_, d| println!("{}", d.borrow()));
}
16 changes: 16 additions & 0 deletions tests-extra/invalid/wrong_covariance_unsize_coercion.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0623]: lifetime mismatch
--> $DIR/wrong_covariance_unsize_coercion.rs:6:1
|
6 | / self_cell! {
7 | | struct WrongVarianceExample {
8 | | owner: (),
9 | |
... |
12 | | }
13 | | }
| | ^
| | |
| |_these two types are declared with different lifetimes...
| ...but data from `x` flows into `x` here
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
Loading