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

Documentation for VariantArray and VariantNames say their VARIANTS constants are arrays, but they're actually slices #335

Closed
IndigoLily opened this issue Feb 22, 2024 · 2 comments · Fixed by #343

Comments

@IndigoLily
Copy link

The incorrect documentation:

/// Implements `Strum::VariantNames` which adds an associated constant `VARIANTS` which is an array of discriminant names.

/// Adds a static array with all of the Enum's variants.

/// A trait for retrieving a static array containing all the variants in an Enum.

The actual types:

const VARIANTS: &'static [&'static str];

const VARIANTS: &'static [Self];

I personally think these constants should be actual arrays (because it's much easier to get a slice from an array than vice versa), but I'm not sure how feasible that is in current stable Rust.

@T0mstone
Copy link

Wow, I was about to write the same issue. What a coincidence!

You're right, making them true arrays won't work yet, since associated types aren't allowed to depend on associated constants. A workaround would be to make the array length a generic parameter of the trait, but that's pretty unwieldy, in addition to being semantically completely wrong.

I would be in favor of the macros also generating an inherent associated constant which is a true array. That way we can at least use the constant length in non-generic code.

@Peternator7
Copy link
Owner

There was discussion when the features were added about slices vs arrays. Like @T0mstone said, associated constants still aren't great to use. It also makes adding a new variant to an enum a breaking change because the length of the array changes, but slices don't have that issue.

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 a pull request may close this issue.

3 participants