Skip to content

Make explicit guarantees about Vec’s allocator #145260

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SabrinaJewson
Copy link
Contributor

This commit amends the documentation of Vec::as_mut_ptr and Vec::into_raw_parts to make it explicit that such calls may be paired with calls to dealloc with a suitable layout. This guarantee was effectively already provided by the docs of Vec::from_raw_parts mentioning alloc.

Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of std::boxed to std::vec. This explains the allocator guarantees in more detail.

This commit amends the documentation of `Vec::as_mut_ptr` and
`Vec::into_raw_parts` to make it explicit that such calls may be paired
with calls to `dealloc` with a suitable layout. This guarantee was
effectively already provided by the docs of `Vec::from_raw_parts`
mentioning `alloc`.

Additionally, we copy-paste and adjust the “Memory layout” section from
the documentation of `std::boxed` to `std::vec`. This explains the allocator
guarantees in more detail.
@rustbot
Copy link
Collaborator

rustbot commented Aug 11, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 11, 2025
Copy link
Member

@Kivooeo Kivooeo left a comment

Choose a reason for hiding this comment

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

Some nits

@ibraheemdev
Copy link
Member

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Aug 11, 2025
@rustbot rustbot assigned dtolnay and unassigned ibraheemdev Aug 11, 2025
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

This guarantee was effectively already provided by the docs of Vec::from_raw_parts mentioning alloc.

This is also my understanding. If you can feed an arbitrary heap allocation into Vec::from_raw_parts and get it back from into_raw_parts / as_mut_ptr / capacity, it's effectively a guarantee that these things can be deallocated in the obvious way.

There is one other piece required to complete the argument. The above by itself does not guarantee that a ZST vector ptr is fine to "leak" as written in this PR: "and if T is zero-sized nothing needs to be done". This scenario falls outside any consequences drawn from from_raw_parts because from_raw_parts is documented as follows: "ptr must have been allocated using the global allocator, such as via the alloc::alloc function", and allocating a zero-sized layout with a global allocator is documented as being UB. If Vec somehow allocated nonzero size internally for ZST vectors, this PR would be a new guarantee. But there is plenty of existing documentation that Vec does not allocate for ZST.

Hypothetically, dropping a vector of ZST could require some other non-allocator thing, such as std::internal::decrement_outstanding_zst_vectors(). But this is pretty outlandish and I do not believe we require a team consensus to rule this out.

Thank you for the improvements!

Separately, it would be good to update the documentation of from_raw_parts to speak about the ZST case. As currently written, it seems to imply round-tripping into_raw_parts -> from_raw_parts can be UB, which is not the intention.

@dtolnay
Copy link
Member

dtolnay commented Aug 11, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 11, 2025

📌 Commit 45e2449 has been approved by dtolnay

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants