-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
update comment at MaybeUninit::uninit_array #83568
Conversation
r? @dtolnay (rust-highfive has picked a reviewer for you, use r? to override) |
/// The example below could then use `let mut buf = [MaybeUninit::<u8>::uninit(); 32];`. | ||
/// when Rust allows | ||
/// [inline const expressions](https://github.com/rust-lang/rust/issues/76001). | ||
/// The example below could then use `let mut buf = [const { MaybeUninit::<u8>::uninit() }; 32];`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it already possible to write let mut buf = [{ const U: MaybeUninit<u8> = MaybeUninit::uninit(); U}; 32];
thus making the method already unnecessary? Or is that too cumbersome to be considered usable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is possible if you have the concrete type; it won't when generic types are involved.
Not sure what people prefer here, for now fixing the docs seemed easier than arguing for the method to be removed. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, I missed the generic case.
Fixing the docs now is a good idea, I just encountered it earlier this week! But it was for a concrete type. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI @tspiteri that appears not to actually uninit.
#![feature(maybe_uninit_uninit_array)]
use std::mem::MaybeUninit;
pub fn f() -> [MaybeUninit<u8>; 32] {
[{ const U: MaybeUninit<u8> = MaybeUninit::uninit(); U}; 32]
}
pub fn g() -> [MaybeUninit<u8>; 32] {
MaybeUninit::uninit_array()
}
example::f:
movq %rdi, %rax
xorps %xmm0, %xmm0
movups %xmm0, 16(%rdi)
movups %xmm0, (%rdi)
retq
example::g:
movq %rdi, %rax
retq
Godbolt: https://rust.godbolt.org/z/8nnvPK1PM
A lot like #83657
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI @tspiteri that appears not to actually uninit.
Ah, good point, currently uninitialized const
s are not actually uninitialized it seems...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RalfJung What about @rodrimati1992's generic code here - for [MaybeUninit<T>; N]
, which compiles on Rust 1.51 / part of arrayvec 0.7? https://github.com/bluss/arrayvec/blob/7b290b7aa52b0c66b9056cfda577363d005f39b8/src/utils.rs
It seems to contradict what was said about generic types here (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's where it was noticed 🙂
@bors r+ rollup |
📌 Commit 5cfc98f has been approved by |
Rollup of 7 pull requests Successful merges: - rust-lang#83568 (update comment at MaybeUninit::uninit_array) - rust-lang#83571 (Constantify some slice methods) - rust-lang#83579 (Improve pointer arithmetic docs) - rust-lang#83645 (Wrap non-pre code blocks) - rust-lang#83656 (Add a regression test for issue-82865) - rust-lang#83662 (Update books) - rust-lang#83667 (Suggest box/pin/arc ing receiver on method calls) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
#49147 is closed; this now instead needs inline const expressions (#76001).