183

C++17 introduces the [[nodiscard]] attribute, which allows programmers to mark functions in a way that the compiler produces a warning if the returned object is discarded by a caller; the same attribute can be added to an entire class type.

I've read about the motivation for this feature in the original proposal, and I know that C++20 will add the attribute to standard functions like std::vector::empty, whose names do not convey an unambiguous meaning regarding the return value.

It's a cool and a useful feature. In fact, it almost seems too useful. Everywhere I read about [[nodiscard]], people discuss it as if you'd just add it to a select few functions or types and forget about the rest. But why should a non-discardable value be a special case, especially when writing new code? Isn't a discarded return value typically a bug or at least a waste of resources?

And isn't one of the design principles of C++ itself that the compiler should catch as many errors as possible?

If so, then why not add [[nodiscard]] in your own, non-legacy code to almost every single non-void function and almost every single class type?

I've tried to do that in my own code, and it works fine, except it's so terribly verbose that it starts to feel like Java. It would seem much more natural to make compilers warn about discarded return values by default except for the few other cases where you mark your intention[*].

As I've seen zero discussions about this possibility in standard proposals, blog entries, Stack Overflow questions or anywhere else on the internet, I must be missing something.

Why would such mechanics not make sense in new C++ code? Is verbosity the only reason not to use [[nodiscard]] almost everywhere?


[*] In theory, you may have something like a [[maydiscard]] attribute instead, which could also be retroactively added to functions like printf in standard-library implementations.

Christian Hackl
  • 2,004
  • 2
  • 13
  • 12

5 Answers5

137

In new code that need not be compatible with older standards, do use that attribute wherever it's sensible. But for C++, [[nodiscard]] makes a bad default. You suggest:

It would seem much more natural to make compilers warn about discarded return values by default except for the few other cases where you mark your intention.

That would suddenly cause existing, correct code to emit lots of warnings. While such a change could technically be considered to be backwards-compatible since any existing code still compiles successfully, that would be a huge change of semantics in practice.

Design decisions for an existing, mature language with a very large code base are necessarily different from design decisions for a completely new language. If this were a new language, then warning by default would be sensible. For example, the Nim language requires unneeded values to be discarded explicitly – this would be similar to wrapping every expression statement in C++ with a cast (void)(...).

A [[nodiscard]] attribute is most useful in two cases:

  • if a function has no effects beyond returning a certain result, i.e. is pure. If the result is not used, the call is certainly useless. On the other hand, discarding the result would not be incorrect.

  • if the return value must be checked, e.g. for a C-like interface that returns error codes instead of throwing. This is the primary use case. For idiomatic C++, that's going to be quite rare.

These two cases leave a huge space of impure functions that do return a value, where such a warning would be misleading. For example:

  • Consider a queue data type with a .pop() method that removes an element and returns a copy of the removed element. Such a method is often convenient. However, there are some cases where we only want to remove the element, without getting a copy. That is perfectly legitimate, and a warning would not be helpful. A different design (such as std::vector) splits these responsibilities, but that has other tradeoffs.

    Note that in some cases, a copy has to be made anyway so thanks to RVO returning the copy would be free.

  • Consider fluent interfaces, where each operation returns the object so that further operations can be performed. In C++, the most common example is the stream insertion operator <<. It would be extremely cumbersome to add a [[nodiscard]] attribute to each and every << overload.

These examples demonstrate that making idiomatic C++ code compile without warnings under a “C++17 with nodiscard by default” language would be quite tedious.

Note that your shiny new C++17 code (where you can use these attributes) may still be compiled together with libraries that target older C++ standards. This backwards compatibility is crucial for the C/C++ ecosystem. So making nodiscard the default would result in many warnings for typical, idiomatic use cases – warnings which you cannot fix without far-reaching changes to the library's source code.

Arguably, the problem here isn't the change of semantics but that the features of each C++ standard apply on a per-compilation-unit scope and not on a per-file scope. If/when some future C++ standard moves away from header files, such a change would be more realistic.

Malice
  • 129
  • 7
amon
  • 135,795
23

Reasons I wouldn't [[nodiscard]] almost everywhere:

  1. (minor:) This would introduce a lot of noise in my headers.
  2. (major:) I don't feel like I should make strong speculative assumptions about other people's code. You wanna discard the return value I give you? Fine, knock yourself out.
  3. (minor:) You would be guaranteeing incompatibility with C++14, as [[nodiscard]] is a C++17 attribute.

Now, if you made it default, you would be forcing all library developers to force all their users to not-discard return values. That would be horrible. Or you force them to add [[maydiscard]] in innumerably many functions, which is also kind of horrible.

I've edited, reducing the significance of reason #1.

einpoklum
  • 2,752
13

My opinion on [[nodiscard]] is that it should be used in places where lack of taking the result likely leads to an incorrect behavior of a program, and not just some redundant computation.

That's why, I wouldn't put [[nodiscard]] on a getter or a result of a pure function. But I would put on those functions where the lifetime of the result matters. malloc-like function comes to mind, but most likely also most of the functions returning a new unique-pointer, or other RAII objects.

Case in point: I was working with a highly reactive code. On every observable you could attach an arbitrary lambda:

value.observe([](auto value) {...})

The semantic was - whenever the content of value was changed, that lambda was called. The catch was - observe was returning a Handle which controlled how long the lambda was observing the value. When the handle was destroyed, the lambda was detaching itself from the observed value.

It was very easy to forget about keeping the handle. But calling observe without keeping the result effectively had no effect - a major semantic departure from the intention, beyond just a minor inefficiency. In this scenaro [[nodiscard]] was a godsend, and I believe these are the scenarios where it should be used. Not everywhere.

CygnusX1
  • 257
6

Most C++ compilers have an option that can be used to enable a warning every time a returned value is ignored. For instance, in g++ you can use -Wunused-result to get these warnings.

The [[nodiscard]] attribute has been added to allow a middle ground between always producing a warning and never producing one, which is why most descriptions of how to use it take the approach of being selective with it.

occipita
  • 209
1

As an example: operator<< has a return value that is depending on the call either absolutely needed or absolutely useless. (std::cout << x << y, the first is needed because it returns the stream, the second is of no use at all). Now compare with printf where everyone discards the return value which is there for error checking, but then operator<< has no error checking so it can’t have been that useful in the first place. So in both cases nodiscard would just be damaging.

gnasher729
  • 49,096