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

Enum#each ignores unnamed members #13160

Open
straight-shoota opened this issue Mar 7, 2023 · 6 comments
Open

Enum#each ignores unnamed members #13160

straight-shoota opened this issue Mar 7, 2023 · 6 comments

Comments

@straight-shoota
Copy link
Member

Enum#each only iterates named enum members and ignores any additional flags created from an explicit integer value.
This is unexpected and confusing which can lead to bugs (cf. #13155).
This behaviour is in conflict with Enum#includes? which affirms the presence of unnamed flags.
Enum does not explicitly implement the Enumerable interface, but it should be expected that #each and #includes? agree on what's part of the collection.

Example:

@[Flags]
enum Foo
  Bar
  Baz
end

boo = Foo::Bar | Foo.new(4)
boo.includes?(boo) # => true
boo.each { |x| p x } # prints only Bar

Enum#each should iterate the named members and also any remainder value, split into individual flags. In the example above (Foo::Baz | Flags.new(4) | Flags.new(8))#each would iterate members with values 2, 4 and 8.

The implementation could be similar to #13155.

@HertzDevil
Copy link
Contributor

One problem is Enum#includes? itself is rather lax and returns true if at least one common bit is set:

@[Flags]
enum Foo
  X = 0b011
  Y = 0b110
end

Foo::X.includes?(Foo::Y) # => true
Foo::Y.includes?(Foo::X) # => true

Foo::X.includes?(Foo::Y | Foo.new(0b1000)) # => true

An #each consistent with the current #includes? implies that Foo::X.each would yield both X and Y. This does not sound right and we might have to fix both #each and #includes? at once.

@straight-shoota
Copy link
Member Author

Oh, yeah that's surely another bug in #includes?.

@asterite
Copy link
Member

asterite commented Mar 7, 2023

In my opinion when you use values that don't have a named member the behavior should be unspecified. So each is behaving correctly.

@straight-shoota
Copy link
Member Author

But why? That seems unnecessarily restrictive and reduces a lot of flexibility that helps overcome some of the hurdles with being unable to reopen enums.

Thinking this forward, I suppose instead of making this unspecified behaviour it might be better to make it an error to instantiate an enum with undefined value if you want to go this route.

In my understanding a flags enum is essentially a bit array. Some bit positions are named, others are not. It's always clear which flags are set and which are not. The only difference between a named member and an unnamed one is the name itself. But there are no inherently attached semantics.
So I don't see much reason to artificially restrict the usability of unnamed members.
Using them is of course not as safe as a named member because there's no associated definition of what it's supposed to stand for. So that's your own risk. But I think it's good to have this option for flexibility.

@asterite
Copy link
Member

asterite commented Mar 7, 2023

Right, I think it should be an error to create an enum value with a number that doesn't correspond to a member. We copied new from C# but it was probably not a good idea.

Enums are closed for a reason: so you can control the behavior. If you want an open enum you probably want something else.

@asterite
Copy link
Member

asterite commented Mar 7, 2023

Actually, nevermind. Ignore me.

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

No branches or pull requests

3 participants