llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.48k stars 11.77k forks source link

detect mismatching argument in predicate #104572

Open firewave opened 1 month ago

firewave commented 1 month ago
#include <algorithm>
#include <map>
#include <string>

void f()
{
    const std::map<int, std::string> m;

    auto it = std::find_if(m.cbegin(), m.cend(), [&](const std::pair<int, std::string>& p){
        return p.second.empty();
    });
    (void)it;
}

https://godbolt.org/z/T5jPWov98

The used type in the predicate argument is wrong. It needs to be std::pair<const int, std::string>. Depending on the data being stored this might lead to it being copied on each invocation.

This could be fixed by using decltype(m.cbegin())::value_type or (starting with C++14) auto. But I do not think the latter should be suggested by this check. That should be suggested by modernize-use-auto instead so the logic stays in a single place (see #104569).

This should probably be reported if the given argument is a const reference and the type does not match decltype(<first>)::value_type. Specifying it by-value is allowed starting with C++11 if "for VT a move is equivalent to a copy" according to https://en.cppreference.com/w/cpp/algorithm/find.

llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-tidy

Author: Oliver Stöneberg (firewave)

```cpp #include <algorithm> #include <map> #include <string> void f() { const std::map<int, std::string> m; auto it = std::find_if(m.cbegin(), m.cend(), [&](const std::pair<int, std::string>& p){ return p.second.empty(); }); (void)it; } ``` https://godbolt.org/z/T5jPWov98 The used type in the predicate argument is wrong. It needs to be `std::pair<const int, std::string>`. Depending on the data being stored this might lead to it being copied on each invocation. This could be fixed by using `decltype(m.cbegin())::value_type` or (starting with C++14) `auto`. But I do not think the latter should be suggested by this check. That should be suggested by `modernize-use-auto` instead so the logic stays in a single place (see #104569). This should probably be reported if the given argument is a const reference and the type does not match `decltype(<first>)::value_type`. Specifying it by-value is allowed starting with C++11 if "for VT a move is equivalent to a copy" according to https://en.cppreference.com/w/cpp/algorithm/find.