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

Add special case to ToPrimitive for enums with primitive representation #64

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

okamt
Copy link

@okamt okamt commented Sep 6, 2024

@cuviper
Copy link
Member

cuviper commented Sep 17, 2024

Do I understand correctly that this will convert non-unit enums based on their discriminant? If so, I think that's a strange use of ToPrimitive that we shouldn't encourage, since that will completely ignore the data in the enum. In other words, this is a numeric trait, but I think it's unlikely that the discriminant represents the entire numeric value of an enum type. Do you have an example otherwise?

@okamt
Copy link
Author

okamt commented Sep 18, 2024

Do I understand correctly that this will convert non-unit enums based on their discriminant?

Yes, my use case for this is an enum that represents different types of messages with a numeric ID like:

#[repr(u8)]
enum Packet {
    Ack = 0x00,
    Message { content: String } = 0x01,
}

Makes it easy to get the ID (discriminant) from a Packet value to encode it and send it over the network or whatever else. I thought it would make sense for ToPrimitive to work here since I was already using it to encode other unitary enums that I also need to send over the network based on their discriminant

@cuviper
Copy link
Member

cuviper commented Sep 19, 2024

Ok -- it makes sense that you want to extract the discriminant for that, but I don't think ToPrimitive is right for that. In particular, you wouldn't want to use that type with an arbitrary fn foo<T: ToPrimitive>. Even for your narrow use case, you would probably be better off for type safety with something that gave you the very specific repr type, and nothing else.

I don't know if another crate exists for that yet, but it would be nice!

@cuviper
Copy link
Member

cuviper commented Sep 19, 2024

This looks like a good fit: https://crates.io/crates/safe-discriminant
(although I haven't used it and can't vouch for it...)

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.

2 participants