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

ManuallyDrop<T> where T: ?Sized #47034

Closed
mikeyhew opened this issue Dec 27, 2017 · 6 comments · Fixed by #53033
Closed

ManuallyDrop<T> where T: ?Sized #47034

mikeyhew opened this issue Dec 27, 2017 · 6 comments · Fixed by #53033
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@mikeyhew
Copy link
Contributor

ManuallyDrop<T> currently requires T: Sized. It would be nice if we could remove this restriction. I'm not sure how to do it with the current implementation using union though.

@kennytm kennytm added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 27, 2017
@jonas-schievink
Copy link
Contributor

Doesn't seem possible until we get by-value DSTs

@mikeyhew
Copy link
Contributor Author

@jonas-schievink care to explain why?

@jonas-schievink
Copy link
Contributor

Ah, nevermind. It would just need to make ManuallyDrop unsized, too.

@mikeyhew
Copy link
Contributor Author

The new and into_inner methods can keep could keep their Sized bounds, we just need to relax the Sized bound on the struct itself, and make sure ManuallyDrop<T>: Unsize<ManuallyDrop<U>> where T: Unsize<U>, U: ?Sized. Then the following code would be possible:

struct Foo<T>(ManuallyDrop<T>);

let foo = Box::new(Foo(ManuallyDrop::new(5i32))) as Box<Foo<::std::any::Any>;

@mikeyhew
Copy link
Contributor Author

I guess this would be possible by allowing union types to be ?Sized when there is only one variant. I was worried that would be unsound, but then I realized, the only way to create a DST union type would be from an unsize coercion, and then way you know what the erased Sized type is and can get the vtable/slice length then, just like any other unsize coercion.

@RalfJung
Copy link
Member

bors added a commit that referenced this issue Jul 28, 2018
Change ManuallyDrop<T> to a lang item.

This PR implements the approach @RalfJung proposes in https://internals.rust-lang.org/t/pre-rfc-unions-drop-types-and-manuallydrop/8025 (lang item `struct` instead of `union`).

A followup PR can easily solve #47034 as well, by just adding a few `?Sized` to `libcore/mem.rs`.

r? @nikomatsakis
kennytm added a commit to kennytm/rust that referenced this issue Aug 14, 2018
unsized ManuallyDrop

I think this matches what @eddyb had in rust-lang#52711 originally.

~~However, I have never added a `CoerceUnsized` before so I am not sure if I did this right. I copied the `unstable` attribute on the `impl` from elsewhere, but AFAIK it is useless because `impl`'s are insta-stable... so shouldn't this rather say "stable since 1.30"?~~

This is insta-stable and hence requires FCP, at least.

Fixes rust-lang#47034
bors added a commit that referenced this issue Aug 14, 2018
unsized ManuallyDrop

I think this matches what @eddyb had in #52711 originally.

~~However, I have never added a `CoerceUnsized` before so I am not sure if I did this right. I copied the `unstable` attribute on the `impl` from elsewhere, but AFAIK it is useless because `impl`'s are insta-stable... so shouldn't this rather say "stable since 1.30"?~~

This is insta-stable and hence requires FCP, at least.

Fixes #47034
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants