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 ManuallyDrop #62

Open
bluss opened this Issue Jul 30, 2017 · 12 comments

Comments

Projects
None yet
3 participants
@bluss
Copy link
Owner

bluss commented Jul 30, 2017

ManuallyDrop is stable in Rust 1.20 (not yet released).

This is the official nodrop that arrayvec has been waiting a long time for; it's likely that we will use Rust 1.20 as the new required version from this point.

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Aug 2, 2017

Consider using version_check which is small enough to be worth it(?)

@tbu-

This comment has been minimized.

Copy link
Collaborator

tbu- commented Aug 3, 2017

This can only get the version at runtime – do you want to add a build script?

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Aug 3, 2017

Yep, exactly.

Example https://github.com/bluss/debugit/blob/master/build.rs (this one is checking nightly and not the version..)

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Aug 6, 2017

We can handle this without breaks, as long as the minimum rust version is reasonable (not 1.2).

However, the question of uninit data inside manuallydrop is unresolved, so we can't really start to use it without including other workarounds too.

@bluss bluss removed the breaking-change label Aug 6, 2017

@niklasf

This comment has been minimized.

Copy link
Contributor

niklasf commented Sep 29, 2017

Looks like f33c4e4 snuck in ManuallyDrop if the use_union feature is set 🎉

Is there anything left to do, besides version detection, before it can entirely replace nodrop?

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Sep 29, 2017

sigh, that was not intended :(

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Sep 29, 2017

It's fine to use ManuallyDrop for use_union. What remains for arrayvec 1.0 is to answer the question about uninitialized data in ManuallyDrop. There was some disagreement in #rustc.

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Sep 29, 2017

Thanks for pointing out that it snuck in, by the way. It was obviously not intended to sneak in.

@tbu-

This comment has been minimized.

Copy link
Collaborator

tbu- commented Oct 1, 2017

Can you link to the discussion about uninit data in ManuallyDrop?

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Oct 1, 2017

The discussion is here: https://botbot.me/mozilla/rustc/2017-08-24/?msg=90238011&page=3

There are different opinions. I don't think specific people are wrong here, but my collected impression was that there is no consensus on the safety of uninitialized in ManuallyDrop

@tbu-

This comment has been minimized.

Copy link
Collaborator

tbu- commented Oct 1, 2017

Your idea of making a union union MaybeInitialized<T>{ t: T, uninit: () } sounds good and forward-compatible if the O(n) cost gets optimized out by the compiler.

@bluss

This comment has been minimized.

Copy link
Owner Author

bluss commented Nov 4, 2017

Current wip is #76

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.