7

Is the [[nodiscard]] attribute necessary on operators? Or is it safe to assume the compiler will emit a warning like it does for most suspiciously discarded things?

E.g. an overloaded operator+, should one apply the attribute? What about special operators like function-cast operators or new operators? When is it pedantic?

Alex Guteniev
  • 12,039
  • 2
  • 34
  • 79
j__
  • 632
  • 4
  • 18
  • Depends on whether it makes sense for calling code to call those operator functions and not use what they return. In the case of binary `operator+()`, for example, does a statement of the form `a + b` (which doesn't use whatever `operator+()` returns) make sense versus (say) `some_result = a + b`? If it doesn't make sense for your class, you may wish to add the `[[nodiscard]]` attribute to the `operator+()` – Peter Aug 01 '20 at 10:15
  • it is never necessary – 463035818_is_not_an_ai Aug 01 '20 at 10:20
  • @Peter In my case, I don't overload operators where it doesn't operate identically to how primitives operate. In other words, `operator+` should never be discarded. That said, I don't see virtually any code that uses the nodiscard attribute on it and I was wondering it's because it defaults to on. – j__ Aug 01 '20 at 10:23
  • 1
    @lajoh90686 - How often do you think users of the class will use the `operator+()` and discard the result? Are there any cases where there are *significant* (however that is specified for your program) drawbacks of doing so? If the answers are "not often" and "no" - which I'd suggest is pretty common in practice - then there is little justification those operators being marked `[[nodiscard]]` – Peter Aug 01 '20 at 10:56
  • I plan to release the code to display my modern C++ abilities and just figured I wanted to perfect everything without looking like I'm "overengineering". The answers are, indeed, not often and no. I'm still on the fence on whether or not to include it. – j__ Aug 01 '20 at 11:02
  • 2
    If discarding the result is not an error, don't use `[[nodiscard]]`. Good examples when `[[nodiscard]]` is useful are `operator new` and `std::async`. `operator+` doesn't look like a good candidate for `[[nodiscard]]`. Suggested reading: [N.Josuttis. `[[nodiscard]]` in the library](https://wg21.link/p0600). – Evg Aug 01 '20 at 11:46
  • So `nodiscard` means that discarding is exclusively dangerous? Or just that discarding doesn't make sense? I figure that no code should have `3 + 4;` and should emit the warning that `nodiscard` will, but it technically has no harm. However, it is certainly a mistake. Is there any authoritative source that explicitly states its reason someone should use it? Is the liberal usage not correct? – j__ Aug 01 '20 at 11:51
  • We really don't want to pollute almost every non-`void` function with `[[nodiscard]]`. Take a look at how it is used in the standard library. I guess it's a good guide. – Evg Aug 01 '20 at 12:08
  • fwiw the only place where the core guidline mentions it is for casting a nodiscard result to `void` to silence the warning (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es48-avoid-casts). Imho such casts should not be necessary for proper use of `[[nodiscard]]`. If you only use the attribute when discarding is a logic error then there it is never needed to silence the warning – 463035818_is_not_an_ai Aug 01 '20 at 12:17

3 Answers3

8

Let me cite the following paper by N.Josuttis: "[[nodiscard]] in the library" (with some omissions, see the full paper):

C++17 introduced the [[nodiscard]] attribute. The question is, where to apply it now in the standard library. It should be added where:

  • not using the return value always is a “huge mistake” (e.g. always resulting in resource leak),
  • not using the return value is a source of trouble and easily can happen (not obvious that something is wrong).

It should not be added when:

  • not using the return value is a possible/common way of programming at least for some input,
  • not using the return value makes no sense but doesn’t hurt and is usually not an error.

So, [[nodiscard]] should not signal bad code if this

  • can be useful not to use the return value,
  • is common not to use the return value,
  • doesn’t hurt and probably no state change was meant that doesn’t happen.
Evg
  • 25,259
  • 5
  • 41
  • 83
2

It is never necessary to add the [[nodiscard]] attribute. From cppreference:

If a function declared nodiscard or a function returning an enumeration or class declared nodiscard by value is called from a discarded-value expression other than a cast to void, the compiler is encouraged to issue a warning.

Note the last part: "... the compiler is encouraged to issue a warning." The is no guarantee, as far as the standard is concerned, that there actually will be a warning. Its a quality of implementation issue. If your compiler does emit a warning (read the docs) and if you are treating such warnings as errors, then the [[nodiscard]] can be of great use.

It is pedantic to use the attribute on operators where discarding the return is only potentially an error. I would only use it when calling the operator and discarding the result is always a logic error. Many operators use the return value merely to enable chaining and the [[nodiscard]] would rather be an annoyance on such operators. There are cases where the decision is not so obvious and it is a matter of opinion and style what you choose.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • So something like iostream's `operator<<` wouldn't have it, but something like a vector class `operator+` should? – j__ Aug 01 '20 at 10:40
  • 1
    @lajoh90686 depends on what a `operator+` on a vector class does. For `operator<<` it would be completely useless because you always discard the returned value (at least of the last call in chaining, eg `std::cout << "hello" << " world";`) – 463035818_is_not_an_ai Aug 01 '20 at 10:41
  • A vector class as in a cartesian vector. Like a coordinate. – j__ Aug 01 '20 at 10:45
  • @lajoh90686 it really depends on details. `operator+` is typically `const` and does return a fresh instance, ie calling it and discarding the result cannot be correct. However, operators can be overloaded to do weird stuff. An `operator+` could be non-`const` and modify `this`, eg it could count how often a vector is added to another, in such case calling the operator has an effect even if you do discard the result – 463035818_is_not_an_ai Aug 01 '20 at 10:48
2

Is nodiscard necessary on operators?

No. nodiscard and other attribures are optional.

Or is it safe to assume the compiler will emit a warning like it does for most suspiciously discarded things?

There is no guarantee about any warning in the language except when the program is ill formed.

I would also not assume warning without nodiscard because there are many cases where result of operation is intentionally discarded. A common example:

a = b;  // result of assignment was discarded

In fact, if all discarded results resulted in a warning, then there would not be any purpose for the nodiscard attribure.

eerorika
  • 232,697
  • 12
  • 197
  • 326