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

===================================
**THIS IS NOT A DISCUSSION THREAD**

Posts in this thread must adhere to the feedback thread rules outlined in the Reviewer Guidelines (and listed at the bottom of this post).

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

That document also provides guidelines on contributing feedback to a DIP review. Please read it before posting here. If you would like to discuss this DIP, please do so in the discussion thread:

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

==================================

You can find DIP 1034 here:

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. Feedback posted to this thread after that point may be ignored.

At the end of this review round, the DIP will be moved into the Post-Community Round 1 state. Significant revisions resulting from this review round may cause the DIP manager to require another round of Community Review, otherwise the DIP will be queued for the Final Review.

==================================
Posts in this thread that do not adhere to the following rules will be deleted at the DIP author's discretion:

* All posts must be a direct reply to the DIP manager's initial post, with only two exceptions:

    - Any commenter may reply to their own posts to retract feedback contained in the original post

    - The DIP author may (and is encouraged to) reply to any feedback solely to acknowledge the feedback with agreement or disagreement (preferably with supporting reasons in the latter case)

* Feedback must be actionable, i.e., there must be some action the DIP author can choose to take in response to the feedback, such as changing details, adding new information, or even retracting the proposal.

* Feedback related to the merits of the proposal rather than to the contents of the DIP (e.g., "I'm against this DIP.") is allowed in Community Review (not Final Review), but must be backed by supporting arguments (e.g., "I'm against this DIP because..."). The supporting arguments must be reasonable. Obviously frivolous arguments waste everyone's time.

* Feedback should be clear and concise, preferably listed as bullet points (those who take the time to do an in-depth review and provide feedback in the form of answers to the questions in this document will receive much gratitude). Information irrelevant to the DIP or is not provided in service of clarifying the feedback is unwelcome.

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

Typo in sentence "Note that rules 1 to 4 don not naturally follow from rule 0 "

"do not" instead of "don not".

On Wed, May 06, 2020 at 11:05:30AM +0000, Mike Parker via Digitalmars-d wrote: [...]
> https://github.com/dlang/DIPs/blob/15081980cd393e21218da6836321ed37ebc48dd3/DIPs/DIP1034.md
[...]

Under the section "Standard name", last point under "Counter arguments", second last word: "immdediately" should be spelt "immediately".


T

-- 
Three out of two people have difficulties with fractions. -- Dirk Eddelbuettel

On Wed, May 06, 2020 at 11:05:30AM +0000, Mike Parker via Digitalmars-d wrote: [...]
> https://github.com/dlang/DIPs/blob/15081980cd393e21218da6836321ed37ebc48dd3/DIPs/DIP1034.md
[...]

Under "Description", "(3) Implicit conversions from noreturn to any other type are allowed.", 2nd bullet point: "respsectively" should be "respectively".


T

-- 
Lottery: tax on the stupid. -- Slashdotter

Copying this over from the discussion thread:

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

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.

I think these rules should at least be reconsidered or relaxed, if not removed.

Wherever there is "=> return" it's wrong. Drop the "return" keyword.

On Wednesday, 6 May 2020 at 11:05:30 UTC, Mike Parker wrote:
> [snip]

If the compiler will really say "element type could not be inferred from empty array" from trying to append to array of `noreturn`, it's a bad error message. It would appear to complain about the initialization, when only the appending in the following line is illegal. A better message would be something like "Attempt to append int to noreturn[], which must always be empty".

Perhaps the `noreturn` name, or whatever it will be, can be better defined at DRuntime level, like `size_t`? It'll still be standard, but can be done without without adding a keyword. It could be what Walter suggested (`alias noreturn = typeof(assert(false));`), but doing `alias noreturn = typeof(&null)` instead might be a bit easier to implement.

I generally liked the rejected proposal of Walter, and I like this one even more. Good luck with the remaining work!

* Mangling: prefer to stick with alpha_numeric characters for mangling. It doesn't need to be short, as I expect it to be rare

* Mention the conversion level, should be "convert".

* For comparisons, I'd use a more popular language than zig.

* Don't put a space between DIP and 1034 - this is so tag-based search works on it. Like #DIP1034

Great DIP!

In the breaking changes, it says "Any code assuming `is(typeof([]) == void[])` will break."

However, this is not exactly correct. If code *assumes* it is typed as void[], it will probably work.

For example:

void foo(void[])
{
}

foo([]); // I'm assuming it's void[], but it will still work.

The point being made is that the result of the expression `typeof([])` is changing. And any code that depends on that specific relationship for declarations will break.

I would change it to "Any code using the expression `typeof([])` for declarations or for template parameters might break. Any code that infers the type of a variable using the expression `[]` will also likely break."

I agree with the rest, because why would you write typeof([]) instead of void[]?

One possible exception:

auto x = []; // instead of typing void[] x, saving 2 characters!
x ~= ...; // now an error.

Another thing that would break -- if for some reason you have a type named "noreturn", and you import that from another module, there will be an ambiguity between object.noreturn and somemodule.noreturn.

I bet the chances of this are almost nil, but it is a possible breakage.

-Steve

I personally like the Scala's concept to name the bottom type "Nothing". I would therefore suggest to similarily name it "nothing" in D as well.

To me it sounds more natural and self-explanatory. noreturn look more like a keyword also due to its similarity to return keyword.

noreturn suggests clearly that the function will not return, but it is natural only for this use of the bottom type, and as the DIP states, there are more uses.

Although returning nothing is a little bit less clear that noreturn, everyone knows void, so it has to be something different. And nothing[] looks much more in place then noreturn[].

So - pun intended - nothing is a thing! ;)