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

use const array repeat expressions for uninit_array #62799

Merged
merged 4 commits into from
Jul 22, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/liballoc/collections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ impl<K, V> LeafNode<K, V> {
LeafNode {
// As a general policy, we leave fields uninitialized if they can be, as this should
// be both slightly faster and easier to track in Valgrind.
keys: uninitialized_array![_; CAPACITY],
vals: uninitialized_array![_; CAPACITY],
keys: uninit_array![_; CAPACITY],
vals: uninit_array![_; CAPACITY],
parent: ptr::null(),
parent_idx: MaybeUninit::uninit(),
len: 0
Expand Down Expand Up @@ -159,7 +159,7 @@ impl<K, V> InternalNode<K, V> {
unsafe fn new() -> Self {
InternalNode {
data: LeafNode::new(),
edges: uninitialized_array![_; 2*B],
edges: uninit_array![_; 2*B],
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
#![feature(box_syntax)]
#![feature(cfg_target_has_atomic)]
#![feature(coerce_unsized)]
#![cfg_attr(not(bootstrap), feature(const_in_array_repeat_expressions))]
#![feature(dispatch_from_dyn)]
#![feature(core_intrinsics)]
#![feature(dropck_eyepatch)]
Expand Down
4 changes: 2 additions & 2 deletions src/libcore/fmt/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ trait GenericRadix {
// characters for a base 2 number.
let zero = T::zero();
let is_nonnegative = x >= zero;
let mut buf = uninitialized_array![u8; 128];
let mut buf = [MaybeUninit::<u8>::uninit(); 128];
let mut curr = buf.len();
let base = T::from_u8(Self::BASE);
if is_nonnegative {
Expand Down Expand Up @@ -189,7 +189,7 @@ static DEC_DIGITS_LUT: &[u8; 200] =
macro_rules! impl_Display {
($($t:ident),* as $u:ident via $conv_fn:ident named $name:ident) => {
fn $name(mut n: $u, is_nonnegative: bool, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut buf = uninitialized_array![u8; 39];
let mut buf = [MaybeUninit::<u8>::uninit(); 39];
let mut curr = buf.len() as isize;
let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf);
let lut_ptr = DEC_DIGITS_LUT.as_ptr();
Expand Down
23 changes: 20 additions & 3 deletions src/libcore/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,20 +626,37 @@ macro_rules! todo {
/// Creates an array of [`MaybeUninit`].
///
/// This macro constructs an uninitialized array of the type `[MaybeUninit<K>; N]`.
/// It exists solely because bootstrap does not yet support const array-init expressions.
///
/// [`MaybeUninit`]: mem/union.MaybeUninit.html
// FIXME: Remove both versions of this macro once bootstrap is 1.38.
#[macro_export]
#[unstable(feature = "maybe_uninit_array", issue = "53491")]
macro_rules! uninitialized_array {
#[cfg(bootstrap)]
macro_rules! uninit_array {
// This `assume_init` is safe because an array of `MaybeUninit` does not
// require initialization.
// FIXME(#49147): Could be replaced by an array initializer, once those can
// be any const expression.
($t:ty; $size:expr) => (unsafe {
MaybeUninit::<[MaybeUninit<$t>; $size]>::uninit().assume_init()
});
}

/// Creates an array of [`MaybeUninit`].
///
/// This macro constructs an uninitialized array of the type `[MaybeUninit<K>; N]`.
/// It exists solely because bootstrap does not yet support const array-init expressions.
///
/// [`MaybeUninit`]: mem/union.MaybeUninit.html
// FIXME: Just inline this version of the macro once bootstrap is 1.38.
#[macro_export]
#[unstable(feature = "maybe_uninit_array", issue = "53491")]
#[cfg(not(bootstrap))]
macro_rules! uninit_array {
($t:ty; $size:expr) => (
[MaybeUninit::<$t>::uninit(); $size]
);
}

/// Built-in macros to the compiler itself.
///
/// These macros do not have any corresponding definition with a `macro_rules!`
Expand Down
1 change: 1 addition & 0 deletions src/libcore/mem/maybe_uninit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ impl<T> MaybeUninit<T> {
/// [type]: union.MaybeUninit.html
#[stable(feature = "maybe_uninit", since = "1.36.0")]
#[inline(always)]
#[rustc_promotable]
Copy link
Member

Choose a reason for hiding this comment

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

Does this have support for feature gating?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. @oli-obk ?

Copy link
Member

Choose a reason for hiding this comment

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

Explicitly puting the MaybeUninit::uninit() seems to work. That would make this unnecessary.

https://play.integer32.com/?version=stable&mode=debug&edition=2018&gist=71886586e61fc9a677ce120c825c4aae

Copy link
Member Author

@RalfJung RalfJung Jul 20, 2019

Choose a reason for hiding this comment

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

That playground does not load here? I just get a blank page. The one at https://play.rust-lang.org/ works.

Can you paste the code you are suggesting here in GitHub?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah the playground at integer32 broken for me too.

The code was:

use std::mem::MaybeUninit;

fn main() {
    const A: MaybeUninit<u8> = MaybeUninit::uninit();
    let a_arr = [A; 2];
}

But looking at the optimized llvm ir, it seems that llvm emits a memset with zero, instead of keeping it uninitialized in case of:

use std::mem::MaybeUninit;

pub fn abc() -> [MaybeUninit<u8>; 16] {
    const A: MaybeUninit<u8> = MaybeUninit::uninit();
    let a_arr = [A; 16]; //unsafe { std::mem::uninitialized() };
    a_arr
}

Copy link
Member Author

Choose a reason for hiding this comment

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

MaybeUninit<u8> is Copy, so this does not really test the new code paths anyway.

But yes, you can probably "trigger" promotion as usual with a manual const.

pub const fn uninit() -> MaybeUninit<T> {
MaybeUninit { uninit: () }
}
Expand Down
4 changes: 2 additions & 2 deletions src/libcore/slice/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,14 @@ fn partition_in_blocks<T, F>(v: &mut [T], pivot: &T, is_less: &mut F) -> usize
let mut block_l = BLOCK;
let mut start_l = ptr::null_mut();
let mut end_l = ptr::null_mut();
let mut offsets_l: [MaybeUninit<u8>; BLOCK] = uninitialized_array![u8; BLOCK];
let mut offsets_l = [MaybeUninit::<u8>::uninit(); BLOCK];

// The current block on the right side (from `r.sub(block_r)` to `r`).
let mut r = unsafe { l.add(v.len()) };
let mut block_r = BLOCK;
let mut start_r = ptr::null_mut();
let mut end_r = ptr::null_mut();
let mut offsets_r: [MaybeUninit<u8>; BLOCK] = uninitialized_array![u8; BLOCK];
let mut offsets_r = [MaybeUninit::<u8>::uninit(); BLOCK];

// FIXME: When we get VLAs, try creating one array of length `min(v.len(), 2 * BLOCK)` rather
// than two fixed-size arrays of length `BLOCK`. VLAs might be more cache-efficient.
Expand Down