llvm / llvm-project

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

clang-tidy: checks "readability-container-contains" does not handle "find()" #79437

Closed adesitter closed 3 weeks ago

adesitter commented 9 months ago

$ cat qq.cpp

#include <map>
bool my_contains_1(std::map<int, int> const& m, int key)
{
   return m.find(key) != m.end(); 
}
bool my_contains_2(std::map<int, int> const& m, int key)
{
   return m.count(key) > 0; 
}

$ /usr/local/clang-17/bin/clang-tidy '-checks=readability-container-contains' contains.cpp -- -std=c++20 1 warning generated. /tmp/contains.cpp:8:13: warning: use 'contains' to check for membership [readability-container-contains] 8 | return m.count(key) > 0; | ^~~ ~ | contains

Despite https://clang.llvm.org/extra/clang-tidy/checks/readability/container-contains.html, readability-container-contains" does not handle "find()".

PiotrZSL commented 9 months ago

Interesting, in C++20 this code looks like this:

 `-CXXRewrittenBinaryOperator <col:11, col:32> 'bool'
|         `-UnaryOperator <col:23, col:32> 'bool' prefix '!' cannot overflow
|           `-CXXOperatorCallExpr <col:11, col:32> 'bool' '==' adl

when in C++17:

CXXOperatorCallExpr <col:11, col:32> 'bool' '!=' adl

And check support only binaryOperator, so it won't work on iterators objects. Looks like support for those 2 need simply be added.

adesitter commented 9 months ago

readability-container-contains is only relevant in C++20 mode. operator== is equally affected:

$ cat contains2.cpp 
#include <map>
bool my_not_contains_1(std::map<int, int> const& m, int key)
{
   return m.find(key) == m.end(); 
}
bool my_not_contains_2(std::map<int, int> const& m, int key)
{
   return m.count(key) == 0; 
}
$ /usr/local/clang-17/bin/clang-tidy '-checks=readability-container-contains' contains2.cpp -- -std=c++20
1 warning generated.
/tmp/contains2.cpp:8:13: warning: use 'contains' to check for membership [readability-container-contains]
    8 |    return m.count(key) == 0; 
      |             ^~~~~      ~~~~
      |           ! contains
adesitter commented 9 months ago

readability-container-contains does not handle pointer dereferencing.

$ cat contains.cpp 
#include <map>
bool my_contains_1(std::map<int, int> const* m, int key)
{
   return m->find(key) != m->end(); 
}
bool my_not_contains_1(std::map<int, int> const* m, int key)
{
   return m->find(key) == m->end(); 
}
bool my_contains_2(std::map<int, int> const* m, int key)
{
   return m->count(key) > 0; 
}
bool my_not_contains_2(std::map<int, int> const* m, int key)
{
   return m->count(key) == 0; 
}
$ /usr/local/clang-17/bin/clang-tidy '-checks=readability-container-contains' contains.cpp -- -std=c++20