[clang-tools-extra] [clang-tidy][readability-container-contains] Fix matching of non-binaryOperator cases (PR #110386)
Nicolas van Kempen via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 21 20:15:19 PDT 2024
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/110386
>From e1dc2f848ee74fbd2775c4943c6bf03990f7c164 Mon Sep 17 00:00:00 2001
From: Nicolas van Kempen <nvankemp at gmail.com>
Date: Sun, 29 Sep 2024 14:05:33 -0400
Subject: [PATCH] [clang-tidy][readability-container-contains] Fix matching of
non-binaryOperator cases
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
```
---
.../readability/ContainerContainsCheck.cpp | 40 +++++++++----------
clang-tools-extra/docs/ReleaseNotes.rst | 3 +-
.../readability/container-contains.cpp | 11 +++--
3 files changed, 30 insertions(+), 24 deletions(-)
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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a9b1ab367f538a..ff6f39b3e3bb40 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -234,7 +234,8 @@ Changes in existing checks
- Improved :doc:`readability-container-contains
<clang-tidy/checks/readability/container-contains>` check to let it work on
- any class that has a ``contains`` method.
+ any class that has a ``contains`` method. Fix some false negatives in the
+ ``find()`` case.
- Improved :doc:`readability-enum-initial-value
<clang-tidy/checks/readability/enum-initial-value>` check by only issuing
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>
More information about the cfe-commits
mailing list