Skip to content

Tracking Issue for Vec::split_at_spare_mut #81944

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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: An issue tracking the progress of sth. like the implementation of an RFC 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: An issue tracking the progress of sth. like the implementation of an RFC 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)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC 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