Stream: general

Topic: maybeuninit not copy


gnzlbg (Oct 15 2019 at 09:28, on Zulip):

@RalfJ why isn't MaybeUninit Copy ?

RalfJ (Oct 15 2019 at 09:35, on Zulip):

It is? (If T is)

gnzlbg (Oct 15 2019 at 09:35, on Zulip):

Why the "If T is" constraint ?

gnzlbg (Oct 15 2019 at 09:36, on Zulip):

(why isn't it unconditionally Copy?)

centril (Oct 15 2019 at 09:43, on Zulip):

Feels like a footgun possibly

gnzlbg (Oct 15 2019 at 10:01, on Zulip):

Is there rationale for this somewhere?

gnzlbg (Oct 15 2019 at 10:09, on Zulip):

https://github.com/rust-lang/rust/issues/53491#issuecomment-443477750

gnzlbg (Oct 15 2019 at 10:10, on Zulip):

I wish there was a Copy wrapper type so that I could just write MaybeUninit<CopyWrapper<NotCopy>>

gnzlbg (Oct 15 2019 at 10:11, on Zulip):

Otherwise MaybeUninit cannot be used with !Copy types

gnzlbg (Oct 15 2019 at 10:11, on Zulip):

The current warning that says that code should stop using mem::uninitialized and use MaybeUninit instead is therefore being emitted too early

gnzlbg (Oct 15 2019 at 10:12, on Zulip):

Since if the type involved is not Copy, then you can't use MaybeUninit for it

gnzlbg (Oct 15 2019 at 10:17, on Zulip):

Opened: https://github.com/rust-lang/rust/issues/65432

centril (Oct 15 2019 at 10:20, on Zulip):

@gnzlbg yea we can add a "note: ..." to impl Copy

gnzlbg (Oct 15 2019 at 10:28, on Zulip):

@centril

Otherwise MaybeUninit cannot be used with !Copy types

Wait, but it can ?

gnzlbg (Oct 15 2019 at 10:28, on Zulip):

MaybeUninit<NotCopy> works ?

centril (Oct 15 2019 at 10:29, on Zulip):

T: Copy => MaybeUninit<T>: Copy

gnzlbg (Oct 15 2019 at 10:29, on Zulip):

Sure, but MaybeUninit is a union, and if T is not Copy, then that would create an union with a NonCopy field

centril (Oct 15 2019 at 10:29, on Zulip):

WF(MaybeUninit<T>) does not require T: Copy

centril (Oct 15 2019 at 10:29, on Zulip):

@gnzlbg did you assume ^--- ?

gnzlbg (Oct 15 2019 at 10:30, on Zulip):

I was trying to play with my own maybeuninit, and got the mem::uninitialized warning

gnzlbg (Oct 15 2019 at 10:30, on Zulip):

(and with ManuallyDrop)

centril (Oct 15 2019 at 10:30, on Zulip):

So you only need T: Copy if you need the whole MaybeUninit<T> to also be Copy

centril (Oct 15 2019 at 10:31, on Zulip):

is that what you need tho?

gnzlbg (Oct 15 2019 at 10:33, on Zulip):

I wanted the whole MaybeUninit<T> to be Copy even if T is not Copy to be able to put it into an union

gnzlbg (Oct 15 2019 at 10:33, on Zulip):

but that cannot work

rkruppe (Oct 15 2019 at 10:33, on Zulip):

I've seen #65432 and I still don't understand why Copy would ever be relevant for replacing mem::uninit with MaybeUninit

gnzlbg (Oct 15 2019 at 10:33, on Zulip):

yet somehow MaybeUninit makes it work

centril (Oct 15 2019 at 10:33, on Zulip):

@gnzlbg I think the larger context might be missing here

centril (Oct 15 2019 at 10:33, on Zulip):

+/- some code

rkruppe (Oct 15 2019 at 10:33, on Zulip):

^ Can you please take a step back and explain from the start, from first principles, what the problem is

gnzlbg (Oct 15 2019 at 10:35, on Zulip):

I wanted to put a MaybeUninit<T: !Copy> in a field that requires Copy

gnzlbg (Oct 15 2019 at 10:35, on Zulip):

that happened to be an union field

gnzlbg (Oct 15 2019 at 10:35, on Zulip):

that did not work on stable

rkruppe (Oct 15 2019 at 10:36, on Zulip):

If T is not Copy, then you can't put the T there directly (using mem::uninitialized instead of MaybeUninit) either

gnzlbg (Oct 15 2019 at 10:36, on Zulip):

I tried to use ManuallyDrop like MaybeUninit does but that did not work

gnzlbg (Oct 15 2019 at 10:37, on Zulip):

I concluded without checking that MaybeUninit<T> can therefore not work for T: !Copy

gnzlbg (Oct 15 2019 at 10:37, on Zulip):

But it does, because its magic

gnzlbg (Oct 15 2019 at 10:37, on Zulip):

@rkruppe you can do that on unstable Rust with untagged unions

rkruppe (Oct 15 2019 at 10:38, on Zulip):

Exactly, MaybeUninit isn't magic, just using an unstable feature

centril (Oct 15 2019 at 10:38, on Zulip):

(which is being worked on by Oliver atm)

rkruppe (Oct 15 2019 at 10:39, on Zulip):

And once "unions fields don't have to be Copy, just must not have drop glue" is implemented and stabilized, you can do it on stable too (with ManuallyDrop)

rkruppe (Oct 15 2019 at 10:39, on Zulip):

union Foo<T> { t: T }, by the way, won't work after that's implemented

gnzlbg (Oct 15 2019 at 10:40, on Zulip):

Do we get a bound for T ?

rkruppe (Oct 15 2019 at 10:40, on Zulip):

No

rkruppe (Oct 15 2019 at 10:40, on Zulip):

https://rust-lang.github.io/rfcs/2514-union-initialization-and-drop.html

gnzlbg (Oct 15 2019 at 10:40, on Zulip):

MaybeUninit is magic because it can use unstable features on stable, just like anything in libstd

gnzlbg (Oct 15 2019 at 10:41, on Zulip):

There is quite a bit of code using union Foo<T> { t: T }

rkruppe (Oct 15 2019 at 10:41, on Zulip):

"using unstable features" is a very low threshold for "magic"

rkruppe (Oct 15 2019 at 10:41, on Zulip):

That code is all on nightly and using an unstable feature

gnzlbg (Oct 15 2019 at 10:41, on Zulip):

Sure, and we can break it

gnzlbg (Oct 15 2019 at 10:42, on Zulip):

I'm just saying that the breakage might be massive

centril (Oct 15 2019 at 10:42, on Zulip):

Also doesn't seem very onerous to transition, rather mechanic?

centril (Oct 15 2019 at 10:42, on Zulip):

@gnzlbg just wait until specialization needs to be rearchitected :P

gnzlbg (Oct 15 2019 at 10:42, on Zulip):

union Foo<T> { t: ManuallyDrop<T> } should work

centril (Oct 15 2019 at 10:42, on Zulip):

you ain't seen nothing yet... :D

centril (Oct 15 2019 at 10:43, on Zulip):

@rkruppe aside: maybe you want in on reviewing https://github.com/rust-lang/rust/pull/62330? seems up your alley

gnzlbg (Oct 15 2019 at 10:43, on Zulip):

Like, there is quite a bit of code using union U<T, U> { a: T, b: U } as a way to do transmutes in statics

gnzlbg (Oct 15 2019 at 10:43, on Zulip):

Because mem::transmute isn't a const fn

gnzlbg (Oct 15 2019 at 10:44, on Zulip):

A warning of the type "don't use unions for transmutes, use mem::transmute instead" would help

centril (Oct 15 2019 at 10:45, on Zulip):

I'm not a fan of making mem::transmute more magic than it already is

rkruppe (Oct 15 2019 at 10:45, on Zulip):

Feel free to implement it but I can't honestly say that this is high on my list of priorities especially when everyone's going to get a reasonable clear compiler error in (hopefully) a few weeks anyway

gnzlbg (Oct 15 2019 at 10:46, on Zulip):

Starting by the standard library :D

rkruppe (Oct 15 2019 at 10:46, on Zulip):

Play stupid games, win stupid prizes

centril (Oct 15 2019 at 10:47, on Zulip):

(I assume "it" is referring to the warning and not reviewing #62330)

gnzlbg (Oct 15 2019 at 10:47, on Zulip):

Not really, since that probably isn't Olis fault, yet the PR that tries to land it will need to fix things there

rkruppe (Oct 15 2019 at 10:48, on Zulip):

(Yes, "it" = such a warning)

centril (Oct 15 2019 at 10:50, on Zulip):

I'm sure that Oliver would be happy to help if he has the time. He's quite a nice fellow! :slight_smile:

gnzlbg (Oct 15 2019 at 10:53, on Zulip):

What I was trying to say is that PRs that change nightly features need to change all components that use them for the PR to be merged.

gnzlbg (Oct 15 2019 at 10:53, on Zulip):

You mentioned specialization before. Whoever tries to land a PR that changes the current semantics would need to workaround everything in the standard library that uses it.

gnzlbg (Oct 15 2019 at 10:54, on Zulip):

That can be done, but is a lot of work.

centril (Oct 15 2019 at 10:54, on Zulip):

@gnzlbg oh; that seems like a given, yes

gnzlbg (Oct 15 2019 at 10:54, on Zulip):

The longer the feature remains unstable, particularly when the feature is useful, the larger the problem.

centril (Oct 15 2019 at 10:54, on Zulip):

but that should mostly be "boring mechanic work"

rkruppe (Oct 15 2019 at 10:55, on Zulip):

Yes, that is true. Why are you stressing it? You don't seem to advocate any alternative, or draw other consequences from it. It's a true observation, now what?

RalfJ (Oct 15 2019 at 10:56, on Zulip):

Otherwise MaybeUninit cannot be used with !Copy types
Sure, but MaybeUninit is a union, and if T is not Copy, then that would create an union with a NonCopy field

you didn't test this, did you? MaybeUninit works fine for allT. it's not always Copy but it works just fine. You rarely want to copy a MaybeUninit.

RalfJ (Oct 15 2019 at 10:57, on Zulip):

@gnzlbg what you are asking for is https://github.com/rust-lang/rust/pull/62330

RalfJ (Oct 15 2019 at 10:57, on Zulip):

this has nothing to do with MaybeUninit, and only coincidentally anything to do with Copy

RalfJ (Oct 15 2019 at 10:58, on Zulip):

ah that has all been said, good :D

centril (Oct 15 2019 at 10:58, on Zulip):

Yeah; Oliver just needs to do some $dayjob first :P

centril (Oct 15 2019 at 10:58, on Zulip):

(before fixing the stuff in the PR)

gnzlbg (Oct 15 2019 at 10:58, on Zulip):

It's a true observation, now what?

Maybe be more conservative with which unstable features are used from within the toolchain ?

gnzlbg (Oct 15 2019 at 10:58, on Zulip):

The bar for that is now zero.

RalfJ (Oct 15 2019 at 10:59, on Zulip):

so let me just note in closing that MaybeUninit is actually not special-cased in the compiler; rather it's a union where the init field has type ManuallyDrop<T>. which will work in unions for any T once we stabilize it.

centril (Oct 15 2019 at 10:59, on Zulip):

@RalfJ well except for the repr(transparent) aspect... -ish

RalfJ (Oct 15 2019 at 10:59, on Zulip):

yeah well that's a separate story

centril (Oct 15 2019 at 10:59, on Zulip):

yep

gnzlbg (Oct 15 2019 at 11:00, on Zulip):

Why the "-ish" ?

rkruppe (Oct 15 2019 at 11:00, on Zulip):

(muffled suppressed ranting about transparent unions)

gnzlbg (Oct 15 2019 at 11:00, on Zulip):

I thought the RFC was merged

gnzlbg (Oct 15 2019 at 11:00, on Zulip):

and implemented

centril (Oct 15 2019 at 11:00, on Zulip):

@gnzlbg it is not completely true that it is special cased... it could ostensibly be stabilized for everyone

centril (Oct 15 2019 at 11:01, on Zulip):

it's the only thing using the feature tho atm

gnzlbg (Oct 15 2019 at 11:01, on Zulip):

Did it end up not working for MaybeUninit?

centril (Oct 15 2019 at 11:01, on Zulip):

I think it worked?

gnzlbg (Oct 15 2019 at 11:01, on Zulip):

That's why I thought too

rkruppe (Oct 15 2019 at 11:01, on Zulip):

It's a true observation, now what?

Maybe be more conservative with which unstable features are used from within the toolchain ?

Sure, that is a thing you could advocate. I don't think you'd be very successful, though, absent hard evidence that the effort of updating parts of libstd/rustc when changing some unstable feature is a real problem outweighing the benefit of using that feature.

gnzlbg (Oct 15 2019 at 11:02, on Zulip):

@rkruppe @bjorn3 is trying to get stdarch to work on cranelift

centril (Oct 15 2019 at 11:02, on Zulip):

I would probably agree with @gnzlbg's point when it comes to unordered

gnzlbg (Oct 15 2019 at 11:02, on Zulip):

let's just say trying to map thousands of link_llvm_intrinsic to cranelift isn't fun

centril (Oct 15 2019 at 11:02, on Zulip):

and in some cases I might require tests in tidy or something

centril (Oct 15 2019 at 11:03, on Zulip):

@gnzlbg we're careful about not exposing e.g. specialization in a problematic way tho

centril (Oct 15 2019 at 11:03, on Zulip):

and we can always remove those impls

centril (Oct 15 2019 at 11:03, on Zulip):

(from a semantic POV)

rkruppe (Oct 15 2019 at 11:04, on Zulip):

@gnzlbg So that sucks, but what's the alternative there that's (1) easier on Cranelift and (2) would have been acceptable during development of std::arch?

gnzlbg (Oct 15 2019 at 11:04, on Zulip):

We had "platform-intrinsics" before. That would have been easier on Cranelift, and probably miri as well.

rkruppe (Oct 15 2019 at 11:05, on Zulip):

I am extremely skeptical

rkruppe (Oct 15 2019 at 11:05, on Zulip):

In any case this is all hypothetical and I have better things to do

RalfJ (Oct 16 2019 at 17:19, on Zulip):

I dont see what intrinsics have to do with MaybeUninit using an unstable union feature though...

Last update: Nov 22 2019 at 00:20UTC