Skip to content

docs: be less harsh in wording for Vec::from_raw_parts #99216

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

Merged
merged 5 commits into from
Oct 3, 2022
Merged
Changes from 1 commit
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
20 changes: 15 additions & 5 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,6 @@ impl<T> Vec<T> {
/// This is highly unsafe, due to the number of invariants that aren't
/// checked:
///
/// * `ptr` needs to have been previously allocated via [`String`]/`Vec<T>`
/// (at least, it's highly likely to be incorrect if it wasn't).
/// * `T` needs to have the same alignment as what `ptr` was allocated with.
/// (`T` having a less strict alignment is not sufficient, the alignment really
/// needs to be equal to satisfy the [`dealloc`] requirement that memory must be
Expand All @@ -500,6 +498,12 @@ impl<T> Vec<T> {
/// to be the same size as the pointer was allocated with. (Because similar to
/// alignment, [`dealloc`] must be called with the same layout `size`.)
/// * `length` needs to be less than or equal to `capacity`.
/// * `capacity` needs to be the capacity that the pointer was allocated with.
/// * The allocated size in bytes must be no larger than `isize::MAX`.
/// See the safety documentation of [`pointer::offset`].
///
/// To ensure these requirements are easily met, ensure `ptr` has previously
/// been allocated via `Vec<T>`.
///
/// Violating these may cause problems like corrupting the allocator's
/// internal data structures. For example it is normally **not** safe
Expand Down Expand Up @@ -648,14 +652,20 @@ impl<T, A: Allocator> Vec<T, A> {
/// This is highly unsafe, due to the number of invariants that aren't
/// checked:
///
/// * `ptr` needs to have been previously allocated via [`String`]/`Vec<T>`
/// (at least, it's highly likely to be incorrect if it wasn't).
/// * `T` needs to have the same size and alignment as what `ptr` was allocated with.
/// * `T` needs to have the same alignment as what `ptr` was allocated with.
/// (`T` having a less strict alignment is not sufficient, the alignment really
/// needs to be equal to satisfy the [`dealloc`] requirement that memory must be
/// allocated and deallocated with the same layout.)
/// * The size of `T` times the `capacity` (ie. the allocated size in bytes) needs
/// to be the same size as the pointer was allocated with. (Because similar to
/// alignment, [`dealloc`] must be called with the same layout `size`.)
/// * `length` needs to be less than or equal to `capacity`.
/// * `capacity` needs to be the capacity that the pointer was allocated with.
/// * The allocated size in bytes must be no larger than `isize::MAX`.
/// See the safety documentation of [`pointer::offset`].
///
/// To ensure these requirements are easily met, ensure `ptr` has previously
Copy link
Contributor

Choose a reason for hiding this comment

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

"ensure" here still sounds like it's a must.

I'd say something along the lines of "These requirements are always upheld by any ptr that has been allocated using a Vec<T>, but manual allocation is okay as long as the invariants are upheld."

That last bit might need some work, but if I read "ensure" in a doc, I read that as a "it is UB if this is not true".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point, I'll reword.

/// been allocated via `Vec<T>`.
///
/// Violating these may cause problems like corrupting the allocator's
/// internal data structures. For example it is **not** safe
Expand Down