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

TryFromPrimitive should return an error on default and catch_all #143

Open
AlexSherbinin opened this issue Mar 10, 2024 · 1 comment
Open

Comments

@AlexSherbinin
Copy link

Now when using #[num_enum(default)] or #[num_enum(default)] with TryFromPrimitive it just ignores them.
But it's more reasonable to return error because of missing support.

@illicitonion
Copy link
Owner

This is a really interesting question to consider, thanks for raising it!

My initial instinct is that it's unusual to derive both FromPrimitive (which supports default) and TryFromPrimitive (which doesn't) - in general, either an enum always has a reasonable default (where FromPrimitive makes sense), or the default is context-specific (and FromPrimitive doesn't have the context of how/where it's being used, so TryFromPrimitive makes sense, and the caller should probably do something like try_into().unwrap_or_default(..)).

Which suggests that we should probably error if num_enum(default) is present and TryFromPrimitive is being derived.

That said, this would be a breaking change, as currently it is possible to derive both. It probably makes sense to make that change though.

Out of interest did this cause you a real problem when using the library, or did you just notice the inconsistency and raise it? Both are valid reasons to fix this, but if you ran into a problem as a result of this issue I'd lean much more heavily towards fixing this soon.

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

2 participants