Skip to content

docs: Mention spare_capacity_mut() in Vec::set_len #126118

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 1 commit into from
Dec 20, 2024
Merged

docs: Mention spare_capacity_mut() in Vec::set_len #126118

merged 1 commit into from
Dec 20, 2024

Conversation

jan-ferdinand
Copy link
Contributor

I recently went down a small rabbit hole when trying to identify safe use of Vec::set_len. The solution was Vec::spare_capacity_mut. I think the docs on Vec::set_len benefit from mentioning this method.

A possible counter-argument could be that the clippy lint uninit_vec already nudges people in the right direction. However, I think a working example on Vec::set_len is still beneficial.

Happy to hear your thoughts on the matter. 😊

@rustbot
Copy link
Collaborator

rustbot commented Jun 7, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@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 Jun 7, 2024
@rust-log-analyzer

This comment has been minimized.

/// unsafe {
/// vec.set_len(256);
/// }
/// ```
Copy link
Member

Choose a reason for hiding this comment

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

It might even be worth it to move this above the FFI example since this use usable with pure rust.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to amend the commit accordingly. Who'd be an authority for such decisions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Who'd be an authority for such decisions?

Whoever says something that makes the most sense, which could be you :) docs changes are usually EAFP. (agreed that it makes sense to move this up).

Comment on lines 1473 to 1475
/// unsafe {
/// vec.set_len(256);
/// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor style:

Suggested change
/// unsafe {
/// vec.set_len(256);
/// }
/// unsafe { vec.set_len(256) };

Copy link
Contributor Author

@jan-ferdinand jan-ferdinand Jun 14, 2024

Choose a reason for hiding this comment

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

Though I'd also write it like you suggest, I've based this on the existing examples for the same method and opted for consistency.

@m-ou-se
Copy link
Member

m-ou-se commented Aug 19, 2024

Maybe it's best to just link to the documentation+example of spare_capacity_mut? Or to re-use the example from that method's documentation?

@jan-ferdinand
Copy link
Contributor Author

I feel queasy about copy-pasting the example from spare_capacity_mut. I've amended the commit with a new suggestion. 😌

@the8472 the8472 assigned the8472 and unassigned m-ou-se Nov 13, 2024
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

This minimal change looks fine to me, I think we should be able to merge this unless @the8472 you have other suggestions.

@the8472
Copy link
Member

the8472 commented Dec 20, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 20, 2024

📌 Commit 07ba1fd has been approved by the8472

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 Dec 20, 2024
@bors bors merged commit 3cdc3b7 into rust-lang:master Dec 20, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 20, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants