69

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.

  • 21
    Well there's a whole lot of functions where the return value can be sensibly discarded. operator = for example. And std::map::insert. – immibis Dec 30 '17 at 20:39
  • 2
    @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? – Christian Hackl Dec 30 '17 at 20:47
  • 9
    "...terribly verbose that it starts to feel like Java..." - reading this in a question about C++ made me twitch a little :-/ – Marco13 Dec 31 '17 at 18:49
  • 3
    @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 '18 at 1:39
  • 2
    @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. – Christian Hackl Jan 1 '18 at 11:45
70

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.

  • 6
    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. – Christian Hackl Dec 30 '17 at 18:22
  • 22
    @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 '17 at 20:34
  • 12
    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 '17 at 22:02
  • 13
    @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 '17 at 22:46
  • 4
    @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. – Mooing Duck Dec 31 '17 at 9:10
7

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.

  • On your "major" point, though, OP already does specify that they found it too verbose and is really asking why this isn't the default (perhaps with an inverse attribute for when discarding is expected?). – Kat Jan 2 '18 at 19:43
  • @Kat: To be frank, OP asks the question both ways - why not add it and why isn't it the default; I'll edit my answer to address the other part of the question, sort of. – einpoklum Jan 2 '18 at 19:46
0

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.

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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