www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Discussion Thread: DIP 1038-- nodiscard--Final Review

reply Mike Parker <aldacron gmail.com> writes:
This is the discussion thread for the Final Review of DIP 1038, 
" nodiscard":

https://github.com/dlang/DIPs/blob/ab056150975a9a8db5b5da3dbffdd81529802a49/DIPs/DIP1038.md

The review period will end at 11:59 PM ET on February 17, 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.
Feb 03
next sibling parent Mike Parker <aldacron gmail.com> writes:
On Wednesday, 3 February 2021 at 11:25:29 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 here: https://forum.dlang.org/post/ipfmxoimroobpzymruzy forum.dlang.org
Feb 03
prev sibling parent reply Q. Schroll <qs.il.paperinik gmail.com> writes:
On Wednesday, 3 February 2021 at 11:25:29 UTC, Mike Parker wrote:
 This is the discussion thread for the Final Review of DIP 1038, 
 " nodiscard":
I wonder how many non-void functions exist whose return values are usually ignored. The DIP correctly identifies the results of assignments as commonly discarded values. Apart from those, printf and readf come to my mind. All in all, I do think those functions are rare. Having an attribute for the common case seems backwards to me. More often than not, it will be omitted. It is probably better to do it the other way around: Add a discard (or maydiscard or discardable) attribute to apply to printf, readf, and friends to indicate that discarding their result is an intended form of their use.
Feb 04
next sibling parent Max Haughton <maxhaton gmail.com> writes:
On Thursday, 4 February 2021 at 20:15:07 UTC, Q. Schroll wrote:
 On Wednesday, 3 February 2021 at 11:25:29 UTC, Mike Parker 
 wrote:
 This is the discussion thread for the Final Review of DIP 
 1038, " nodiscard":
I wonder how many non-void functions exist whose return values are usually ignored. The DIP correctly identifies the results of assignments as commonly discarded values. Apart from those, printf and readf come to my mind. All in all, I do think those functions are rare. Having an attribute for the common case seems backwards to me. More often than not, it will be omitted. It is probably better to do it the other way around: Add a discard (or maydiscard or discardable) attribute to apply to printf, readf, and friends to indicate that discarding their result is an intended form of their use.
Doing dataflow analysis on every single function call would probably be slow. This DIP solves a specific problem - it's effectively (an approximation) of a linear type system - there are certain non-trivial types that must be consumed for the program to be correct in some non-trivial way: Extending this logic to *all* functions, e.g. ones returning integers may not be as fruitful as you say, as we often have these return values to maintain the flexibility of code rather than to enable a specific pattern, by requiring everything to be annotated with some negative form of this attribute is not ideal in many circumstances. If you want to show that it is the common case, you have to show that it is the common case.
Feb 04
prev sibling next sibling parent Paul Backus <snarwin gmail.com> writes:
On Thursday, 4 February 2021 at 20:15:07 UTC, Q. Schroll wrote:
 Having an attribute for the common case seems backwards to me. 
 More often than not, it will be omitted. It is probably better 
 to do it the other way around: Add a  discard (or  maydiscard 
 or  discardable) attribute to apply to printf, readf, and 
 friends to indicate that discarding their result is an intended 
 form of their use.
The fact that this approach would break a huge amount of existing code is already enough to disqualify it from further consideration.
Feb 04
prev sibling next sibling parent reply IGotD- <nise nise.com> writes:
On Thursday, 4 February 2021 at 20:15:07 UTC, Q. Schroll wrote:
 I wonder how many non-void functions exist whose return values 
 are usually ignored.
There are plenty of them and I do it all the time. Sometimes it is unnecessary and sometimes not and I pay the price. I consider nodiscard a very niche problem in D, as described in the DIP mostly for system code. I think that in system code you do so much unsafe things anyway and if you want to be sure to check return values, then check the documentation of the library that you are using. It's a nanny when you need it the least. I think the DIP is heavily influenced by Rust but there the return value is part of the default language error handling which makes more sense. In D there are exceptions but this DIP doesn't regard them so the benefit is limited. I think that having a compiler flag that warns when a return value is not used should be enough. That should cover those projects where always checking the return value is a requirement.
Feb 04
next sibling parent Bruce Carneal <bcarneal gmail.com> writes:
On Friday, 5 February 2021 at 01:04:01 UTC, IGotD- wrote:
 On Thursday, 4 February 2021 at 20:15:07 UTC, Q. Schroll wrote:
 I wonder how many non-void functions exist whose return values 
 are usually ignored.
There are plenty of them and I do it all the time. Sometimes it is unnecessary and sometimes not and I pay the price. I consider nodiscard a very niche problem in D, as described in the DIP mostly for system code. I think that in system code you do so much unsafe things anyway and if you want to be sure to check return values, then check the documentation of the library that you are using. It's a nanny when you need it the least.
I'll take every bit of concise help that moves checking off the error-prone-human plate and on to the much-less-error-prone compiler plate. Such help is especially valuable at an underlying system level when it comes at compile time.
 I think the DIP is heavily influenced by Rust but there the 
 return value is part of the default language error handling 
 which makes more sense. In D there are exceptions but this DIP 
 doesn't regard them so the benefit is limited.
Limited in what way? How might the DIP "regard" exceptions?
 I think that having a compiler flag that warns when a return 
 value is not used should be enough. That should cover those 
 projects where always checking the return value is a 
 requirement.
I much prefer errors to warnings. If I accept warnings then I must carry more of the burden of correctness manually, unaided, as long as the code base is active. YMMV of course. The happily cavalier crowd should be fine because the capability is opt-in. They'll only be bitten if a less-cavalier author updates support code underneath them IIUC. I like the DIP, a lot. It is useful, performant (compile time), and orthogonal to other compiler concerns AFAICS. I also like the writing. Nicely done.
Feb 04
prev sibling parent Dennis <dkorpel gmail.com> writes:
On Friday, 5 February 2021 at 01:04:01 UTC, IGotD- wrote:
 I think that having a compiler flag that warns when a return 
 value is not used should be enough. That should cover those 
 projects where always checking the return value is a 
 requirement.
In my current project that uses `return this;` to allow chaining methods, that would give hundreds of false positives.
Feb 05
prev sibling parent reply claptrap <clap trap.com> writes:
On Thursday, 4 February 2021 at 20:15:07 UTC, Q. Schroll wrote:
 On Wednesday, 3 February 2021 at 11:25:29 UTC, Mike Parker

 Having an attribute for the common case seems backwards to me. 
 More often than not, it will be omitted. It is probably better 
 to do it the other way around: Add a  discard (or  maydiscard 
 or  discardable) attribute to apply to printf, readf, and 
 friends to indicate that discarding their result is an intended 
 form of their use.
Are you going to annotate every function that has a return value and no side effects? All the math functions, min, max, sqrt? I mean is there a difference between "the point of the function is its return value", and "if you dont handle the return value bad things will happen", and nodisgard is for the latter? If so, then maybe nodisgard is not the common case.
Feb 05
parent reply Paul Backus <snarwin gmail.com> writes:
On Friday, 5 February 2021 at 08:33:21 UTC, claptrap wrote:
 Are you going to annotate every function that has a return 
 value and no side effects? All the math functions, min, max, 
 sqrt? I mean is there a difference between "the point of the 
 function is its return value", and "if you dont handle the 
 return value bad things will happen", and nodisgard is for the 
 latter?
Remember that the compiler already produces a warning (not an error) if you discard the return value of a `pure nothrow` function, which includes most std.math functions: void main() { import std.math; sqrt(2.0); } Compile with `dmd -wi` and you get: onlineapp.d(4): Warning: calling std.math.sqrt without side effects discards return value of type double, prepend a cast(void) if intentional https://run.dlang.io/is/cE80Uv
Feb 05
parent reply claptrap <clap trap.com> writes:
On Friday, 5 February 2021 at 11:35:34 UTC, Paul Backus wrote:
 On Friday, 5 February 2021 at 08:33:21 UTC, claptrap wrote:
 Remember that the compiler already produces a warning (not an 
 error) if you discard the return value of a `pure nothrow` 
 function, which includes most std.math functions:

     void main()
     {
         import std.math;
         sqrt(2.0);
     }

 Compile with `dmd -wi` and you get:

 onlineapp.d(4): Warning: calling std.math.sqrt without side 
 effects discards return value of type double, prepend a 
 cast(void) if intentional

 https://run.dlang.io/is/cE80Uv
Tbh i didnt even know D had warnings, nor that it warned about unused returns. Cant see the point with stuff like sqrt() though.
Feb 05
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Friday, 5 February 2021 at 14:09:09 UTC, claptrap wrote:
 Tbh i didnt even know D had warnings, nor that it warned about 
 unused returns.
You would have learned if you'd read the DIP all the way through. ;) https://github.com/dlang/DIPs/blob/ab056150975a9a8db5b5da3dbffdd81529802a49/DIPs/DIP1038.md#in-d
 Cant see the point with stuff like sqrt() though.
It's a generic warning that applies to any discarded expression without side effects. For example, it will also trigger if you write 1 + 1;
Feb 05
next sibling parent claptrap <clap trap.com> writes:
On Friday, 5 February 2021 at 14:16:22 UTC, Paul Backus wrote:
 On Friday, 5 February 2021 at 14:09:09 UTC, claptrap wrote:
 Tbh i didnt even know D had warnings, nor that it warned about 
 unused returns.
You would have learned if you'd read the DIP all the way through. ;)
Touche' :)
Feb 05
prev sibling parent sarn <sarn theartofmachinery.com> writes:
On Friday, 5 February 2021 at 14:16:22 UTC, Paul Backus wrote:
 On Friday, 5 February 2021 at 14:09:09 UTC, claptrap wrote:
 Cant see the point with stuff like sqrt() though.
It's a generic warning that applies to any discarded expression without side effects. For example, it will also trigger if you write 1 + 1;
That one's an outright error (without -wi) because if you replace the 1s with identifiers, and + with *, you get a parsing ambiguity (in C, too): https://theartofmachinery.com/2020/08/18/d_declarations_for_c_programmers.html#syntax-ambiguities
Feb 05
prev sibling parent Max Haughton <maxhaton gmail.com> writes:
On Friday, 5 February 2021 at 14:09:09 UTC, claptrap wrote:
 On Friday, 5 February 2021 at 11:35:34 UTC, Paul Backus wrote:
 On Friday, 5 February 2021 at 08:33:21 UTC, claptrap wrote:
 Remember that the compiler already produces a warning (not an 
 error) if you discard the return value of a `pure nothrow` 
 function, which includes most std.math functions:

     void main()
     {
         import std.math;
         sqrt(2.0);
     }

 Compile with `dmd -wi` and you get:

 onlineapp.d(4): Warning: calling std.math.sqrt without side 
 effects discards return value of type double, prepend a 
 cast(void) if intentional

 https://run.dlang.io/is/cE80Uv
Tbh i didnt even know D had warnings, nor that it warned about unused returns. Cant see the point with stuff like sqrt() though.
It may alter the state of the floating point unit which may effect program behaviour even though you would be thinking "But why, I never used it?"
Feb 05