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

Specify layout of Cell and UnsafeCell #79303

Closed
Kestrer opened this issue Nov 22, 2020 · 3 comments · Fixed by #106921
Closed

Specify layout of Cell and UnsafeCell #79303

Kestrer opened this issue Nov 22, 2020 · 3 comments · Fixed by #106921
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Kestrer
Copy link
Contributor

Kestrer commented Nov 22, 2020

Currently, Cell<T> and UnsafeCell<T> are both #[repr(transparent)]. However, it is unclear whether this is stable or just an implementation detail. This line in the implementation of UnsafeCell notes that "there is no guarantee for user code that this [casting a *const UnsafeCell<T> to *mut T] will work in future versions of the compiler"; but it is unclear whether this applies to the casting of *const UnsafeCell<T> to *const T or the casting of *const T to *mut T. as_slice_of_cells also relies on the layout of UnsafeCell, but makes no mention of it being exclusive to std.

Even if they do stably have a transparent layout, there are also some questions to be resolved around whether creating UnsafeCells from references is UB; conversions like *const T to *const UnsafeCell<T>, &T to &UnsafeCell<T>, &mut T to &UnsafeCell<T>, &[UnsafeCell<T>] to &UnsafeCell<[T]> should all probably be specified.

@steffahn
Copy link
Member

@rustbot modify labels to T-doc, T-lang, C-enhancement.

Also, here’s the link to the relevant URLO discussion.

@rustbot rustbot added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Nov 22, 2020
@sfackler
Copy link
Member

sfackler commented Nov 22, 2020

From a libs perspective, a #[repr(transparent)] annotation on a public type is a stable guarantee. I think that warning has more to due with the unsafe code guidelines decisions around what exactly is UB or not with pointer casts in relation to the "specialness" of UnsafeCell than just if the type is transparent.

@the8472
Copy link
Member

the8472 commented Nov 22, 2020

From a libs perspective, a #[repr(transparent)] annotation on a public type is a stable guarantee.

What does that tell a user though? Its field is not part of the public API thus one can't know in relation to what it is transparent. In other words there could in theory be yet another wrapper inside and the repr(transparent) only gives us the wrapper's ABI and not T's.

@bors bors closed this as completed in b90277e Mar 11, 2023
saethlin pushed a commit to saethlin/miri that referenced this issue Mar 15, 2023
Add documentation about the memory layout of `Cell`

rust-lang/rust#101717 guaranteed the memory layout of `UnsafeCell<T>`.

This property (a guaranteed memory layout) can be useful to have on `Cell<T>` as well.

(Note that `Cell<u8>` [already doesn't trigger the `improper_ctypes` lint](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=34af59ef60b96d8a8bdaec1d52cb5420) since it is `#[repr(transparent)]`).

The concrete use-case is for the crate [`objc2`](https://github.com/madsmtm/objc2) to specify that `Cell<T>` is safe to use as an instance variable when `T` is.

Fixes rust-lang/rust#79303.

---

I'm unsure if we should specify less, for example say that the `Cell` may have extra restrictions on when it may be accessed, or if that's implicit in the (deliberately minimal) way I've worded it here?
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jun 1, 2023
Add documentation about the memory layout of `Cell`

rust-lang/rust#101717 guaranteed the memory layout of `UnsafeCell<T>`.

This property (a guaranteed memory layout) can be useful to have on `Cell<T>` as well.

(Note that `Cell<u8>` [already doesn't trigger the `improper_ctypes` lint](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=34af59ef60b96d8a8bdaec1d52cb5420) since it is `#[repr(transparent)]`).

The concrete use-case is for the crate [`objc2`](https://github.com/madsmtm/objc2) to specify that `Cell<T>` is safe to use as an instance variable when `T` is.

Fixes rust-lang/rust#79303.

---

I'm unsure if we should specify less, for example say that the `Cell` may have extra restrictions on when it may be accessed, or if that's implicit in the (deliberately minimal) way I've worded it here?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants