bugprone-unused-return-value warns about stream insertion operator (operator<<)

[firewave]

If you enable the check to warn about all unused returns values by setting bugprone-unused-return-value.CheckFunctions ::.* it will also report the unchecked usage of operator<<.

The following are all common code which is regularly unchecked. So I think it might sense to not warn about these.

#include <iostream>

void f()
{
std::cout << "str";
}

#include <sstream>

void f()
{
std::ostringstream oss;
oss << "str";
}

#include <fstream>

void f()
{
std::ofstream of("file");
of << "str";
}

It should still be reported for the extraction operator as it might indicate actual issues:

#include <sstream>

void f()
{
std::istringstream iss("str");
double d;
iss >> d;
}

In this case the extraction fails and you need to check the result. See https://godbolt.org/z/KzPor3hMK.

That could be a dedicated check though and this check could just ignore both of them.

CC @HerrCai0907

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Oliver Stöneberg (firewave)

If you enable the check to warn about all unused returns values by setting `bugprone-unused-return-value.CheckFunctions` `::.*` it will also report the unchecked usage of `operator<<`.

The following are all common code which is regularly unchecked. So I think it might sense to not warn about these.

#include <iostream>

void f()
{
std::cout << "str";
}

#include <sstream>

void f()
{
std::ostringstream oss;
oss << "str";
}

#include <fstream>

void f()
{
std::ofstream of("file");
of << "str";
}

It should still be reported for the extraction operator as it might indicate actual issues:

#include <sstream>

void f()
{
std::istringstream iss("str");
double d;
iss >> d;
}

In this case the extraction fails and you need to check the result. See https://godbolt.org/z/KzPor3hMK.

That could be a dedicated check though and this check could just ignore both of them.

CC @HerrCai0907

[HerrCai0907]

I don't think it is a good idea to ignore << operator since it has two different semantics. One for stream and the other for bit operator. So the check cannot ignore it simply.

[NagyDonat]

The checker could easily check the return type of operator<< and silence the reports where it's a stream.

[NagyDonat]

(This comment was edited, because I realized that I misunderstood the issue.)
I thought that I could implement a quick solution for this issue, but I didn't notice that:

If you enable the check to warn about all unused returns values by setting bugprone-unused-return-value.CheckFunctions ::.* it will also report the unchecked usage of operator<<.

I assumed that the issue happens with the default behavior of the checker, and not after a custom override that explicitly activates warnings on all unused return values.

I'd say that here the behavior of Clang-Tidy is 100% correct: if the user specifies ::.*, then it should indeed match all function calls. (It's important to ensure that the default behavior is sane and useful; but would be a bad idea to hardcode narrow exceptions that are aimed for other particular configurations.)

I don't think that using the wildcard ::.* is a good idea (I didn't check but I presume that operator<< is not the only problematic function), but if we want to support this, then we could theoretically extend the "configuration language" of the checker to let the user specify that they want to match "::.* minus these stream operators". However, this would be a non-trivial new feature and I don't have the time to implement it.

[HerrCai0907]

I'd say that here the behavior of Clang-Tidy is 100% correct: if the user specifies ::.*, then it should indeed match all function calls. (It's important to ensure that the default behavior is sane and useful; but would be a bad idea to hardcode narrow exceptions that are aimed for other particular configurations.)

Agree. The reason I disable the operator overloading for "+=", "-=" style things is that iIn the common sense, there is no ambiguity in these operators. But for "<<", unfortunately not.

[firewave]

See danmar/cppcheck#6673 for some context why/how I (ab)use this check.