2

In a project using C++20, CLion suggested me to add [[nodiscard]] to my const class method definitions, e.g.,

class Test {
public:
    [[nodiscard]] int f(int a, int b) const {
        return a + b;
    }
}

The explanation is

Adds [[nodiscard]] attributes (introduced in C++17) to member functions in order to highlight at compile time which return values should not be ignored.

and I also checked cppreference.com:

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. ... Appears in a function declaration, enumeration declaration, or class declaration.

If, from a discarded-value expression other than a cast to void,

  • a function declared nodiscard is called, or
  • a function returning an enumeration or class declared nodiscard by value is called, or
  • a constructor declared nodiscard is called by explicit type conversion or static_cast, or
  • an object of an enumeration or class type declared nodiscard is initialized by explicit type conversion or static_cast,

the compiler is encouraged to issue a warning.

I honestly don't really understand why this annotation is needed in this location. Why would the compiler ignore my return values if the caller processes them further? Is there an intuitive explanation of what it tells the compiler exactly and why it's needed?

Aamir
  • 1,974
  • 1
  • 14
  • 18
Green绿色
  • 1,620
  • 1
  • 16
  • 43
  • 1
    It is to warn about usage `test.f(0, 1);` (whereas `int r = test.f(0, 1);` would be ok). – Jarod42 Jun 16 '23 at 11:15
  • It's not compiler ignoring the result, it can be you. If you write `Test t; t.f(1, 2);`, you discard the value, but it doesn't make sense to do so, because the function doesn't do anything else except returning a value. Without `[[nodiscard]]`, this passes by silently. With `[[nodiscard]]`, compiler will issue a warning to turn your attention to this point. – Yksisarvinen Jun 16 '23 at 11:16
  • @Yksisarvinen In my case, my IDE highlights every function with this recommendation, no matter whether I do or do not use the return value. I double checked, the return value is assigned to variables and processed further by the caller. – Green绿色 Jun 16 '23 at 11:23
  • I don't use **CLion** (nothing against it; I love JetBrains stuff), but I suspect that you can disable it's `[[nodiscard]]` suggestion feature. – Eljay Jun 16 '23 at 11:26
  • @Eljay Yes, I can. But I better first understand what it is all about before I go ahead and ignore that. The developers at JetBrains probably had a reason to activate it in the first place. :) – Green绿色 Jun 16 '23 at 11:30
  • Some functions are called for their side-effects. Their return value is informative, but not important. Other functions are called for their return value, discarding the return value makes no sense because the function serves no purpose to call. And a third category of functions return an error code, and ignoring an error code is usually a source of bugs. – Eljay Jun 16 '23 at 11:35
  • @Green绿色 if you go into the clang-tidy settings of CLion you will find a lot more warnings that are disabled and some are enabled by default. A lot of the warnings and suggestions are activated only in some situations (like embedded development). – Kevin Spaghetti Jun 16 '23 at 11:36
  • @Green绿色 It's not about the uses. clang-tidy (through CLion) only checks the function definition, it doesn't check the calls. If a function is pure (i.e. it has no side effects), it recommends that you should mark it with `[[nodiscard]]`, because in any *future* code that you write you might make a mistake and forget to actually use its result for something. It's to prevent you from making new bugs when you write code. – Yksisarvinen Jun 16 '23 at 12:07

3 Answers3

5

The idea is that if it only makes sense to call your function when you also take its return value, calling it without taking the return value is a programming error. The annotation [[nodiscard]] helps programmers interfacing with your code avoid this error.

In your specific example, your function computes a result without side effects, so the static code analyzer realizes that it would be a good fit for [[nodiscard]]. For example:

Test a;
auto x = a.f(1, 2); // ok
std::cout << "For 1, 2 we get " << a.f(1,2) << std::endl; // also ok
a.f(1, 2); // warning here

Here the compiler is encouraged to warn in the last line about the function call without further processing of the result.

Discussion

An example of good use for [[discard]] can be object methods that manipulate an object, but provide the result in a copy. Example:

DateTime yesterday = DateTime.now().substractOneDay();
std::cout << "Yesterday was " << yesterday.nameOfDay() << std::endl;

vs:

DateTime yesterday = DateTime.now();
yesterday.substractOneDay();
std::cout << "Yesterday was " << yesterday.nameOfDay() << std::endl;

A [[nodiscard]] would tell the programmer of the second example, that they are using it wrong.

ypnos
  • 50,202
  • 14
  • 95
  • 141
  • Note that you can silence the warning by casting to `void`, if you ever wanted to discard the value intentionally: `void(a.f(1, 2))` – Jan Schultke Jun 16 '23 at 11:28
  • 3
    Some standard library functions have `[[nodiscard]]` too, like `std::vector::empty()` to prevent developers from confusing *"is something empty"* and *"to empty something"*. – Jan Schultke Jun 16 '23 at 11:35
  • 1
    Good answer! [`std::async`](https://en.cppreference.com/w/cpp/thread/async) seems to be another good example. Discarding it's result will instantly wait for the async execution to finish iirc. – Lukas-T Jun 16 '23 at 11:44
2

[[nodiscard]] is useful when you have class methods that return something and you want to highlight that the method does not work in place but instead returns something.
For example if you had a linked list a possible reverse method could be marked [[nodiscard]] to signal that the method returns a new list.

It helps avoid code like this:

List l{1, 2, 3, 4};
l.reverse();
std::cout << l;

>> 1, 2, 3, 4 //Why is this not working?

To me it seems that the most useful place to employ [[nodiscard]] is when you have a method/function that modifies the object/parameters but also returns something (so the method/function cannot be const).

List l{1, 2, 3, 4};
l.doubleEveryElementAndSum(); //the sum returned is silently ignored
std::cout << l;

>> 2, 4, 6, 8

The fact that CLion highlights the method and tells you that the method could be marked [[nodiscard] has nothing to do with CLion or the compiler but with the tool clang-tidy. This tool scans your code and checks for things that you can improve, CLion then visualizes the results of this tool.
When you compile your code the compiler will read the annotation and issue a warning whenever it finds code that ignores the return value of the function marked [[nodiscard]].

My suggestion is to not pollute your code by putting [[nodiscard]] everywhere as CLion (clang-tidy) suggests. If you find the warnings annoying you can disable them in the clang-tidy settings.

Kevin Spaghetti
  • 620
  • 4
  • 16
  • What confuses me is that if it wouldn't return anything, it's return type would be `void`? This annotation really seems redundant to me. – Green绿色 Jun 16 '23 at 11:21
  • If you try to call the method marked with `[[nodiscard]]` and you don't store the returned value then the compiler issues a warning. Without the annotation the fact that you did not store the value would not generate any warning. – Kevin Spaghetti Jun 16 '23 at 11:28
1

No, this annotation is not needed in this case.

The suggestion to add it came from your text editor. Your text editor is not a C++ compiler. Only a C++ compiler fully understands C++ source code. Your C++ text editor merely has a few, simple, rules for adding suggestions that it thinks are helpful in improving code. The specific reason why it might've shown this suggestion are mere speculation, without knowing exactly what your text editor's logic does.

which return values should not be ignored.

The key word there is "should". It is not an error to ignore values returned from functions or class methods. That, by itself, does not make code ill-formed. Now, it's true that in some instances ignoring return values can lead to issues. But that must require other factors to come into play. For example, ignoring the return value from read() and blissfully assuming that anything was actually read from a file or a socket, is nearly always a bug.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • So, for example, `int read(int[] buffer, int buffer_size)` usually modifies the content of the input array and returns the number of read bytes. If I were to ignore `n_bytes` and only do `read(my_buffer, 256)`, I would probably process rubbish data. So, if the `[[nodiscard]]` annotation was added to the signature of `read`, the compiler can help its users to discover this error? Did I understood it correctly? That means, `[[nodiscard]]` is a hint to my users that the return type is actually important to consider? – Green绿色 Jun 16 '23 at 11:28
  • This is correct. – Sam Varshavchik Jun 16 '23 at 12:57
  • I don't agree with the first part of your answer. The IDEs I use are all linting with clang (I do not know about CLion specifically). So yes, the linter "is" the compiler and the linter understands the code the very same as the compiler does. Which is not of the greatest help here--if the compiler knew where nodiscard makes sense, we wouldn't need to specify it. – ypnos Jun 17 '23 at 09:26