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 <nvank...@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> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits