[clang-tools-extra] [clang-tidy][readability-container-contains] Fix matching of non-binaryOperator cases (PR #110386)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Sep 28 14:53:36 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-tools-extra
Author: Nicolas van Kempen (nicovank)
<details>
<summary>Changes</summary>
Fix #<!-- -->79437.
It works with non-mock `std::map`:
```
# All cases detailed in #<!-- -->79437.
> cat tmp.cpp
#include <map>
bool a(std::map<int, int> &m, int key) { return m.find(key) != m.end(); }
bool b(std::map<int, int> &m, int key) { return m.count(key) > 0; }
bool c(std::map<int, int> &m, int key) { return m.find(key) == m.end(); }
bool d(std::map<int, int> &m, int key) { return m.count(key) == 0; }
bool e(std::map<int, int> *m, int key) { return m->find(key) != m->end(); }
bool f(std::map<int, int> *m, int key) { return m->find(key) == m->end(); }
bool g(std::map<int, int> *m, int key) { return m->count(key) > 0; }
bool h(std::map<int, int> *m, int key) { return m->count(key) == 0; }
> ./build/bin/clang-tidy -checks="-*,readability-container-contains" tmp.cpp -- -std=c++20
8 warnings generated.
tmp.cpp:2:51: warning: use 'contains' to check for membership [readability-container-contains]
2 | bool a(std::map<int, int> &m, int key) { return m.find(key) != m.end(); }
| ^~~~ ~~~~~~~~~~
| contains
tmp.cpp:3:51: warning: use 'contains' to check for membership [readability-container-contains]
3 | bool b(std::map<int, int> &m, int key) { return m.count(key) > 0; }
| ^~~~~ ~~~
| contains
tmp.cpp:4:51: warning: use 'contains' to check for membership [readability-container-contains]
4 | bool c(std::map<int, int> &m, int key) { return m.find(key) == m.end(); }
| ^~~~ ~~~~~~~~~~
| ! contains
tmp.cpp:5:51: warning: use 'contains' to check for membership [readability-container-contains]
5 | bool d(std::map<int, int> &m, int key) { return m.count(key) == 0; }
| ^~~~~ ~~~~
| ! contains
tmp.cpp:6:52: warning: use 'contains' to check for membership [readability-container-contains]
6 | bool e(std::map<int, int> *m, int key) { return m->find(key) != m->end(); }
| ^~~~ ~~~~~~~~~~~
| contains
tmp.cpp:7:52: warning: use 'contains' to check for membership [readability-container-contains]
7 | bool f(std::map<int, int> *m, int key) { return m->find(key) == m->end(); }
| ^~~~ ~~~~~~~~~~~
| ! contains
tmp.cpp:8:52: warning: use 'contains' to check for membership [readability-container-contains]
8 | bool g(std::map<int, int> *m, int key) { return m->count(key) > 0; }
| ^~~~~ ~~~
| contains
tmp.cpp:9:52: warning: use 'contains' to check for membership [readability-container-contains]
9 | bool h(std::map<int, int> *m, int key) { return m->count(key) == 0; }
| ^~~~~ ~~~~
| ! contains
```
---
Full diff: https://github.com/llvm/llvm-project/pull/110386.diff
2 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp (+20-20)
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp (+8-3)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
index 05d4c87bc73cef..fb68c7d334b7f8 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -62,44 +62,44 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
.bind("positiveComparison"),
this);
AddSimpleMatcher(
- binaryOperator(hasOperatorName("!="), hasOperands(CountCall, Literal0))
+ binaryOperation(hasOperatorName("!="), hasOperands(CountCall, Literal0))
.bind("positiveComparison"));
AddSimpleMatcher(
- binaryOperator(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0))
+ binaryOperation(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0))
.bind("positiveComparison"));
AddSimpleMatcher(
- binaryOperator(hasLHS(Literal0), hasOperatorName("<"), hasRHS(CountCall))
- .bind("positiveComparison"));
- AddSimpleMatcher(
- binaryOperator(hasLHS(CountCall), hasOperatorName(">="), hasRHS(Literal1))
- .bind("positiveComparison"));
- AddSimpleMatcher(
- binaryOperator(hasLHS(Literal1), hasOperatorName("<="), hasRHS(CountCall))
+ binaryOperation(hasLHS(Literal0), hasOperatorName("<"), hasRHS(CountCall))
.bind("positiveComparison"));
+ AddSimpleMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName(">="),
+ hasRHS(Literal1))
+ .bind("positiveComparison"));
+ AddSimpleMatcher(binaryOperation(hasLHS(Literal1), hasOperatorName("<="),
+ hasRHS(CountCall))
+ .bind("positiveComparison"));
// Find inverted membership tests which use `count()`.
AddSimpleMatcher(
- binaryOperator(hasOperatorName("=="), hasOperands(CountCall, Literal0))
- .bind("negativeComparison"));
- AddSimpleMatcher(
- binaryOperator(hasLHS(CountCall), hasOperatorName("<="), hasRHS(Literal0))
- .bind("negativeComparison"));
- AddSimpleMatcher(
- binaryOperator(hasLHS(Literal0), hasOperatorName(">="), hasRHS(CountCall))
+ binaryOperation(hasOperatorName("=="), hasOperands(CountCall, Literal0))
.bind("negativeComparison"));
+ AddSimpleMatcher(binaryOperation(hasLHS(CountCall), hasOperatorName("<="),
+ hasRHS(Literal0))
+ .bind("negativeComparison"));
+ AddSimpleMatcher(binaryOperation(hasLHS(Literal0), hasOperatorName(">="),
+ hasRHS(CountCall))
+ .bind("negativeComparison"));
AddSimpleMatcher(
- binaryOperator(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1))
+ binaryOperation(hasLHS(CountCall), hasOperatorName("<"), hasRHS(Literal1))
.bind("negativeComparison"));
AddSimpleMatcher(
- binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall))
+ binaryOperation(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall))
.bind("negativeComparison"));
// Find membership tests based on `find() == end()`.
AddSimpleMatcher(
- binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall))
+ binaryOperation(hasOperatorName("!="), hasOperands(FindCall, EndCall))
.bind("positiveComparison"));
AddSimpleMatcher(
- binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall))
+ binaryOperation(hasOperatorName("=="), hasOperands(FindCall, EndCall))
.bind("negativeComparison"));
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
index 9a9b233e07229b..f345b3e7768a8a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
@@ -1,14 +1,19 @@
-// RUN: %check_clang_tidy -std=c++20-or-later %s readability-container-contains %t
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-container-contains %t
// Some *very* simplified versions of `map` etc.
namespace std {
template <class Key, class T>
struct map {
+ struct iterator {
+ bool operator==(const iterator &Other) const;
+ bool operator!=(const iterator &Other) const;
+ };
+
unsigned count(const Key &K) const;
bool contains(const Key &K) const;
- void *find(const Key &K);
- void *end();
+ iterator find(const Key &K);
+ iterator end();
};
template <class Key>
``````````
</details>
https://github.com/llvm/llvm-project/pull/110386
More information about the cfe-commits
mailing list