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

Implement From<E> for u32 when E is #[repr(u32)] enum E #2596

Open
Ekleog opened this issue Nov 16, 2018 · 9 comments
Open

Implement From<E> for u32 when E is #[repr(u32)] enum E #2596

Ekleog opened this issue Nov 16, 2018 · 9 comments

Comments

@Ekleog
Copy link

Ekleog commented Nov 16, 2018

I think everything is in the title. I was very surprised to see that appears not to be already done, so I guess I'm missing something.

Should this go through the RFC process or just get implemented directly as an obvious thing, given eg. u32: From<u16>?

@Ravenslofty
Copy link

From<T> for U implies Into<U> for T. Imagine your u32 is outside the valid discriminant range for the enum. That's undefined behaviour. Then what should the custom conversion function do? The conversion can't fail, so panic?

I think TryFrom might be a better solution here.

@oberien
Copy link

oberien commented Nov 16, 2018

From<E> for u32 implies Into<u32> for E, which allows you to call enum_value.into(). That is still totally valid. Only Into<E> for u32 would be problematic, but that would only be implied by From<u32> for E, which is not what is suggested here.

@Ravenslofty
Copy link

Okay, yeah, I got confused. Don't mind me.

@Ixrec
Copy link
Contributor

Ixrec commented Nov 16, 2018

I believe the most recent discussion on enum<->integer conversions is: https://internals.rust-lang.org/t/pre-rfc-enum-from-integer/6348

@Ekleog
Copy link
Author

Ekleog commented Nov 16, 2018

This other discussion appears to be mostly about int -> enum, isn't it? Here I'm just thinking of a trait (in this case From/Into) to do the same as as u32 already does, so that I could be generic over u32 and #[repr(u32)] enums :)

@bluss
Copy link
Member

bluss commented Nov 17, 2018

This seems to fit better as something that could be derived, but not be automatic. Representation and having the conversion available in the API are not exactly the same thing.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 18, 2018

Since we do have conversion by as casts already, it feels reasonable to also support From automatically, too

@briansmith
Copy link

Since we do have conversion by as casts already, it feels reasonable to also support From automatically, too

I disagree. The fact that as works for this was a mistake, IMO. Something might be #[repr(u32)] without wanting to expose the actual values used for each variant, so this shouldn't be the default.

@illicitonion
Copy link

I just published a full RFC proposing making From/Into derivable on such enums, and deprecating as conversions - comments very welcome!

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

No branches or pull requests

8 participants