Improve trait safety documentation #283
Conversation
src/lib.rs
Outdated
| /// reference] for a description of how discriminant values are chosen). | ||
| /// - The type must not contain any [`UnsafeCell`]s (this is required in order | ||
| /// for it to be sound to construct a `&[u8]` and a `&T` to the same region of | ||
| /// memory). |
There was a problem hiding this comment.
Might be worth clarifying that it can point to UnsafeCell, it just can't have any inside the struct itself. So Option<&Cell<T>> is OK, but Cell<u8> is not.
There was a problem hiding this comment.
I wasn't sure how to phrase this since we don't implement FromZeroes for Option<&...> today. How does this sound?
The type must not contain any
UnsafeCells (this is required in order for it to be sound to construct a&[u8]and a&Tto the same region of memory). The type may contain references or pointers toUnsafeCells so long as those values can themselves be initialized from zeroes (FromZeroesis not currently implemented for, e.g.,Option<&UnsafeCell<_>>, but it could be one day).
There was a problem hiding this comment.
That sounds good, other than the generally applicable technical writing advice to avoid parentheticals longer than a word or two.
There was a problem hiding this comment.
Do you have any recommendations for how to phrase it with shorter parentheticals (or without any parentheticals)?
My thinking is that the text in parentheticals is useful to someone who wants a deeper intuition for the rules, but is not technically part of the rules themselves, and can be ignored by someone who's just trying to read the rules and doesn't care about understanding them. If the text weren't in parentheticals, it would make it feel like it was necessary to understand that text in order to understand how to apply the rules (which would be confusing since it obviously doesn't add any semantic content to the rules). If the text weren't present at all, readers who wanted to understand the reasoning behind the rules might be confused by what's problematic about UnsafeCell or might be confused by the implication that &UnsafeCell is FromZeroes since it clearly isn't.
There was a problem hiding this comment.
Gonna merge this for now; feel free to follow up with more discussion here and I'd be happy to update the docs in a follow-up PR.
For each of our unsafe traits, clarify who can skip reading the Safety section. For each of our unsafe traits (except for `Unaligned`): - Clarify that it must be sound to construct `&[u8]` and `&T` to the same memory region (this addresses #8) - Clarify that, in order to implement the trait, the type's fields need to satisfy the same requirements, but don't actually need to implement the trait - Clarify that, in order to implement the trait, the type must not contain any `UnsafeCell`s Closes #8
For each of our unsafe traits, clarify who can skip reading the Safety section.
For each of our unsafe traits (except for
Unaligned):&[u8]and&Tto the same memory region (this addresses Document soundness requirements around references #8)UnsafeCellsCloses #8