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

Private enum variants #32770

Closed
SimonSapin opened this Issue Apr 6, 2016 · 9 comments

Comments

Projects
None yet
8 participants
@SimonSapin
Contributor

SimonSapin commented Apr 6, 2016

https://stackoverflow.com/questions/36440021/whats-purpose-of-errorkind-nonexhaustive explains why the std::io::ErrorKind enum has this variant:

    #[unstable(feature = "io_error_internals",
               reason = "better expressed through extensible enums that this \
                         enum cannot be exhaustively matched against",
               issue = "0")]
    #[doc(hidden)]
    __Nonexhaustive,

It is to force users to have a catch-all _ arm when matching on an ErrorKind value, so that more variants can be added in the future.

Clearly this is a future-proofing mechanism that is useful. There is no reason it wouldn’t be as useful in external libraries on crates.io than in the standard library. But to achieve it, ErrorKind abuses the stability mechanism with #[unstable], which can only be used in the standard library.

I’m sure this was discussed before, but a quick search did not show why we don’t allow variants of the same enum to have different privacy/visibility with pub or priv keywords, like we do for struct fields. Given the above, should this be reconsidered?

CC @aturon & lang team

@petrochenkov

This comment has been minimized.

Show comment
Hide comment
@petrochenkov

petrochenkov Apr 6, 2016

Contributor

AFAIR, private variants/variant fields/trait items were considered rarely necessary and nobody wanted to make them private by default, and at the same time nobody wanted to revive priv, so it all was postponed for post-1.0.
Now, with pub(restricted) accepted, reviving priv is not necessary, so adding visibility to all these entities seems like a reasonable step.

Contributor

petrochenkov commented Apr 6, 2016

AFAIR, private variants/variant fields/trait items were considered rarely necessary and nobody wanted to make them private by default, and at the same time nobody wanted to revive priv, so it all was postponed for post-1.0.
Now, with pub(restricted) accepted, reviving priv is not necessary, so adding visibility to all these entities seems like a reasonable step.

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon Apr 6, 2016

Member

@SimonSapin This is indeed a std-only hack, and we've always planned to replace it with something more principled.

There are a couple ways we might go about this:

  • Via privacy, as you're suggesting. The lang team has been resistant to re-instating priv (to keep complexity down), but the good news is that we have accepted an RFC that gives you this expressivity and much more. (In terms of this RFC, you could write something like pub(self) to get the equivalent to priv). It's still a bit unfortunate to have to have a dedicated future-proofing variant, though.
  • Via a dedicated mechanism. That proposal didn't make it in for 1.0, but could be reconsidered.

Given that we've already accepted the new privacy mechanism, I suspect that'll be the way to go here. (Of course, it still needs to be implemented!)

Member

aturon commented Apr 6, 2016

@SimonSapin This is indeed a std-only hack, and we've always planned to replace it with something more principled.

There are a couple ways we might go about this:

  • Via privacy, as you're suggesting. The lang team has been resistant to re-instating priv (to keep complexity down), but the good news is that we have accepted an RFC that gives you this expressivity and much more. (In terms of this RFC, you could write something like pub(self) to get the equivalent to priv). It's still a bit unfortunate to have to have a dedicated future-proofing variant, though.
  • Via a dedicated mechanism. That proposal didn't make it in for 1.0, but could be reconsidered.

Given that we've already accepted the new privacy mechanism, I suspect that'll be the way to go here. (Of course, it still needs to be implemented!)

@durka

This comment has been minimized.

Show comment
Hide comment
@durka

durka Apr 6, 2016

Contributor

@aturon RFC 1422 didn't address enums. Is pub(...) going to be allowed in front of enum variants/fields?

Contributor

durka commented Apr 6, 2016

@aturon RFC 1422 didn't address enums. Is pub(...) going to be allowed in front of enum variants/fields?

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon Apr 6, 2016

Member

@durka Ah! You're right, the RFC's detailed design only covers an expansion of the grammar for places that currently permit pub.

I had always assumed that this RFC would also introduce new locations for pub, including trait items and enum variants -- and I believe that I at least discussed this with @pnkfelix offline. But we should definitely clarify.

Paging @rust-lang/lang!

Member

aturon commented Apr 6, 2016

@durka Ah! You're right, the RFC's detailed design only covers an expansion of the grammar for places that currently permit pub.

I had always assumed that this RFC would also introduce new locations for pub, including trait items and enum variants -- and I believe that I at least discussed this with @pnkfelix offline. But we should definitely clarify.

Paging @rust-lang/lang!

@durka durka referenced this issue Apr 6, 2016

Open

RFC 1422 #10

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Apr 7, 2016

Contributor

@aturon I am in favor of permitting pub in more places, but I think the idea was to bring that up as its own distinct RFC. Or at least, that's the idea now :)

Contributor

nikomatsakis commented Apr 7, 2016

@aturon I am in favor of permitting pub in more places, but I think the idea was to bring that up as its own distinct RFC. Or at least, that's the idea now :)

@jimmycuadra

This comment has been minimized.

Show comment
Hide comment
@jimmycuadra

jimmycuadra Apr 7, 2016

Contributor

Another possibility for a dedicated mechanism is an attribute on the enum to control whether or not exhaustive matches are allowed, e.g. #[no_exhaustive].

Contributor

jimmycuadra commented Apr 7, 2016

Another possibility for a dedicated mechanism is an attribute on the enum to control whether or not exhaustive matches are allowed, e.g. #[no_exhaustive].

@SimonSapin

This comment has been minimized.

Show comment
Hide comment
@SimonSapin

SimonSapin Apr 7, 2016

Contributor

pub(self) sounds good.

I’m not a fan of enum { Foo, Bar, .. }. This use case (though real) seems too narrow to warrant dedicated syntax.

Contributor

SimonSapin commented Apr 7, 2016

pub(self) sounds good.

I’m not a fan of enum { Foo, Bar, .. }. This use case (though real) seems too narrow to warrant dedicated syntax.

@LeoTestard

This comment has been minimized.

Show comment
Hide comment
@LeoTestard

LeoTestard Apr 8, 2016

Contributor

For compareason with other languages, OCaml has a lint that a lot of people use that warns (or errors, depending on the flags passed to the compiler) about _ patterns for this specific reason. (The difference here is that it warns about all such patterns, for any enum, since it assumes that all types could be extended. It's a bit strong but at least you are guaranteed not to forgot to add a case for new variants when you add them, even inside the same module. It's not exactly the same as what we want here (in fact it's much rather the opposite: disallow _ patterns instead of making them mandatory) but it shares a similar goal: forcing programmers to take new variants into account.

There is also another (newer) feature: open variants.

type my_variant = ..

my_variant += Foo(int)
my_variant += Bar(string)

New variants can be added anywhere, even in other modules. The exn (exception) type is now implemented like that (it was special-cased before). Pattern-matching on those types can of course never be exhaustive and all pattern-matches have to include a _ pattern. This is closer to what we want here.

OCaml also has variants with private constructors but they have a different meaning: they allow you to match against these variants but not to use them to actually construct new values outside of the module they were defined in. The main use case of this is to allow a module to maintain invariants about values of this type. For example you could imagine a sorted_list type that has the same structure than the standard list type (with two constructors Nil and Cons) and that you can match against, but the difference being that to add an element into the list you cannot just use Cons but must use instead a function that will insert the element at the right place to maintain the ordering. This is a very useful feature, we should consider whether we eventually want to add something like that when changing how the visibility of enum variants works.

Contributor

LeoTestard commented Apr 8, 2016

For compareason with other languages, OCaml has a lint that a lot of people use that warns (or errors, depending on the flags passed to the compiler) about _ patterns for this specific reason. (The difference here is that it warns about all such patterns, for any enum, since it assumes that all types could be extended. It's a bit strong but at least you are guaranteed not to forgot to add a case for new variants when you add them, even inside the same module. It's not exactly the same as what we want here (in fact it's much rather the opposite: disallow _ patterns instead of making them mandatory) but it shares a similar goal: forcing programmers to take new variants into account.

There is also another (newer) feature: open variants.

type my_variant = ..

my_variant += Foo(int)
my_variant += Bar(string)

New variants can be added anywhere, even in other modules. The exn (exception) type is now implemented like that (it was special-cased before). Pattern-matching on those types can of course never be exhaustive and all pattern-matches have to include a _ pattern. This is closer to what we want here.

OCaml also has variants with private constructors but they have a different meaning: they allow you to match against these variants but not to use them to actually construct new values outside of the module they were defined in. The main use case of this is to allow a module to maintain invariants about values of this type. For example you could imagine a sorted_list type that has the same structure than the standard list type (with two constructors Nil and Cons) and that you can match against, but the difference being that to add an element into the list you cannot just use Cons but must use instead a function that will insert the element at the right place to maintain the ordering. This is a very useful feature, we should consider whether we eventually want to add something like that when changing how the visibility of enum variants works.

@Mark-Simulacrum

This comment has been minimized.

Show comment
Hide comment
@Mark-Simulacrum

Mark-Simulacrum May 24, 2017

Member

I'm going to close this in favor of rust-lang/rfcs#943.

Member

Mark-Simulacrum commented May 24, 2017

I'm going to close this in favor of rust-lang/rfcs#943.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment