Closed makenowjust closed 3 years ago
I would prefer to unreachable!
be a compiler error rather than a runtime exception. But unreachable
or unreachable?
could be the one that raise an exception during runtime.
Currently, it is not possible for all the cases to properly use compile time analysis. The first case is a case
over an enum value to express that it should be an exhaustive match. But even if this is not possible right know I would reserve the bang version for a compile time unreachable detection that could be improved in the future.
@bcardiff I'm not sure what you mean by compile-time unreachable detection, surely if the compiler can prove that it's unreachable then you don't need to tell the compiler that it's unreachable. Unreachable methods like this have a use only because the compiler is not correct in all cases.
When you have an enum with values E1, E2
. And you have a case
over all those values, you need to add an else
with some NoReturn
in order to avoid a nil
in the type of the resulting expression.
If a E3
is added, the compiler helps you nothing to identify that a when
clause is indeed missing in that case
. You need to stress that path in a spec or somehow to detect it.
But if the unreachable!
acts during compile time, it would abort the compilation because that is missing.
I think that is safer and doable in the future. Other use cases might involve checking type hierarchy and modules.
I agree that a runtime check is useful and make sense. I just don't agree on naming it unreachable!
because of these points.
Having to remember to use unreachable
on case over enums sounds like a pain point, surely there must be a better way to do it.
I know the better way is called "exhaustiveness check against case ... when
expression", in other words the compiler checks all when
-clauses conditions cover every case
values. However I think it is very difficult (To determine the specification, we have to discuss about this very very long time, and to implement it, we take more time...), and this is a problem in real world, so we should provide the aid to it as quickly.
Another point, I believe unreachable!
is useful utility even if "exhaustiveness check" is implemented. Because Rust has "exhaustiveness check", however it has unreachable!
also. You can see another unreachable!
usage in Rust document:
This is useful any time that the compiler can't determine that some code is unreachable. For example:
- Match arms with guard conditions.
- Loops that dynamically terminate.
- Iterators that dynamically terminate.
I think these are fit in Crystal too.
I'd be for adding unreachable
, but I think we should add proper case/enum checking as well.
So should it be unreachable!
or unreachable
?
I'd follow @bcardiff's argument that unreachable
should be used for runtime checks and unreachable!
might later be implemented as a compiler error. It doesn't need to be happening in that does certainly not have to be decided now, but it would surely be helpful to hold that path open.
@straight-shoota Even if we introduce compile-time unreachable!
check in future, we shouldn't introduce unreachable
for now. When implement compile-time check, then we introduce unreachable
and replace unreachable!
which cannot check on compile-time with unreachable
. Because, without detailed specification, we cannot decide which to use unreachable
or unreachable!
.
I think the purpose of unreachable!
is to control type inference to success compilation by expressing unreachable code explicitly.
The difference between unreachable code and a compile time assertion in my eyes, is that unreachable code is when checks in the current method make it logically impossible for a branch to be taken. If you rely on other code not to call the method with certain arguments, then that's a runtime assertion and not unreachable code.
Just letting you guys know that reachability analysis is definitely possible (its done in Scala where the compiler will always warn you if you don't do exhaustiveness checking). It wouldn't be the worst idea to have a look at the compiler source for this detection, because there is an algorithm behind it (the compiler also goes out if its way to optimize case checks with if-else and jump statements)
In any case, the unreachable!
should be a runtime check (which actually should never occur at runtime) which is also used to control the typesystem. Doing it as a compile time check doesn't make sense, the proper compile time check (in this scenario) is proper exhaustiveness checking. In Scala, this sought of thing is done all of the time (i.e. asInstanceOf
) to signify when you definitely know what the type of something is. The equivalent of unreachable!
in Scala land is actually throwing an exception (which means the resulting expression won't get a union or Any
type).
One thing to note is that in Scala, there isn't really a specific unreachable
exception because it has proper exhaustiveness checking, however it can still crop up when doing FFI, which is going to be much more common in Crystal due to its great FFI with C (in Scala/Java land, FFI is avoided when possible)
But hold on, doesn't raising an exception already serve as NoReturn
? How is writing unreachable!
better than writing raise
with a proper message for explanation?
Also having unreachable
but not having assert
is completely bizarre.
I would argue for naming the method without the !
, the same way Array#delete don't have a !
. The expectation of what it does is built in to the name already.
It's very similar to .not_nil!
. I guess it's about the exception raising.
As @oprypin says: what benefit would unreachable
have, versus the actual raise "unreachable"
? Except introduce a oneliner method/macro that will jusr raise at runtime... thus be exactly the same?
I belive we don't need this in Crystal.
I think that having a unified way to express a common intention is something good.
How to assert that that line is unreachable, in order to guide the type system, is something that could be agreed upon project/shard basis. But I don't think it would harm to have a standard way.
Regarding the name, I'm ok with using unreachable!
since it could mimic the not_nil!
that raises at runtime. And leave for the future the non bang to express unreachable code that could be detected during compile time. It could look weird that a keyword has a bang at the end dunno.
I think that having a unified way to express a common intention is something good.
You mean like assert
that almost every other language has?
Yes @oprypin , I never complain (nor encourage) about an assert
proposal . But I find myself more times wanting to express unreachable that to express invariants in the code with assert. Those conditions I usually type them for clarification.
All I'm saying is that unreachable
is also an invariant, just a special case of assert
.
I wont write unreachable
as assert false
since asserts could go away in release mode changing the semantic in this case. But yes, they are invariants.
http://smallcultfollowing.com/babysteps/blog/2018/08/13/never-patterns-exhaustive-matching-and-uninhabited-types-oh-my/ goes through how rust seems to be in the process of adding !
for something somewhat similar in intention.
I'd certainly prefer unreachable
over an exclamation mark though, for googleability if nothing else.
Looks like the general consensus is that we want unreachable
raising an exception at runtime. The compile time part might be better covered by the exhaustive case efforts anyway.
As the exhaustive case
clause is now implemented (case … in …
), I think the debate between runtime and compile time unreachable code is over. Should we go with unreachable!
to follow the syntax of not_nill!
?
I agree, this is not a new feature, this is just syntactic sugar. But I still think it would be useful, like getter
and property
are. As @bcardiff says, it is a unify way to express a common intention, and it would make reading code easier.
While looking at the Rust macro, I noticed they also have two similar features: unimplemented
and todo
. Their semantics are pretty similar, the only distinction is that todo
expresses intent of future implementation.
In Crystal we have NotImplementedError
for this. Maybe we could consider a shortand method for this? It actually isn't so much of an improvement. But when theres unreachable!
, it feels weird to have raise NotImplementedError.new
for a similar feature.
I'm not that fond of todo
though. IMO that's just better expressed as unimplemented
+ TODO comment.
Sometimes we use
raise "BUG: ..."
to express unreachable code. For example:This
raise "BUG: unreachable"
is important because it is required to inferfoo_bar_flip
's result type asFooBar
correctly, and if not, it is inferred toFooBar?
due to the case of matching nowhen
-clause (it is unreachable case also).There are many unreachable code:
git grep 'raise "BUG: ' | wc -l
in crystal repository shows 81 in fact. So, I think stdlib should provide the method to express such an unreachable code more explicitly thanraise "BUG: "
. This method makes easier to read code. I think such a method is calledunreachable!
.unreachable!
usage:And
unreachable!
spec:unreachable!
raises an error, so this type isNoReturn
.unreachable!
has default message to an error, but we can specify more helpful message.My questions:
I think
unreachable!
is the best name for such a method. However we have some ideas:unreach
(verb?),unreachable
(without bang) and etc... Which do you think the best?Should stdlib have
unreachable!
specific error class? In other words, shouldunreachable!
raiseUnreachableError
-like object? OrException
object simply?What is the default message? I think it is
BUG: unreachable
. Or, shouldunreachable
have the default message?Related issues:
1846: @asterite suggested such a method named
assert_empty_type
and @RX14 suggestedimpossible!
.And I was inspired by Rust's
unreachable!
macro. Thank you.