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

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?

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.

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

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.

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.

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-possible-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!!!

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 Rust's `#[must_use]` attribute, 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.

Thanks for your comments!

[1] https://run.dlang.io/is/tK14gJ

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

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