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

Tracking Issue for Vec::split_at_spare_mut #81944

Open
1 of 5 tasks
KodrAus opened this issue Feb 9, 2021 · 8 comments
Open
1 of 5 tasks

Tracking Issue for Vec::split_at_spare_mut #81944

KodrAus opened this issue Feb 9, 2021 · 8 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@KodrAus
Copy link
Contributor

KodrAus commented Feb 9, 2021

Feature gate: #![feature(vec_split_at_spare)]

This is a tracking issue for Vec::split_at_spare_mut, which is used as the basis for a few methods internally to give a slice pointing to the initialized portion of the Vec and a slice of MaybeUninit pointing to the uninitialized portion.

Public API

impl<T, A: Allocator> Vec<T, A> {
    pub fn split_at_spare_mut(&mut self) -> (&mut [T], &mut [MaybeUninit<T>]) {}
}

Steps / History

Unresolved Questions

  • Should we consider changing the name? split_at_spare_mut doesn't seem like an intuitive name
  • Should we deprecate Vec::spare_capacity_mut? Any usecase of Vec::spare_capacity_mut can be replaced with Vec::split_at_spare_mut (but not vise-versa):
    edit: spare_capacity_mut has been stabilised now in Stabilize vec_spare_capacity #93016 so deprecating it is probably not an option
@KodrAus KodrAus added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Feb 9, 2021
@Amanieu
Copy link
Member

Amanieu commented Feb 10, 2021

Interesting! I personally use spare_capacity_mut in my code to read into uninitialized buffers. I feel that it may be worth keeping it as a convenience method, but then again it is quite rarely used so I don't mind having to write an extra .1.

@WaffleLapkin
Copy link
Member

Hm, why there are no unresolved questions yet? It doesn't seem that the questions from the implementation PR were resolved:

  • Should we consider changing the name? split_at_spare_mut doesn't seem like an intuitive name
  • Should we deprecate Vec::spare_capacity_mut? Any usecase of Vec::spare_capacity_mut can be replaced with Vec::split_at_spare_mut (but not vise-versa)

@KodrAus
Copy link
Contributor Author

KodrAus commented Feb 21, 2021

@WaffleLapkin I just missed those questions that came up after the PR approval is all 🙂 They're in the tracking issue now.

@RalfJung
Copy link
Member

Any usecase of Vec::spare_capacity_mut can be replaced with Vec::split_at_spare_mut (but not vise-versa)

That is not true: split_at_spare_mut creates mutable references to the entire initialized part of the vector and will thus invalidate any preexisting pointers. spare_capacity_mut, if it is implemented carefully (which I have not checked) could be made to not invalidate existing pointers into the initialized part of the vector.

@WaffleLapkin
Copy link
Member

@RalfJung ohhh. I haven't thought of this. Then I guess the question "do we need both?" is resolved: yes.

if it is implemented carefully

Current implementation just calls self.split_at_spare_mut().1 😅

@RalfJung
Copy link
Member

Current implementation just calls self.split_at_spare_mut().1

Well, that doesn't count as "careful" I guess. ;)

@WaffleLapkin
Copy link
Member

I guess so 😅

I'll try to make the implementation more careful (revert it?) and make a PR :)

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 4, 2021
…lfJung

Revert `Vec::spare_capacity_mut` impl to prevent pointers invalidation

The implementation was changed in rust-lang#79015.

Later it was [pointed out](rust-lang#81944 (comment)) that the implementation invalidates pointers to the buffer (initialized elements) by creating a unique reference to the buffer. This PR reverts the implementation.

r? `@RalfJung`
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 4, 2021
…lfJung

Revert `Vec::spare_capacity_mut` impl to prevent pointers invalidation

The implementation was changed in rust-lang#79015.

Later it was [pointed out](rust-lang#81944 (comment)) that the implementation invalidates pointers to the buffer (initialized elements) by creating a unique reference to the buffer. This PR reverts the implementation.

r? ``@RalfJung``
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 4, 2021
…lfJung

Revert `Vec::spare_capacity_mut` impl to prevent pointers invalidation

The implementation was changed in rust-lang#79015.

Later it was [pointed out](rust-lang#81944 (comment)) that the implementation invalidates pointers to the buffer (initialized elements) by creating a unique reference to the buffer. This PR reverts the implementation.

r? ```@RalfJung```
@saethlin
Copy link
Member

saethlin commented Jan 4, 2022

Oops, what I commented here is far more relevant to Vec::spare_capacity_mut, comment moved: #75017 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants