Skip to content

Conversation

@joshlf
Copy link
Member

@joshlf joshlf commented Mar 6, 2024

This will allow us to implement TransparentWrapper for T where T and T::Inner do not have UnsafeCells at the same byte ranges. This, in turn, will let us automatically implement traits for these types so long as those traits don't require reasoning about UnsafeCell ranges (in other words, all traits other than NoCell).

This will allow us to implement `TransparentWrapper` for `T` where `T`
and `T::Inner` do not have `UnsafeCell`s at the same byte ranges. This,
in turn, will let us automatically implement traits for these types so
long as those traits don't require reasoning about `UnsafeCell` ranges
(in other words, all traits other than `NoCell`).
@joshlf joshlf requested a review from jswrenn March 6, 2024 15:11
@joshlf joshlf enabled auto-merge March 6, 2024 15:11
Comment on lines +115 to +116
// SAFETY: Per [1], `MaybeUninit<T>` has `UnsafeCell`s at the same byte
// ranges as `Inner = T`, and `UnsafeCell`s at the same byte offsets as `T`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the distinction you're drawing here between ranges and offsets?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should perhaps say "covering" the same byte ranges. E.g., the type #[repr(C)] struct Foo(UnsafeCell<()>, usize) doesn't have any byte ranges which are covered by UnsafeCells, but there is a ZST UnsafeCell at byte offset 0. Until rust-lang/unsafe-code-guidelines#495 is resolved (and until we remove the restriction from NoCell that even ZST UnsafeCells are banned), this distinction is required for soundness.

@joshlf joshlf added this pull request to the merge queue Mar 6, 2024
Merged via the queue into main with commit f45a227 Mar 6, 2024
@joshlf joshlf deleted the unsafe-cell-variance branch March 6, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants