Why clang-tidy suggests to add [[nodiscard]] everywhere?

Question

I have a C++ project where clang-tidy is suggesting to add [[nodiscard]] everywhere. Is this a good practice ? The understanding I have is that [[nodiscard]] should be used only when ignoring the return value could be fatal for program. I have an object Car and it has a member const unsigned int m_ID. Should the getter unsigned int getID() have [[nodiscard]] ? clang-tidy suggests so.

EDIT:

Of course, I do not want to ignore a getter. BUT
My point is if every function that returns something should have a [[nodiscard]], then the attribute [[nodiscard]] is anyway redundant. Compiler can simply check all functions that return something.

user_734069 · Answer

This option is apparently "modernize-use-nodiscard", so you can deactivate that if you prefer.

It should be noted that the rules this option outlines are not the rules the C++ standard committee themselves use for when to apply [[nodiscard]]. Those rules being:

It should be added where:

For existing API’s

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)

For new API’s (not been in the C++ standard yet)

not using the return value is usually an error.

It should not be added when:

For existing API’s

not using the return value is a possible/common way of programming at least for some input

for example for realloc(), which acts like free when the new site[sic] is 0

not using the return value makes no sense but doesn’t hurt and is usually not an error (e.g., because programmers meant to ask for a state change).

it is a C function, because their declaration might not be under control of the C++ implementation

This is why functions like operator new are [[nodiscard]], while functions like optional::value are not. There is a difference between being your code having a minor mistake and your code being fundamentally broken. [[nodiscard]], as far as the committee is concerned, is for the latter.

Note that container empty methods are a special case. They seem to fit the "do not use [[nodiscard]]" pattern, but because the name of empty is similar to the name for clear, if you don't use the return value of empty, odds are good that you meant to call clear.

Obviously, this cannot be known from just a declaration, so there's no way for Clang-Tidy to implement said rules.

user_2079303 · Answer

Why clang-tidy suggests to add [[nodiscard]] everywhere?

clang-tidy doesn't suggest to add [[nodiscard]] everywhere. The cases where it is suggested are described in the documentation of the check.

Is this a good practice ?

Yes, using [[nodiscard]] is a good practice when discarding the result is likely a bug. That is the case quite often.

Should the getter unsigned int getID() have [[nodiscard]] ?

Can you imagine any use case where it would be useful to call that getter without using the returned value? If you are certain that such case won't exist, then you should use [[nodiscard]]. I think such case doesn't exist in the described example.

The understanding I have is that [[nodiscard]] should be used only when ignoring the return value could be fatal for program.

That's a rather conservative understanding. You can disable the check in question if you don't agree with it.

Why clang-tidy suggests to add [[nodiscard]] everywhere?

2 Answers