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 EnumDiscriminants trait and provide for deriving it #176

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cole-miller
Copy link

@cole-miller cole-miller commented Aug 30, 2021

This PR adds the following trait to strum:

pub trait EnumDiscriminants {
    type Discriminant;

    fn discriminants(&self) -> Self::Discriminants;
}

It also causes derive(EnumDiscriminants) to generate an implementation of this trait, e.g.

#[derive(strum::EnumDiscriminants)]
enum E {
    X,
    Y(i32),
    Z(String),
}

generates (in addition to what it generated before) this impl:

impl EnumDiscriminants for E {
    type Discriminants = EDiscriminants; // (the type generated by the derive macro)

    fn discriminants(&self) -> Self::Discriminants {
        // actual generated impl uses fully qualified syntax
        self.into()
    }
}

I'm happy to write tests/documentation for this addition, but before I do, is this something you'd be interested in adding?

@Peternator7
Copy link
Owner

Hey @cole-miller, thanks for this PR! Abstractly, the idea makes sense to me; could you help me understand your use-case a little better? Are you trying to use this trait as a generic constraint, or do you want to a more descriptive method name than e.into() for getting the discriminants? In general, I'm onboard with the PR, I just want to make sure I understand the motivation.

@cole-miller
Copy link
Author

cole-miller commented Sep 7, 2021

I wrote this PR quickly after participating in this URLO thread, where the OP ended up wanting to get and store the enum discriminant of a value in a generic context:

https://users.rust-lang.org/t/parametric-counting-of-enum-variants-in-vec/64186

Sorry for not mentioning the motivation! I'm not sure whether this idea is generally useful, but it seemed like something potentially handy that strum could provide without much additional code, and I wanted to know what you/other maintainers thought of it.

(I do think that specific traits are preferable to using From and Into when there's some coherent shared behavior behind them. For one thing, it helps with type inference.)

@Peternator7
Copy link
Owner

I see; I'm not sure if it's exactly something I would use, but like you said, it's not much additional code, and there's some motivation for it from rust-users.

I'll happily accept a PR for this if you're still interested in implementing it. If not, I'm fine to leave this open until someone else comes along and pushes 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