www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - DIP 1038: nodiscard - Unofficial "post-final" review

reply Paul Backus <snarwin gmail.com> writes:
In response to Walter's feedback [1] from the final review round, 
I've added a new section to DIP 1038 ( nodiscard) on "Design 
Goals and Possible Alternatives".

Since it's a substantial addition (almost 50% of the DIP's 
original length), Mike Parker, the DIP Manager, has graciously 
allowed me to postpone formal assessment to give the community a 
chance to look it over, and point out any silly mistakes I may 
have made.

If you'd like to help  nodiscard succeed, I hope you'll give the 
revised version a read and reply to this thread with any 
questions or feedback you may have, especially regarding the new 
section. You can also reach out to me on the dlang slack or the D 
community Discord if you'd prefer.

Revised DIP 1038:
https://github.com/pbackus/DIPs/blob/dip1038-final-revisions/DIPs/DIP1038.md

Direct link to new section:
https://github.com/pbackus/DIPs/blob/dip1038-final-revisions/DIPs/DIP1038.md#design-goals-and-possible-alternatives

[1] https://forum.dlang.org/post/rvgdj6$15na$1 digitalmars.com
Feb 22
next sibling parent reply Dukc <ajieskola gmail.com> writes:
On Monday, 22 February 2021 at 12:03:40 UTC, Paul Backus wrote:
 Since it's a substantial addition (almost 50% of the DIP's 
 original length), Mike Parker, the DIP Manager, has graciously 
 allowed me to postpone formal assessment to give the community 
 a chance to look it over, and point out any silly mistakes I 
 may have made.
Shouldn't this mandate another official final review round instead (mainly asking Mike here)? Or even another community review?
Feb 22
next sibling parent reply Mike Parker <aldacron gmail.com> writes:
On Monday, 22 February 2021 at 12:27:54 UTC, Dukc wrote:

 Shouldn't this mandate another official final review round 
 instead (mainly asking Mike here)? Or even another community 
 review?
No. If the changes affected the details of the proposed feature, then yes, I would reboot the process and go back to another official community review round. That's not the case here, so it's fine to do it informally so as not to drag the process out any longer than we need to.
Feb 22
parent reply Dukc <ajieskola gmail.com> writes:
On Monday, 22 February 2021 at 12:59:02 UTC, Mike Parker wrote:
 No. If the changes affected the details of the proposed 
 feature, then yes, I would reboot the process and go back to 
 another official community review round. That's not the case 
 here, so it's fine to do it informally so as not to drag the 
 process out any longer than we need to.
Ok. Did a quick read and started to ponder one thing. The DIP claims no breaking changes. If somebody is currently annotating something with ` nodiscard` and using it so that the function return value is discarded, it means that he must have a local definition of ` nodiscard` and that will override the `core.attribute` definition, and be no longer recognised by the compiler. Is that correct? Even in that case, there is a theoretical possibility that the user-defined ` nodiscard` is annotated in different module than where it is used, and the user also happens to import `core.attribute`. That would result in an ambiguous symbol. This is so far-fetched that it'll have no practical signifiance, but if we're strict about mentioning all breaking changes it goes there.
Feb 22
parent Paul Backus <snarwin gmail.com> writes:
On Monday, 22 February 2021 at 20:12:05 UTC, Dukc wrote:
 Ok. Did a quick read and started to ponder one thing. The DIP 
 claims no breaking changes. If somebody is currently annotating 
 something with ` nodiscard` and using it so that the function 
 return value is discarded, it means that he must have a local 
 definition of ` nodiscard` and that will override the 
 `core.attribute` definition, and be no longer recognised by the 
 compiler. Is that correct?
Yes, the compiler will only recognize `core.attribute.nodiscard`.
 Even in that case, there is a theoretical possibility that the 
 user-defined ` nodiscard` is annotated in different module than 
 where it is used, and the user also happens to import 
 `core.attribute`. That would result in an ambiguous symbol.

 This is so far-fetched that it'll have no practical 
 signifiance, but if we're strict about mentioning all breaking 
 changes it goes there.
This can happen any time a new public symbol is added to a library. As far as I'm aware, adding a new public symbol is not normally considered a breaking change, so I don't think it's necessary to mention it in the DIP. Still, thanks for bringing it up--it never hurts to be thorough.
Feb 22
prev sibling parent Paul Backus <snarwin gmail.com> writes:
On Monday, 22 February 2021 at 12:27:54 UTC, Dukc wrote:
 Shouldn't this mandate another official final review round 
 instead (mainly asking Mike here)? Or even another community 
 review?
I asked Mike the same question. Because the new section does not change "the core of the proposal," it does not require another official review round. You can read Mike's full reply on Github: https://github.com/dlang/DIPs/pull/204#issuecomment-783126503
Feb 22
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/22/21 7:03 AM, Paul Backus wrote:
 In response to Walter's feedback [1] from the final review round, I've 
 added a new section to DIP 1038 ( nodiscard) on "Design Goals and 
 Possible Alternatives".
 
 Since it's a substantial addition (almost 50% of the DIP's original 
 length), Mike Parker, the DIP Manager, has graciously allowed me to 
 postpone formal assessment to give the community a chance to look it 
 over, and point out any silly mistakes I may have made.
 
 If you'd like to help  nodiscard succeed, I hope you'll give the revised 
 version a read and reply to this thread with any questions or feedback 
 you may have, especially regarding the new section. You can also reach 
 out to me on the dlang slack or the D community Discord if you'd prefer.
 
 Revised DIP 1038:
 https://github.com/pbackus/DIPs/blob/dip1038-final-revisions/DIPs/DIP1038.md 
 
 
 Direct link to new section:
 https://github.com/pbackus/DIPs/blob/dip1038-final-revisions/DIPs/DIP1038.md#design-goals-and-p
ssible-alternatives 
 
 
 [1] https://forum.dlang.org/post/rvgdj6$15na$1 digitalmars.com
This is nice and well argued. The time-tested presence of the attribute in C++ is important, probably could use 1-2 paragraphs to emphasize that. (It's been a non-standard extension for a long time and clearly there was a lot of demand to standardize it.) For integration, it would be nice if the proposal made pure non-void functions automatically nodiscard. That way ignoring the reusult of such a function is an error, as it should. The proposal should mention what happens if two declararions differ only by nodiscard: void* malloc(size_t); nodiscard void* malloc(size_t); I forgot what we usually do. I think it's safe to mark that as as error. The proposal should mention how nodiscard works with overriding. Given that nodiscard is more restrictive, by the old OOP principles a nodiscard function should be overridable by a non- nodiscard function. (Rationale: overriding functions ASK LESS and RETURN MORE.) However, a practicality consideration is that the author of the override simply forgot to add nodiscard, so we can make the attribute equivariant. It's important in all cases that a no- nodiscard function cannot be overriden by a nodiscard one. Great work!!!
Feb 22
parent reply Paul Backus <snarwin gmail.com> writes:
On Monday, 22 February 2021 at 22:01:01 UTC, Andrei Alexandrescu 
wrote:
 This is nice and well argued. The time-tested presence of the 
 attribute in C++ is important, probably could use 1-2 
 paragraphs to emphasize that. (It's been a non-standard 
 extension for a long time and clearly there was a lot of demand 
 to standardize it.)
Are there any well-known [[nodiscard]] success stories that the DIP could cite? I have little personal experience with C++, and do not know where to look for examples outside of the standard library.
 For integration, it would be nice if the proposal made pure 
 non-void functions automatically nodiscard. That way ignoring 
 the reusult of such a function is an error, as it should.
The compiler currently issues a warning when the return value of a strongly-pure, nothrow, non-void function is discarded. I agree that there is a case to be made that this should be an error instead, but I think the appropriate place to make that case is in an enhancement request on Bugzilla, not DIP 1038.
 The proposal should mention what happens if two declararions 
 differ only by nodiscard:

 void* malloc(size_t);
  nodiscard void* malloc(size_t);

 I forgot what we usually do. I think it's safe to mark that as 
 as error.
nodiscard is a user-defined attribute, and the compiler currently allows multiple declarations of a function that differ only by UDAs. [1] Again, I agree that this should probably be an error, but since it is an issue that affects UDAs in general, it should be addressed in a Bugzilla issue, not DIP 1038.
 The proposal should mention how  nodiscard works with 
 overriding. Given that  nodiscard is more restrictive, by the 
 old OOP principles a  nodiscard function should be overridable 
 by a non- nodiscard function. (Rationale: overriding functions 
 ASK LESS and RETURN MORE.) However, a practicality 
 consideration is that the author of the override simply forgot 
 to add  nodiscard, so we can make the attribute equivariant. 
 It's important in all cases that a no- nodiscard function 
 cannot be overriden by a  nodiscard one.
nodiscard is a UDA, and UDAs are not inherited, so applying nodiscard to a base-class method will have no affect on derived-class methods that override it. The way to achieve the outcome you desire here is to apply nodiscard to the method's return type, rather than the method itself: nodiscard struct Result { int unwrap; } class A { Result func() { return Result(1); } } class B : A { override Result func() { return Result(2); } } The idea behind the proposed design is that if you want strong guarantees, you should be applying nodiscard to your types, not to your functions. If you are willing to accept weaker guarantees in return for greater ease-of-use, that's when you should use nodiscard as a function attribute. (This design is cribbed from Of course, the fact we're having this exchange means that the above explanation really ought to be in the DIP itself. Thanks for your comments! [1] https://run.dlang.io/is/tK14gJ
Feb 22
next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2021-02-23 00:32, Paul Backus wrote:

  nodiscard is a user-defined attribute, and the compiler currently 
 allows multiple declarations of a function that differ only by UDAs. [1] 
 Again, I agree that this should probably be an error, but since it is an 
 issue that affects UDAs in general, it should be addressed in a Bugzilla 
 issue, not DIP 1038.
I would like to point out that compiler recognized user defined attributes were introduced as an alternative to keywords and attributes prefixed with ` `. Keywords are reserved words and cannot be used for user defined symbols. Attributes prefixed with ` `, i.e. ` property`, were introduced as a form of an additonal namespace for keywords. The word after ` ` is not reserved so it can be used as user defined symbols. After that, user defined attributes were introduced. Even though you can have a user defined symbol named `property`, you cannot use that as a UDA, because it would cause a conflict with ` property`. This makes attributes prefixed with ` ` for or less the same as keywords. Then compiler recognized UDAs were introduced to solve all of this. ` selector` is a recognized UDA, but you can still name your symbols `selector` and you can still use that as a UDA. If there's a conflict with the regular UDA and the compiler recognized UDA, the user can use the normal language features to disambiguate the names, i.e. renamed imports, static imports, aliases an so on. That means that the compiler should be free to, more or less, add any semantic behavior it wants to a compiler recognized UDA. For example, both the ` selector` and the ` optional` compiler recognized UDAs have several restrictions and other semantic behavior which a regular UDA doesn't have. They can only appear once in a method declaration, they can only be attached to methods with Objective-C linkage, they cannot be attached to templates, the number of colons in the string passed to ` selector` need to match the number of parameters. ` optional` affects how a class implements an interface. If you say that you cannot add a specific semantic behavior to a compiler recognized UDA because it's a UDA it all falls apart. If you don't add any special semantic behavior at **all**, it would just be a regular UDA. -- /Jacob Carlborg
Feb 26
parent Paul Backus <snarwin gmail.com> writes:
On Friday, 26 February 2021 at 20:00:52 UTC, Jacob Carlborg wrote:
 On 2021-02-23 00:32, Paul Backus wrote:

  nodiscard is a user-defined attribute, and the compiler 
 currently allows multiple declarations of a function that 
 differ only by UDAs. [1] Again, I agree that this should 
 probably be an error, but since it is an issue that affects 
 UDAs in general, it should be addressed in a Bugzilla issue, 
 not DIP 1038.
[...]
 If you say that you cannot add a specific semantic behavior to 
 a compiler recognized UDA because it's a UDA it all falls 
 apart. If you don't add any special semantic behavior at 
 **all**, it would just be a regular UDA.
You've misread me. I am not saying that I *cannot* add this behavior specifically for nodiscard, I am saying that I *should not*, because the current behavior is a bug that affects other attributes as well. Maybe you meant to reply to the part of the post about inheritance? Certainly it would be possible to have overrides of virtual methods inherit nodiscard, or at least to forbid them from dropping it. The question is whether that's actually a good idea. Personally, I think the increased spec and implementation complexity would not be worth the benefit you'd get--especially since it would only apply to the weak syntax-level check, which is trivial to defeat even if you plug this particular hole. It's worth noting that [[nodiscard]] is not inherited in C++. [1] [1] https://stackoverflow.com/questions/49576298/are-function-attributes-inherited
Feb 26
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/22/21 6:32 PM, Paul Backus wrote:
 On Monday, 22 February 2021 at 22:01:01 UTC, Andrei Alexandrescu wrote:
 This is nice and well argued. The time-tested presence of the 
 attribute in C++ is important, probably could use 1-2 paragraphs to 
 emphasize that. (It's been a non-standard extension for a long time 
 and clearly there was a lot of demand to standardize it.)
Are there any well-known [[nodiscard]] success stories that the DIP could cite? I have little personal experience with C++, and do not know where to look for examples outside of the standard library.
Couldn't find an article specifically discussing a success story, but those are not easy to find quickly. One thing good to cite is that implementers do deprecate/ignore unsuccessful attributes (e.g. gcc had a "returns" attribute for a while indicating that a function returns a specific parameter, which was superseded by better RVO implementation in the compiler). Au contraire, noreturn was present in all major compilers and graduated to standardization. This only happens to useful attributes. There's even an enhancement to it that you may also want to consider: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1301r2.html
 void* malloc(size_t);
  nodiscard void* malloc(size_t);

 I forgot what we usually do. I think it's safe to mark that as as error.
nodiscard is a user-defined attribute, and the compiler currently allows multiple declarations of a function that differ only by UDAs. [1] Again, I agree that this should probably be an error, but since it is an issue that affects UDAs in general, it should be addressed in a Bugzilla issue, not DIP 1038.
Hm, bit of a bummer.
 The way to achieve the outcome you desire here is to apply  nodiscard to 
 the method's return type, rather than the method itself:
 
       nodiscard struct Result { int unwrap; }
 
      class A { Result func() { return Result(1); } }
      class B : A { override Result func() { return Result(2); } }
 
 The idea behind the proposed design is that if you want strong 
 guarantees, you should be applying  nodiscard to your types, not to your 
 functions. If you are willing to accept weaker guarantees in return for 
 greater ease-of-use, that's when you should use  nodiscard as a function 

 which works the same way.)
 
 Of course, the fact we're having this exchange means that the above 
 explanation really ought to be in the DIP itself.
Yah, would be great to include such considerations, and now you've already done most of the work.
Feb 27