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

turn mem::uninitialized into a constant function #50150

Closed
wants to merge 1 commit into from

Conversation

japaric
Copy link
Member

@japaric japaric commented Apr 21, 2018

one can implement a const fn version of mem::uninitialized using unions (see below) so there's no real reason to not make the standard version const, IMO.

#![feature(const_fn)]
#![feature(untagged_unions)]

fn main() {
    static mut X: u32 = unsafe { uninitialized() };
    println!("{}", unsafe { X });
}

const unsafe fn uninitialized<T>() -> T {
    union U<T> {
        none: (),
        some: T,
    }

    U { none: () }.some
}

r? @oli-obk
cc @rust-lang/libs

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 21, 2018
match dest {
Place::Local { frame, local } => ecx.modify_local(frame, local, uninit)?,
Place::Ptr {
ptr,
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 copy pasted this code from src/tools/miri. The miri version had ptr: PtrAndAlign { ptr, aligned: true } in the pattern, but that got lost in translation. How do I check that ptr is aligned in this context? Pointer API didn't have any method to check for alignment AFAICT.

Copy link
Member

Choose a reason for hiding this comment

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

Did you use the old code? You could try looking again, now that miri has been updated.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 22, 2018

At the all-hands @nikomatsakis and @RalfJung mentioned that they want to eliminate the uninitiated intrinsic because it's not very good to reason about. Instead one should be using a union. Not sure in which way that should be happening though.

@RalfJung
Copy link
Member

RalfJung commented Apr 22, 2018

@japaric Indeed you can implement mem::uninitialized that way but it is extremely dangerous. As @oli-obk mentioned, we want to deprecate that function. See #49248 for an example of the kinds of problems it creates.

Here's the RFC for the replacement: rust-lang/rfcs#1892. I'd rather see us not allow or encourage more uses of mem::uninitialized; instead, we should allow const code to use MaybeUninot once that is stable.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 22, 2018

Stability is not the issue at hand. We can make the methods on MaybeUninit const fn behind a feature gate for now

@japaric
Copy link
Member Author

japaric commented Apr 23, 2018

@RalfJung MaybeUninit sounds fine to me; I'd be OK with using that instead of mem::uninitialized.

Does mem::zeroed / intrinsics::init have the same problem? I was planning to send a follow up PR to make that function into a const fn. My use case is being able to initialize a fixed capacity hash map in a static variable, and the only way to have const constructor is to turn mem::zeroed into a const fn: I have to initialize GenericArray<Option<NonZeroU32>, N> (GenericArray<T, N> is a hack / workaround for [T; N]) to a bunch of zeros and GenericArray has no const constructor (I think it's not even possible to write one). Also, mem::zeroed can't be implemented using unions; this (see below) doesn't work due to rustc limitations:

#![feature(const_fn)]
#![feature(untagged_unions)]

use std::mem;

const unsafe fn zeroed<T>() -> T {
    union U<T> {
        bytes: [u8; mem::size_of::<T>()],
        data: T,
    }

    U {
        bytes: [0; mem::size_of::<T>()],
    }.data
}

@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2018

@japaric I think mem::zeroed is fine

Why do you need mem::zeroed? Can't you start marking all the relevant constructors as const?

@RalfJung
Copy link
Member

@japaric mem::zeroed has the same issues as mem::uninitialized -- you can use it to create an inhabitant of the empty type.

It can be reimplemented in a sane way on top of MaybeUninit using something like

impl<T> MaybeUninit<T> {
  pub fn zeroed() -> Self {
    let mut u = MaybeUninit::uninitialized();
    ptr::write_bytes(&mut u as *mut MaybeUninit<T>, 0u8, 1);
    u
  }
}

@bors
Copy link
Contributor

bors commented Apr 27, 2018

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

@shepmaster
Copy link
Member

Ping from triage, @japaric and @oli-obk — where does this PR fall in the process at the moment?

@oli-obk
Copy link
Contributor

oli-obk commented May 7, 2018

This won't be happening, as the function is supposed to get deprecated at some point.

@oli-obk oli-obk closed this May 7, 2018
alevy added a commit to alevy/tock that referenced this pull request Jul 25, 2018
Uses a trick stolen from @japaric here:
rust-lang/rust#50150 to avoid using
`mem::uninitialized`, which isn't currently marked `const`
@glandium
Copy link
Contributor

This won't be happening, as the function is supposed to get deprecated at some point.

I'll note that this would be useful even if the function is going to be deprecated, because MaybeUninit doesn't seem to come any time soon. The alternative horror I'm currently using is ... to define some statics as extern "C" and to (re)define them in a C file, uninitialized.

@RalfJung
Copy link
Member

Useful, yes -- for about 2 or 3 releases or so. Then, for the rest of time, it's a liability. I don't think we should stabilize and support forever (!) a feature we know to be broken. Instead, we should focus on making progress on the proper way of expressing this. (And we are making progress.)

I am sorry that you have to use horrible work-arounds. That's not great, and we should fix it. But introducing something half-broken is not "fixing" anything.

Also, AFAIK, C zero-initializes statics by default...

@glandium
Copy link
Contributor

Also, AFAIK, C zero-initializes statics by default...

Yes, and I'd totally be okay with std::mem::zeroed() being const. And rust-lang/rfcs#411 has been ignored in favor of MaybeUninit too.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2018

You can work around this with a custom union, can you not?

union MaybeUninit<T: Copy> {
    uninit: (),
    init: T
}

static mut FOO: MaybeUninit<u32> = MaybeUninit { uninit: () };

@glandium
Copy link
Contributor

glandium commented Nov 22, 2018

I was starting to wonder if I can't just copy/paste MaybeUninit.

Edit: Ah, not because of const_fn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants