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

Use #[non_exhaustive] where applicable #959

Merged
merged 4 commits into from
Sep 13, 2020
Merged

Use #[non_exhaustive] where applicable #959

merged 4 commits into from
Sep 13, 2020

Conversation

LikeLakers2
Copy link
Contributor

@LikeLakers2 LikeLakers2 commented Sep 13, 2020

Hi there! This changes all of those odd __Nonexhaustive enum cases, so that they instead use the #[non_exhaustive] attribute.

However, I opted not to add the #[non_exhaustive] attribute to any structs where it could apply (that is, those who had a __nonexhaustive: () field), since I'm unsure if the effects would be acceptable:

Non-exhaustive structs could have additional fields added in future. Therefore, non-exhaustive structs cannot be constructed in external crates using the traditional Struct { .. } syntax; cannot be matched against without a wildcard ..; and struct update syntax will not work.

That said, if it's desired that I add it to any applicable structs, I can do so.

Side note: I did notice #752 while searching to see if there was an existing PR that made this change -- but being that the PR is from December 2019, I imagine there would be lots of merge conflicts and other misc issues that would make it hard to merge that. So I decided to make my own PR.

@arqunis arqunis added enhancement An improvement to Serenity. model Related to the `model` module. builder Related to the `builder` module. client Related to the `client` module. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate utils Related to the `utils` module. voice Related to the `voice` module and `serenity_voice_model` crate. labels Sep 13, 2020
@arqunis
Copy link
Member

arqunis commented Sep 13, 2020

since I'm unsure if the effects would be acceptable

The private field enforces the effects already. The attribute just makes it more apparent to the user and the compiler that a struct/enum is non-exhaustive, thus tooling can support the fact (e.g., rustdoc showing it in documentation). You should switch the structs to use the attribute as well.

@LikeLakers2 LikeLakers2 changed the title Use #[non_exhaustive] where applicable on enums Use #[non_exhaustive] where applicable Sep 13, 2020
@LikeLakers2
Copy link
Contributor Author

@acdenisSK I've added #[non_exhaustive] to any applicable structs, as you requested.

@arqunis arqunis merged commit 9ee42f1 into serenity-rs:current Sep 13, 2020
arqunis pushed a commit to arqunis/serenity that referenced this pull request Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builder Related to the `builder` module. client Related to the `client` module. enhancement An improvement to Serenity. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate model Related to the `model` module. utils Related to the `utils` module. voice Related to the `voice` module and `serenity_voice_model` crate.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants