RFC: unreachable! method to express unreachable code explicitly

[makenowjust]

Sometimes we use raise "BUG: ..." to express unreachable code. For example:

enum FooBar
  Foo
  Bar
end

def foo_bar_flip(foo_bar : FooBar)
  case foo_bar
  when .foo?
    Bar
  when .bar?
    Foo
  else
    raise "BUG: unreachable" # <- this!!
  end
end

This raise "BUG: unreachable" is important because it is required to infer foo_bar_flip's result type as FooBar correctly, and if not, it is inferred to FooBar? due to the case of matching no when-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 than raise "BUG: ". This method makes easier to read code. I think such a method is called unreachable!.

unreachable! usage:

def foo_bar_flip(foo_bar : FooBar)
  case foo_bar
  when .foo?
    Bar
  when .bar?
    Foo
  else
    unreachable!
  end
end

And unreachable! spec:

• unreachable! raises an error, so this type is NoReturn.
• unreachable! has default message to an error, but we can specify more helpful message.

---

My questions:

1. 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?

2. Should stdlib have unreachable! specific error class? In other words, should unreachable! raise UnreachableError-like object? Or Exception object simply?

3. What is the default message? I think it is BUG: unreachable. Or, should unreachable have the default message?

   • I think having the default message is important because this is unreachable, so we never see this message in normal cases. Do you take a time to consider such a message?

Related issues:

• case when covering all types of a type restricted variable shouldn't need an else case #1846: @asterite suggested such a method named assert_empty_type and @RX14 suggested impossible!.

And I was inspired by Rust's unreachable! macro. Thank you.

[bcardiff]

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.

[RX14]

@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.

[bcardiff]

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.

[RX14]

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.

[makenowjust]

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.

[RX14]

I'd be for adding unreachable, but I think we should add proper case/enum checking as well.

[straight-shoota]

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.

[makenowjust]

@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!.

[makenowjust]

I think the purpose of unreachable! is to control type inference to success compilation by expressing unreachable code explicitly.

[RX14]

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.

[mdedetrich]

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)

[oprypin]

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?

[oprypin]

Also having unreachable but not having assert is completely bizarre.

[yxhuvud]

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.

[oprypin]

It's very similar to .not_nil!. I guess it's about the exception raising.

[ysbaddaden]

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.

[bcardiff]

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.

[oprypin]

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?

[bcardiff]

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.

[oprypin]

All I'm saying is that unreachable is also an invariant, just a special case of assert.

[bcardiff]

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.

[yxhuvud]

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.

[jhass]

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.

[erdnaxeli]

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.

[straight-shoota]

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.

[straight-shoota]

Closing per #9988 (comment)