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

RFC: Unsafe enums #724

Closed
wants to merge 6 commits into from
Closed

RFC: Unsafe enums #724

wants to merge 6 commits into from

Conversation

retep998
Copy link
Member

Unsafe enums are effectively untagged unions, useful for FFI.

Rendered

@retep998 retep998 changed the title Unsafe enums RFC: Unsafe enums Jan 23, 2015
@eddyb
Copy link
Member

eddyb commented Jan 23, 2015

If you already require Copy, you could go further and add a OIBIT-like trait which is implemented by all types that have valid values for all possible bit patterns in [u8; size_of::<T>()]. That way this "unsafe" enum could be safe.
As for annotation, another thought I had was #[repr(union)] but that might be too much for an attribute.

unsafe {
let Variant1(bar) = foo;
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be kind of annoying for when you want to only put the minimum number of non-unsafe things inside the unsafe block; you’d have to write something like this:

let x;
unsafe {
    let Variant1(bar) = foo;
    x = bar;
}
// use `x`

because unsafe creates its own scope. I don’t really have a better suggestion, but it’s good Rust style to put only actually unsafe stuff inside unsafe blocks, and this would make that a little harder.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps something like let unsafe { Variant1(bar) } = foo;? I'm not sure how feasible that would be to implement though.
If we went with the fancy restrictive trait as suggested by @eddyb then we'd be able to remove the unsafe blocks entirely making the resulting code much more concise.

@kennytm
Copy link
Member

kennytm commented Jan 24, 2015

@eddyb That is similar to http://discuss.rust-lang.org/t/new-type-kind-rawdata/996.

However, I think this is too restrictive as an unsafe enum may need to contain:

  • char (only valid from 0 to 0x10ffff without surrogates)
  • extern "C" fn(A) -> R (if the enum is safe we are able to call a dangling function in safe code)
    • Though we could restrict function pointers to unsafe extern "C" fn(A) -> R.

@eddyb
Copy link
Member

eddyb commented Jan 24, 2015

You can't call extern "ABI" fn(..A) -> R in safe code unless "ABI" is "Rust" or "rust-call" (which is the tupling/untupling hack we use for the Fn/FnMut/FnOnce traits).
But it can't be null, so only Option<extern "C" fn(..A) -> R> would actually be able to implement such a trait.
As for char - C doesn't have any concept of it, and you can always substitute with u32 (and do checked conversions).

@kennytm
Copy link
Member

kennytm commented Jan 24, 2015

@eddyb: I think you have mixed with extern "C" { /* ffi_functions */ }. You can call an extern "ABI" function pointer in safe code. playpen. The FFI functions generate unsafe extern "C" fn(A) -> R which are unsafe to call.

So ok an unsafe fn could be a RawData. Perhaps we could exclude char since the FFI code may be using u32 anyway.

@glaebhoerl
Copy link
Contributor

A somewhat more minimalistic idea for addressing the same problem.

@brson brson self-assigned this Jan 29, 2015
@brson
Copy link
Contributor

brson commented Feb 17, 2015

Although there's definitely a missing feature here, I think it's something we can hold off on for a while. I've opened #877 to keep track of it.

@retep998
Copy link
Member Author

retep998 commented Dec 6, 2015

I started a discussion on internals to get a push going for this again
https://internals.rust-lang.org/t/pre-rfc-unsafe-enums/2873

@joshtriplett joshtriplett mentioned this pull request Mar 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants