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

Add index/iter methods for MaybeUninit<[T; N]> #88837

Closed
wants to merge 1 commit into from

Conversation

kpp
Copy link
Contributor

@kpp kpp commented Sep 10, 2021

While it's possible to create a MaybeUninit::<[T; N]> there is no way to index elements to initialize them other than to call .as_mut_ptr() and index over a raw pointer. I added several methods to make life a little bit easier. The idea is to get &[mut] MaybeUninit<T> from MaybeUninit<[T; N]> either through index or iter methods.

Use cases

#[test]
fn uninit_array_iter_mut() {
    let mut v = MaybeUninit::<[i32; 4]>::uninit();

    v.iter_mut().zip(1..=4).for_each(|(v, val)| {
        v.write(val);
    });

    let v = unsafe { v.assume_init() };
    assert_eq!(v, [1, 2, 4, 8]);
}

@rust-highfive
Copy link
Collaborator

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 10, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@the8472
Copy link
Member

the8472 commented Sep 10, 2021

That seems redundant with uninit_array and array_assume_init.

Maybe conversions between [MaybeUninit<T>; N] and MaybeUninit<[T; N]> should be added instead?

@nbdd0121
Copy link
Contributor

I agree with @the8472 that this is redundant, and uninit_array and array_assume_init should be used instead.

Plus, implementation of stable trait of stable type is insta-stable. The unstable annotation has no effect. Therefore this would need libs team signoff.

@kpp
Copy link
Contributor Author

kpp commented Sep 10, 2021

I guess you are right. ::transpose()?

@nbdd0121
Copy link
Contributor

What code would benefit from the transpose method?

@kpp
Copy link
Contributor Author

kpp commented Sep 10, 2021

Same as assume_init_ref/mut. This can be useful when we want to access it but don’t have ownership of the MaybeUninit.

@the8472
Copy link
Member

the8472 commented Sep 10, 2021

No new method is needed, two From impls should do the job.

@the8472 the8472 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 21, 2021
@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2021
@crlf0710
Copy link
Member

crlf0710 commented Oct 9, 2021

@kpp Ping from triage, seems CI is still red here.

@rust-log-analyzer

This comment has been minimized.

Add impl IndexMut for `MaybeUninit<[T; N]>`
Add impl IntoIterator for `&mut MaybeUninit<[T; N]>`
Add `MaybeUninit<[T; N]>::iter_mut` method
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling unwind v0.0.0 (/checkout/library/unwind)
error: implementation has missing stability attribute
    --> library/core/src/mem/maybe_uninit.rs:1164:1
     |
1164 | / impl<T, const N: usize> Index<usize> for MaybeUninit<[T; N]> {
1165 | |     type Output = MaybeUninit<T>;
1167 | |     #[inline]
...    |
1170 | |     }
1171 | | }
1171 | | }
     | |_^

error: implementation has missing stability attribute
    --> library/core/src/mem/maybe_uninit.rs:1173:1
     |
1173 | / impl<T, const N: usize> IndexMut<usize> for MaybeUninit<[T; N]> {
1175 | |     fn index_mut(&mut self, index: usize) -> &mut Self::Output {
1176 | |         IndexMut::index_mut(self.as_mut(), index)
1177 | |     }
1178 | | }
1178 | | }
     | |_^

error: implementation has missing stability attribute
    --> library/core/src/mem/maybe_uninit.rs:1180:1
     |
1180 | / impl<T, const N: usize> AsRef<[MaybeUninit<T>]> for MaybeUninit<[T; N]> {
1181 | |     #[inline]
1182 | |     fn as_ref(&self) -> &[MaybeUninit<T>] {
1183 | |         let data = self.as_ptr().cast::<MaybeUninit<T>>();
1187 | |     }
1188 | | }
     | |_^


error: implementation has missing stability attribute
    --> library/core/src/mem/maybe_uninit.rs:1190:1
     |
1190 | / impl<T, const N: usize> AsMut<[MaybeUninit<T>]> for MaybeUninit<[T; N]> {
1191 | |     #[inline]
1192 | |     fn as_mut(&mut self) -> &mut [MaybeUninit<T>] {
1193 | |         let data = self.as_mut_ptr().cast::<MaybeUninit<T>>();
1197 | |     }
1198 | | }
     | |_^


error: implementation has missing stability attribute
    --> library/core/src/mem/maybe_uninit.rs:1200:1
     |
1200 | / impl<'a, T, const N: usize> IntoIterator for &'a mut MaybeUninit<[T; N]> {
1201 | |     type Item = &'a mut MaybeUninit<T>;
1202 | |     type IntoIter = IterMut<'a, MaybeUninit<T>>;
...    |
1206 | |     }
1207 | | }
     | |_^
     | |_^

error: missing documentation for an associated function
    --> library/core/src/mem/maybe_uninit.rs:1211:5
     |
1211 |     pub fn iter_mut(&mut self) -> IterMut<'_, MaybeUninit<T>> {
     |
     |
     = note: `-D missing-docs` implied by `-D warnings`
error: could not compile `core` due to 6 previous errors
Build completed unsuccessfully in 0:01:16

@kpp
Copy link
Contributor Author

kpp commented Oct 9, 2021

I can't fight with implementation has missing stability attribute vs this stability annotation is useless. Would you please help me?

@the8472
Copy link
Member

the8472 commented Oct 9, 2021

The impl block for a trait needs a stability attribute, not the methods. But it must be stable since trait impls are insta-stable.
For inherent impls it's the other way around, the methods need the annotation but not the block. And there methods can be unstable.

@JohnCSimon
Copy link
Member

Ping from triage:
@kpp Can you please address the comment from the reviewer, and, if necessary set S-waiting-on-review? Thank you.

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 13, 2021
@bors
Copy link
Contributor

bors commented Nov 28, 2021

☔ The latest upstream changes (presumably #91311) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2021
@JohnCSimon
Copy link
Member

Ping from triage:
@kpp
I'm closing this due to inactivity, please feel to reopen when you are ready to continue with this. Thank you.

@JohnCSimon JohnCSimon closed this Jan 30, 2022
@JohnCSimon JohnCSimon added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

9 participants