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

Vec::reserve() (etc) panic conditions don't apply to zero-sized or empty types #119149

Closed
bsilver8192 opened this issue Dec 20, 2023 · 4 comments · Fixed by #119374
Closed

Vec::reserve() (etc) panic conditions don't apply to zero-sized or empty types #119149

bsilver8192 opened this issue Dec 20, 2023 · 4 comments · Fixed by #119374
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools

Comments

@bsilver8192
Copy link

Location

https://doc.rust-lang.org/std/vec/struct.Vec.html#method.reserve (and several other methods)

Summary

Vec::<()>::new().capacity() is usize::MAX, which is greater than isize::MAX. Calling reserve(1) on this vector does not panic, and the capacity afterwards is still greater than isize::MAX. The docs say this should unconditionally panic.

I would find it reasonable to rely on this documentation as part of ensuring soundness of some unsafe code interacting with Vec, without duplicating the assertion in my code.

This same comment is repeated on several other Vec methods, and the same thing applies.

For reference, #117437 and #99790 are another issue and a PR touching on this same sentence of the documentation.

@bsilver8192 bsilver8192 added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Dec 20, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 20, 2023
@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Dec 20, 2023

The docs say (emphasis mine):

Panics

Panics if the new capacity exceeds isize::MAX bytes.

A Vec::<()> will never have more than 0 bytes of capacity, and therefore reserve will never panic. The documentation is correct.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 20, 2023
@bsilver8192
Copy link
Author

Oh you're right, I read that many times and never processed the last word. That's the only place I see which talks about "capacity" as a number of bytes instead of a number of elements, which is easy to miss IMHO.

@Noratrieb
Copy link
Member

Might make sense to make thebytes bold.

@gurry
Copy link
Contributor

gurry commented Dec 28, 2023

Might make sense to make thebytes bold.

I have raised PR #119374 doing just that (except italicising it instead making it bold)

@bors bors closed this as completed in ef55e75 Dec 30, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 30, 2023
Rollup merge of rust-lang#119374 - gurry:119149-improve-vec-docs, r=cuviper

Italicise "bytes" in the docs of some `Vec` methods

On a cursory read it's easy to miss that the limit is in terms of bytes not no. of elements. The italics should help with that.

Fixes rust-lang#119149
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants