Prevent None and All members in flags enums #4395
Conversation
|
Just saw these specs. As per #1251 I don't see a usage for redefining those so i'll remove the specs. |
RX14
force-pushed
the
feature/flags-enum-error
branch
from
7aaf518
to
b5b3292
Compare
|
Happy compiler hacking then! I think we need to allow redefining All and None for enums inside libs. We can't assume the None/All semantic will hold outside crystal. In the current state the user is able to override All and None members. It could lead to some inconsistencies where So.
|
RX14
force-pushed
the
feature/flags-enum-error
branch
from
add508c
to
96c0bc6
Compare
|
These two fixup commits should fix your feedback. |
| @@ -550,6 +550,10 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor | |||
| node.raise "can't reopen enum and add more constants to it" | |||
| end | |||
|
|
|||
| if is_flags && !@in_lib && {"None", "All"}.includes?(member.name) | |||
There was a problem hiding this comment.
Shouldn't that be {"none", "all"}.includes?(member.name.downcase)?
There was a problem hiding this comment.
enum Foo
Foo
FOo
FOO
end
Foo.values # => [Foo, FOo, FOO]
|
This PR seems to be fixed on macos now. |
|
@RX14, I think I rushed a bit to merge this, and I should have requested you to review the related documentation to make sure it at least is consistent with these changes. Would mind doing that and sending another PR along? |
|
@mverzilli Not a problem, is it just the language reference that needs updating? I can't think of anywhere else where this would be documented. |
|
Actually nevermind. It is a subtle detail and in general we try to keep the language reference simple and straight to the point. Thanks! |
|
@mverzilli I've looked through the language reference section and I agree with you, it's the wrong place for this detail. |
|
Hi, I just noticed this and realized that my project has been "broken" by this. It's no big deal, I can just remove my definition of None, but I am losing a documented enum member with important information. |
This is my first "real" compiler contribution, please tell me if there's anything I could improve.
See #1251.