`bugprone-unused-return-value` reported on iterator increment

[firewave]

With bugprone-unused-return-value.CheckFunctions set to ::.*.

#include <string>

void f()
{
    std::string s;
    for (std::string::const_iterator it = s.cbegin(); it != s.cend(); ++it) {}
    for (std::string::const_iterator it = s.cbegin(); it != s.cend(); it++) {}
}

test.cpp:6:71: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [bugprone-unused-return-value,-warnings-as-errors]
    6 |     for (std::string::const_iterator it = s.cbegin(); it != s.cend(); ++it) {}
      |                                                                       ^~~~
test.cpp:6:71: note: cast the expression to void to silence this warning
test.cpp:7:71: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [bugprone-unused-return-value,-warnings-as-errors]
    7 |     for (std::string::const_iterator it = s.cbegin(); it != s.cend(); it++) {}
      |                                                                       ^~~~
test.cpp:7:71: note: cast the expression to void to silence this warning

The result is actually used by the loop condition.

Some context on why I am trying to match all functions which might seem excessive.
This is actually something I wanted for quite a while. The idea was a bit more limited in that I wanted to find functions within test code which do not check the results. But simply enabling it in the context of the test folder actually has the same effect and actually goes a bit further.

[firewave]

CC @HerrCai0907

[firewave]

The result is actually used by the loop condition.

Stupid me. Obviously it is not used as it is in the "end of loop" part.

But still this is a common pattern so it might be unreasonable to warn. But it might also be my use case which is unreasonable.

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Oliver Stöneberg (firewave)

With `bugprone-unused-return-value.CheckFunctions` set to `::.*`.

#include <string>

void f()
{
    std::string s;
    for (std::string::const_iterator it = s.cbegin(); it != s.cend(); ++it) {}
    for (std::string::const_iterator it = s.cbegin(); it != s.cend(); it++) {}
}

test.cpp:6:71: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [bugprone-unused-return-value,-warnings-as-errors]
    6 |     for (std::string::const_iterator it = s.cbegin(); it != s.cend(); ++it) {}
      |                                                                       ^~~~
test.cpp:6:71: note: cast the expression to void to silence this warning
test.cpp:7:71: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [bugprone-unused-return-value,-warnings-as-errors]
    7 |     for (std::string::const_iterator it = s.cbegin(); it != s.cend(); it++) {}
      |                                                                       ^~~~
test.cpp:7:71: note: cast the expression to void to silence this warning

The result is actually used by the loop condition.

Some context on why I am trying to match all functions which might seem excessive.
This is actually something I wanted for quite a while. The idea was a bit more limited in that I wanted to find functions within test code which do not check the results. But simply enabling it in the context of the test folder actually has the same effect and actually goes a bit further.

[firewave]

This occurs with any increment of an iterator.

[HerrCai0907]

Ahhh, beside +=, -=, there are still have ++, -- which have side effect and also need to be ignored