-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Explosive birth of 'case!' #4870
Explosive birth of 'case!' #4870
Conversation
Goddamn!! Why this p-r number is not 4869!?!? 4869 is the special number for me, so I desire to get this. Ah... |
e206566
to
e249e75
Compare
spec/compiler/parser/parser_spec.cr
Outdated
@@ -927,6 +927,10 @@ describe "Parser" do | |||
assert_syntax_error "case {1, 2}; when {3}; 4; end", "wrong number of tuple elements (given 1, expected 2)", 1, 19 | |||
assert_syntax_error "case 1; end", "unexpected token: end (expecting when or else)", 1, 9 | |||
|
|||
it_parses "case! a\nwhen Int32\n1\nend", Case.new("a".call, [When.new(["Int32".path] of ASTNode, 1.int32)], check_exhaustiveness: true) | |||
assert_syntax_error "case!\nwhen Int32\n1\nend", "unexpected token: when ('case!' must have a condition)", 2, 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the error for this should be: unexpected token: when ('case!' must have an expression)
(not necessarily a condition).
spec/compiler/parser/to_s_spec.cr
Outdated
@@ -108,4 +108,9 @@ describe "ASTNode#to_s" do | |||
expect_to_s %(1 <= 2 <= 3) | |||
expect_to_s %((1 <= 2) <= 3) | |||
expect_to_s %(1 <= (2 <= 3)) | |||
|
|||
expect_to_s %(case\nwhen 1\nwhen 2\nend) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering, what is the use of a case
not followed by an expression, shouldn't this be disallowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's case
, not case!
. There is no case
case in to_s_spec.cr
, so I added them also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A case without an expression is an if elseif elseif
but with conditions at the same indentation level:
if x
elsif y
elsif z
end
case
when x
when y
when z
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ysbaddaden
Please, let's merge #4837 instead. |
property! parent_visitor : MainVisitor | ||
|
||
# `nil` means 'not checked'. `true` means 'this case is exhaustive'. `false` means 'it cannot check exhaustiveness'. | ||
property? exhaustive : Bool? = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use an Enum here?
e249e75
to
7cc59b6
Compare
@RX14 Yes, I think so too, but I can't (I don't have the logic to) convince others. |
7cc59b6
to
b0c33ba
Compare
This can probably be closed now that #8424 is merged? |
case!
looks likecase
, but it checks exhaustiveness like Scala and Rust.case!
spec:case!
checks exhaustiveness for enums, union types,Bool
(true
orfalse
) andnil
.Not such a type (e.g.
Int32
,String
) cannot check exhaustiveness, butcase!
raises a compilation error in this case. It means:But, this case can be compiled:
Of course tuple conditions can be checked:
case!
must have a condition, it's a syntax error:case!
must not haveelse
clause, and can usewhen _
instead:crystal tool format
appends 2 spaces afterwhen
ofcase!
to alignwhen
conditions. For example:It is my preference, but I believe this is more beautiful than that: