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

Should #[non_exhaustive] be advised over private members for extensibility? #132

Closed
haibane-tenshi opened this issue Jan 2, 2021 · 8 comments · Fixed by #135
Closed
Labels
A-pattern Area: Content about Patterns C-enhancement Category: Enhancements to content C-needs discussion Area: Something that is not clear to everyone if it fixes something/adds valuable content

Comments

@haibane-tenshi
Copy link

#[non_exhaustive] attribute on Rust reference.

I believe #[non_exhaustive] emerged as a better solution to the same problem and private members are an artifact from the rime before it. Still it seems there are a few differences.

From what I can understand, the attribute has three primary differences compared to private members:

  • It is still exhaustive inside original crate. There can be cases where this is undesirable (but I struggle to imagine when) - in which case private members still can be useful.
  • It can be applied to unit structs/variants. Impossible to achieve with private members (have to convert it to tuple/struct and tuple/struct variant respectively).
  • It can be applied to enums. A different approach is required to achieve similar effect without the attribute - a variant with never type (empty enum or !). It is not mentioned in the book.

Overall I consider the new approach as much superior (less boilerplate, better consistency), the old one should be deprecated and used only when very specific semantics is required (e. g. module-level non-exhaustiveness vs crate-level).

@simonsan simonsan added C-enhancement Category: Enhancements to content C-needs discussion Area: Something that is not clear to everyone if it fixes something/adds valuable content labels Jan 2, 2021
@AnthonyYoManz
Copy link

#[non_exhaustive] was made with the very purpose of replacing the old-school, unintuitive private member pattern so I think it makes sense to update our advice.

@simonsan
Copy link
Collaborator

simonsan commented Jan 3, 2021

the old one should be deprecated and used only when very specific semantics is required (e. g. module-level non-exhaustiveness vs crate-level)

That said, how do you imagine it. Do you want to keep both variants in the repo and state their use cases explicitly (link between them, improve doc to state both)? #135 replaces it now, which I'm not sure of if there is still at least one scenario where it will be used?

@MarcoIeni
Copy link
Collaborator

MarcoIeni commented Jan 3, 2021

An obvious scenario where it will be used is for compatibility with MSRV (Minimum supported Rust version).

Do you want to keep both variants in the repo and state their use cases explicitly (link between them, improve doc to state both)?

I would keep both variants in the same file, if it is necessary to keep the old approach.

Maybe the file could be called "Fields extensibility" and then we discuss both the approaches, first the new one and then we also write the old one (again, if we decide it is necessary), by explicitly saying it is less idiomatic than using non_exhaustive.

@haibane-tenshi
Copy link
Author

haibane-tenshi commented Jan 4, 2021

I'm a third person here, but I would probably create a "Legacy" category with patterns which are outdated/replaced with better alternatives, and then create cross-links on both approaches (maybe with MSRV mentioned etc.)

  • This will clearly show which approach is preferable.
  • There is probably some code in the wild which still uses the old approach. This will keep the pattern discoverable if needed.
  • There are probably other patterns which can go into the new category (like never type in enums I mentioned in the post).

@MarcoIeni
Copy link
Collaborator

Mmm..I don't dislike the idea of a "legacy" category :)

@simonsan
Copy link
Collaborator

simonsan commented Jan 6, 2021

I think something like:

Introduction
Contributing
Idioms
Patterns
Anti-patterns
Functional Programming
Refactoring
Legacy
|--- Idioms
|--- Patterns
|--- Anti-Patterns
Additional Resources

Could be then a nice build up rather than mentioning legacy in each category.

@MarcoIeni
Copy link
Collaborator

I agree! Of course for now there will be only the idioms directory there because this will be the only entry.
So in #135 we could move the old file in legacy/idioms/ and idioms/priv-extend.md will be updated with the new approach.
If you agree with this plan we could remove the needed-discussion label.

@simonsan simonsan added the A-pattern Area: Content about Patterns label Jan 21, 2021
@simonsan
Copy link
Collaborator

simonsan commented Sep 1, 2021

A different approach is required to achieve similar effect without the attribute - a variant with never type (empty enum or !). It is not mentioned in the book.

This would be the question if it's worth to mention then and should be added.

For now I think the legacy category didn't make sense. Also it's the question if we would need such a category overall and won't only keep the preferable approach and maybe an alternative inside the same file to have a discussion about both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pattern Area: Content about Patterns C-enhancement Category: Enhancements to content C-needs discussion Area: Something that is not clear to everyone if it fixes something/adds valuable content
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants