177

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.

11
  • 28
    Well there's a whole lot of functions where the return value can be sensibly discarded. operator = for example. And std::map::insert.
    – user253751
    Dec 30, 2017 at 20:39
  • 5
    @immibis: True. The standard library is full of such functions. But in my own code, they tend to be extremely rare; do you think it's a matter of library code vs. application code? Dec 30, 2017 at 20:47
  • 21
    "...terribly verbose that it starts to feel like Java..." - reading this in a question about C++ made me twitch a little :-/
    – Marco13
    Dec 31, 2017 at 18:49
  • 5
    @ChristianHackl The comments are likely not the right place for "language bashing", and of course, Java also is verbose compared to other languages. But header files, assignment operators, copy constructors and proper usage of const can bloat an otherwise "simple" class (or rather "plain old data object") considerably in C++.
    – Marco13
    Jan 1, 2018 at 1:39
  • 3
    @Marco13: I cannot address so many points in one comment. Let's make it brief: Java doesn't have header files, which reduces file count but increases coupling; OTOH it forces you to put every top-level class in its own file and every function in a class. In C++, you almost never implement assignment operators and copy constructors yourself; those functions are more relevant for standard-library classes like std::vector or std::unique_ptr, of which you just need data members in your class definition. I've worked with both languages; Java is an okayish language, but it is more verbose. Jan 1, 2018 at 11:45

5 Answers 5

134

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.

20
  • 10
    I've upvoted this, as it contains useful insights. Not yet sure if it really answers the question, though. Re impure functions like pop or operator<<, those would be explicitly marked by my imaginary [[maydiscard]] attribute, so no warnings would be generated. Are you saying such functions are generally much more common than I'd like to believe, or much more common in general than in my own codebases? Your first paragraph about existing code is certainly true, but still makes me wonder if anyone else is currently peppering all their new code with [[nodiscard]], and if not, why not. Dec 30, 2017 at 18:22
  • 33
    @ChristianHackl: There was a time in C++'s history where it was considered good practice to return values as const. You can't say returns_an_int() = 0, so why should returns_a_str() = "" work? C++11 showed this was actually a terrible idea, because now all this code was prohibiting moves because the values were const. I think the proper take-away from that lesson is "it's not up to you what your callers do with your result". Ignoring the result is a perfectly valid thing that callers might want to do, and unless you know it's wrong (a very high bar), you shouldn't block it.
    – GManNickG
    Dec 30, 2017 at 20:34
  • 17
    A nodiscard on empty() is also sensible because the name is quite ambiguous: is it a predicate is_empty() or is it a mutator clear()? You wouldn't typically expect such a mutator to return anything, so a warning about a return value can alert the programmer to a problem. With a clearer name, this attribute would not be as necessary.
    – amon
    Dec 30, 2017 at 22:02
  • 15
    @ChristianHackl: As others have said, I think pure functions a good example of when this should be used. I would go a step further and say there should be a [[pure]] attribute, and it should imply [[nodiscard]]. That way it's clear why this result should not be discarded. But other than pure functions, I can't think of another class of functions that should have this behavior. And most functions aren't pure, hence I don't think nodiscard as a default is the right choice.
    – GManNickG
    Dec 30, 2017 at 22:46
  • 9
    @GManNickG: Having attempted and failed to debug code that ignored error codes, I strongly believe the vast majority of error codes need to be checked by default. Dec 31, 2017 at 9:10
23

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

  1. (major:) This would introduce way too much 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

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.

7
  • 1
    point 2 is countered by (void) Aug 15, 2021 at 17:34
  • 1
    @NooneAtAll: It is not.
    – einpoklum
    Aug 15, 2021 at 17:49
  • 3
    @einpoklum Respectfully, it is. [[nodiscard]] provides a strong suggestion that the value should not be discarded. If the end-user really wants to discard they can suppress the warning with a void cast. The point in the top answer about functions with no effects beyond returning a result is really important: Discarding such a result is almost certainly a mistake and [[nodiscard]] will catch it. Lastly, you don't guarantee incompatibility with C++14: godbolt.org/z/WzaoasEjo. Since GCC 7, Clang 3.9, and MSVC v19.24 the attribute is silently ignored if compiling with < C++17.
    – qz-
    Feb 20, 2022 at 18:28
  • @qz That "strong suggest" is there whether the user is able to ignore the suggestion or not. (void) lets the user ignore the suggestion - but I would still be making the suggestion, and like I said - I don't believe I have a basis making it
    – einpoklum
    Mar 6 at 21:37
  • "You wanna discard the return value" makes it sound like discarding the return value is a conscious decision on the programmer's part, when the problem is that it is often an error/oversight, unless there is a (void) tag explicitly added to the call, in which case you can be pretty sure it was a conscious decision. Apr 14 at 19:49
11

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.

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.

1
  • 1
    -Wunused-result is turned on by default. And it is not "universal", it works only for functions marked with the attribute warn_unused_result. Therefore it is not better in any way than [[ nodiscard ]]. gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
    – MacDada
    Apr 15 at 22:13
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.

1
  • 2
    This answers a question which wasn't asked, i.e. "What's the reason for not using [[nodiscard]] everywhere?". Aug 19, 2019 at 13:19

Not the answer you're looking for? Browse other questions tagged or ask your own question.