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

update comment at MaybeUninit::uninit_array #83568

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

RalfJung
Copy link
Member

#49147 is closed; this now instead needs inline const expressions (#76001).

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(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 Mar 27, 2021
/// 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];`.
Copy link
Contributor

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?

Copy link
Member Author

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. ;)

Copy link
Contributor

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. ;)

Copy link
Member

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

Copy link
Member Author

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 consts are not actually uninitialized it seems...

Copy link
Member

@bluss bluss Mar 30, 2021

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 (?)

Copy link
Member Author

Choose a reason for hiding this comment

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

@bluss associated const are the only generic consts we currently have. So yes that is a way to work around this limitation.

This will also run into #83657 though.

Copy link
Member

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 🙂

@dtolnay
Copy link
Member

dtolnay commented Mar 30, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 30, 2021

📌 Commit 5cfc98f has been approved by dtolnay

@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 Mar 30, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2021
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
@bors bors merged commit 9ab5f7d into rust-lang:master Mar 30, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 30, 2021
@RalfJung RalfJung deleted the uninit_array branch April 6, 2021 09:02
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants