www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Feedback Thread: DIP 1034--Add a Bottom Type (reboot)--Final Review

reply Mike Parker <aldacron gmail.com> writes:
This is the feedback thread for the Final 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/heylgwkzcpfqmqytiezq forum.dlang.org

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

You can find DIP 1034 here:

https://github.com/dlang/DIPs/blob/b5bf9c4bed7afdae3bea1485093dbf2431bf72c7/DIPs/DIP1034.md

The review period will end at 11:59 PM ET on October 6, or when I 
make a post declaring it complete. Feedback posted to this thread 
after that point may be ignored.

At the end of the Final Review, the DIP will be forwarded to the 
language maintainers for Formal Assessment.

==================================
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 
the reviewer guidelines will receive much gratitude). Information 
which irrelevant to the DIP or is not provided in service of 
clarifying the feedback is unwelcome.
Sep 22 2020
next sibling parent reply Mike Parker <aldacron gmail.com> writes:
On Tuesday, 22 September 2020 at 12:17:09 UTC, Mike Parker wrote:

 https://github.com/dlang/DIPs/blob/b5bf9c4bed7afdae3bea1485093dbf2431bf72c7/DIPs/DIP1034.md
****PLEASE NOTE**** I apologize. The above is the wrong link. The correct link is here: https://github.com/dlang/DIPs/blob/d492715d3c7ba2cee898930d261ab113c0a66ef9/DIPs/DIP1034.md
Sep 22 2020
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Tuesday, 22 September 2020 at 12:25:17 UTC, Mike Parker wrote:
 On Tuesday, 22 September 2020 at 12:17:09 UTC, Mike Parker 
 wrote:

 https://github.com/dlang/DIPs/blob/b5bf9c4bed7afdae3bea1485093dbf2431bf72c7/DIPs/DIP1034.md
****PLEASE NOTE**** I apologize. The above is the wrong link. The correct link is here: https://github.com/dlang/DIPs/blob/d492715d3c7ba2cee898930d261ab113c0a66ef9/DIPs/DIP1034.md
The formatting of the unordered list under "Community Review Round 1" is broken.
Sep 22 2020
parent Mike Parker <aldacron gmail.com> writes:
On Tuesday, 22 September 2020 at 12:40:54 UTC, Paul Backus wrote:
https://github.com/dlang/DIPs/blob/d492715d3c7ba2cee898930d261ab113c0a66ef9/DIPs/DIP1034.md
 The formatting of the unordered list under "Community Review 
 Round 1" is broken.
Thanks. I'll fix that at the end of the review period.
Sep 22 2020
prev sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
Well written and very compelling!

I have a couple feedback points:

1. In "The cast operator":

```
int x;
int bar();
auto foo() {
     cast(noreturn) (x = bar()); // same as {x = bar(); assert(0);}
}
```

This is odd, and I'm not understanding the reason for the cast if you 
aren't assigning.

Also, earlier you say that you can declare a noreturn variable, but as 
long as you don't use it, you don't generate the assert 0. Is this 
example a contradiction of that?

2. Inside Description, change (3) "Implicit conversions from `noreturn` 
to any other type are allowed", I am having a difficult time with rules 
6 and 7:

```
/* 6 */ !(is(T == function)) || is(noreturn* : T)
/* 7 */ !(is(T == delegate)) || is(noreturn* : T)
```

What are these trying to say? I think these rules need some 
clarification or rewriting

-Steve
Sep 22 2020
parent Dennis <dkorpel gmail.com> writes:
On Tuesday, 22 September 2020 at 14:16:19 UTC, Steven 
Schveighoffer wrote:
 ```
 int x;
 int bar();
 auto foo() {
     cast(noreturn) (x = bar()); // same as {x = bar(); 
 assert(0);}
 }
 ```

 This is odd, and I'm not understanding the reason for the cast 
 if you aren't assigning.
This code has no reason, it's just an example of the rewrite that happens with `cast(noreturn)`. It could actually be useful in generic code, or when you are using a function that is noreturn but does not have the correct return type. ``` // someone else's code void panic(string msg); // your code: auto f = () => cast(noreturn) panic("error"); ```
 Also, earlier you say that you can declare a noreturn variable, 
 but as long as you don't use it, you don't generate the assert 
 0. Is this example a contradiction of that?
I don't think so, the example does not declare a local variable. I could rewrite it to this: ``` import std; int x; int bar(); auto foo() { noreturn y; // no assert here yet writeln("this will get printed"); y = cast(noreturn) (x = bar()); // here it results in assert(0) } ``` The reason for this is argued by Johannes Loher here: https://forum.dlang.org/post/r8v8tt$14fk$1 digitalmars.com
 ```
 /* 6 */ !(is(T == function)) || is(noreturn* : T)
 /* 7 */ !(is(T == delegate)) || is(noreturn* : T)
 ```

 What are these trying to say? I think these rules need some 
 clarification or rewriting
In formal logic, "(not p) or q" is equivalent to "if p then q", so it means "if T is a function/delegate, noreturn* implicitly converts to it". The DIP follows the rules up with a short explanation:
 5, 6 and 7 mean that null (which is of type noreturn*) may 
 still be assigned to a function pointer or array.
The point is that these things should still be allowed when `typeof(null)` becomes equivalent to `noreturn*`: ``` int[] a = null; void function() f = null; void delegate() d = null; Object o = null; ``` (I just realized I have no rule for `T == class`. I sometimes forget classes exist in D :p) I'll try to make it clearer.
Sep 22 2020
prev sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Tuesday, 22 September 2020 at 12:17:09 UTC, Mike Parker wrote:
 This is the feedback thread for the Final Review of DIP 1034, 
 "Add a Bottom Type (reboot)".
From the DIP:
  * A union has the size of the largest member. Adding a 
 noreturn field to a union never increases the size, so its size 
 is 0.
Currently, all structs and unions must have .init values that are computable at compile time, which means that adding a noreturn field would have to be a compile-time error, since it makes the .init value impossible to compute. You should probably specify in the section about `noreturn.init` than any aggregate with a noreturn field also has no .init value.
Sep 22 2020
parent Dennis <dkorpel gmail.com> writes:
On Tuesday, 22 September 2020 at 17:24:02 UTC, Paul Backus wrote:
 Currently, all structs and unions must have .init values that 
 are computable at compile time, which means that adding a 
 noreturn field would have to be a compile-time error, since it 
 makes the .init value impossible to compute.
You can still compute an init value, it'd be the same bytes as when the noreturn fields weren't there. I don't think foo0 and foo1 should act differently here: ``` struct S { int x; noreturn y; double z; } void foo0() { S s; } void foo1() { int x; noreturn y; double z; } ```
Sep 23 2020