This is the discussion thread for the first round of Community Review of DIP 1034, "Add a Bottom Type (reboot)":

https://github.com/dlang/DIPs/blob/15081980cd393e21218da6836321ed37ebc48dd3/DIPs/DIP1034.md

The review period will end at 11:59 PM ET on May 20, or when I make a post declaring it complete. Discussion in this thread may continue beyond that point.

Here in the discussion thread, you are free to discuss anything and everything related to the DIP. Express your support or opposition, debate alternatives, argue the merits, etc.

However, if you have any specific feedback on how to improve the proposal itself, then please post it in the feedback thread. The feedback thread will be the source for the review summary I write at the end of this review round. I will post a link to that thread immediately following this post. Just be sure to read and understand the Reviewer Guidelines before posting there:

https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md

And my blog post on the difference between the Discussion and Feedback threads:

https://dlang.org/blog/2020/01/26/dip-reviews-discussion-vs-feedback/

Please stay on topic here. I will delete posts that are completely off-topic.

On Wednesday, 6 May 2020 at 11:04:06 UTC, Mike Parker wrote:

>
> However, if you have any specific feedback on how to improve the proposal itself, then please post it in the feedback thread. The feedback thread will be the source for the review summary I write at the end of this review round. I will post a link to that thread immediately following this post.

The feedback thread is located here:

https://forum.dlang.org/post/arcpszmdarekxtnsnwfl@forum.dlang.org

On Wednesday, 6 May 2020 at 11:04:06 UTC, Mike Parker wrote:
> This is the discussion thread for the first round of Community Review of DIP 1034, "Add a Bottom Type (reboot)":
>
> [...]

I think this is one the best written DIPs I've seen in while independent from the changes it discusses.

On Wednesday, 6 May 2020 at 11:04:06 UTC, Mike Parker wrote:
> This is the discussion thread for the first round of Community Review of DIP 1034, "Add a Bottom Type (reboot)":
>
> [...]

Love it!
There was no point in DIP-1017 but this one makes an excellent case for the bottom type and explains it well enough that even dummies like me understand it.

On Wednesday, 6 May 2020 at 11:04:06 UTC, Mike Parker wrote:
> This is the discussion thread for the first round of Community Review of DIP 1034, "Add a Bottom Type (reboot)":
>
> https://github.com/dlang/DIPs/blob/15081980cd393e21218da6836321ed37ebc48dd3/DIPs/DIP1034.md

In one of the examples, it is asserted that there is no way to use `assert(0)` in a switch-like higher-order function that uses lambdas for its cases (such as sumtype's `match`).

> In a switch statement, it is possible to add a case default: assert(0);. This is not possible when using lambda-handlers to simulate a switch, like the sumtype package does:

This is not strictly true. It can be done by specifying the return type of the `assert(0)` lambda explicitly, rather than relying on type inference:

int round(Var v) {
    return v.match!(
        (int x) => return x,
        (double x) => return cast(int) x,
        function int (other) { assert(0);  } // compiles
    );
}

noreturn x0; // compile error, must have bottom value

noreturn[1] x4; // compile error, init value is [assert(0)]

struct S {int x; noreturn y;} // definition is fine
S x5; // compile error, must have bottom value

enum E : noreturn {x = assert(0), y = assert(0)}
E e; // compile error, must have bottom value

Why are these defined to cause a compile error?

On Wednesday, 6 May 2020 at 16:22:48 UTC, Meta wrote:
> Why are these defined to cause a compile error?

My reasoning is that the following are all equivalent:

> noreturn x;
> noreturn x = noreturn.init;
> noreturn x = assert(0);
> auto x = assert(0);
> auto x = () {assert(0);} ();

The bottom one gives a compile error today.

> Error: `assert(0)` failed

I don't intend to change that. (This comparison with errors during CTFE is also mentioned in "Interaction with other language features" by the way).

On Wednesday, 6 May 2020 at 14:21:43 UTC, Panke wrote:
> [snip]
>
> I think this is one the best written DIPs I've seen in while independent from the changes it discusses.

I had the same thought as well.

On Wednesday, 6 May 2020 at 16:39:30 UTC, Dennis wrote:
> On Wednesday, 6 May 2020 at 16:22:48 UTC, Meta wrote:
>> Why are these defined to cause a compile error?
>
> My reasoning is that the following are all equivalent:
>
>> noreturn x;
>> noreturn x = noreturn.init;
>> noreturn x = assert(0);
>> auto x = assert(0);
>> auto x = () {assert(0);} ();
>
> The bottom one gives a compile error today.
>
>> Error: `assert(0)` failed
>
> I don't intend to change that. (This comparison with errors during CTFE is also mentioned in "Interaction with other language features" by the way).

The problem is that these require special cases in generic code. If these common cases cause compile errors, then every template will either have to have a `if (!is(T == noreturn))`, or allow itself to fail (possibly deep inside a stack of instantiated templates).

std.algorithm.group, for example, returns a Group struct, defined as follows:

struct Group(alias pred, R)
if (isInputRange!R)
{
    import std.typecons : Rebindable, tuple, Tuple;

    private alias E = ElementType!R;
    static if ((is(E == class) || is(E == interface)) &&
               (is(E == const) || is(E == immutable)))
    {
        private alias MutableE = Rebindable!E;
    }
    else static if (is(E : Unqual!E))
    {
        private alias MutableE = Unqual!E;
    }
    else
    {
        private alias MutableE = E;
    }

    private R _input;
    private Tuple!(MutableE, uint) _current;
    ...
}

But if R is noreturn[], then this becomes:

struct Group(alias pred, R)
if (isInputRange!R)
{
    private noreturn[] _input;
    private Tuple!(noreturn, uint) _current;
}

And because Tuple is itself a struct, and Tuple!(noreturn, uint) has a field of type noreturn, the following code will fail to compile:

[].group()

With a confusing error message inside Tuple.

Maybe this is not a realistic scenario, but I think it should at least be considered that this will cause code that previously seemed fairly innocuous to now fail to compile, for a reason that may not be obvious.

On Wednesday, 6 May 2020 at 17:03:56 UTC, Meta wrote:
> Maybe this is not a realistic scenario, but I think it should at least be considered that this will cause code that previously seemed fairly innocuous to now fail to compile, for a reason that may not be obvious.

It doesn't compile today, as it would be instantiated with a void[]:

```
import std;
void main() {writeln([].group());}
```

> instance std.algorithm.iteration.Group!("a == b", void[]) does not match template declaration ...

You do raise a very good point though, I don't want to introduce a lot of `static if (is(T == noreturn[]))` to make range algorithms work with []. I'm considering changing it so that local noreturn variables give an error on usage instead of initialization.

I need think about the implications of that though. I'll try to make a prototype implementation before final review and see if I can make [] work with range functions in Phobos.