https://github.com/dl8sd11 updated https://github.com/llvm/llvm-project/pull/110351
>From b98e9a4d50d74c298096d2bd2d70ff4c0ef5c4a4 Mon Sep 17 00:00:00 2001 From: dl8sd11 <gcc...@google.com> Date: Sat, 28 Sep 2024 07:37:50 +0000 Subject: [PATCH 1/4] [clang-tidy] support string::contains --- .../readability/ContainerContainsCheck.cpp | 18 +++++-- .../readability/container-contains.cpp | 49 +++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp index 05d4c87bc73cef..a5e5960eaa6a3c 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp @@ -9,6 +9,7 @@ #include "ContainerContainsCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" using namespace clang::ast_matchers; @@ -32,7 +33,8 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { const auto FindCall = cxxMemberCallExpr( - argumentCountIs(1), + anyOf(argumentCountIs(1), + allOf(argumentCountIs(2), hasArgument(1, cxxDefaultArgExpr()))), callee(cxxMethodDecl( hasName("find"), hasParameter(0, hasType(hasUnqualifiedDesugaredType( @@ -51,6 +53,12 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { const auto Literal0 = integerLiteral(equals(0)); const auto Literal1 = integerLiteral(equals(1)); + const auto StringLikeClass = cxxRecordDecl( + hasAnyName("::std::basic_string", "::std::basic_string_view", + "::absl::string_view")); + const auto StringNpos = declRefExpr( + to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass)))); + auto AddSimpleMatcher = [&](auto Matcher) { Finder->addMatcher( traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this); @@ -94,12 +102,14 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall)) .bind("negativeComparison")); - // Find membership tests based on `find() == end()`. + // Find membership tests based on `find() == end() or find() == npos`. AddSimpleMatcher( - binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall)) + binaryOperator(hasOperatorName("!="), + hasOperands(FindCall, anyOf(EndCall, StringNpos))) .bind("positiveComparison")); AddSimpleMatcher( - binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall)) + binaryOperator(hasOperatorName("=="), + hasOperands(FindCall, anyOf(EndCall, StringNpos))) .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..69cc5c88479040 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 @@ -29,6 +29,43 @@ struct multimap { bool contains(const Key &K) const; }; +using size_t = decltype(sizeof(int)); + +// Lightweight standin for std::string_view. +template <typename C> +class basic_string_view { +public: + basic_string_view(); + basic_string_view(const basic_string_view &); + basic_string_view(const C *); + ~basic_string_view(); + int find(basic_string_view s, int pos = 0); + int find(const C *s, int pos = 0); + int find(const C *s, int pos, int n); + int find(char c, int pos = 0); + static constexpr size_t npos = -1; +}; +typedef basic_string_view<char> string_view; + +// Lightweight standin for std::string. +template <typename C> +class basic_string { +public: + basic_string(); + basic_string(const basic_string &); + basic_string(const C *); + ~basic_string(); + int find(basic_string s, int pos = 0); + int find(const C *s, int pos = 0); + int find(const C *s, int pos, int n); + int find(char c, int pos = 0); + bool contains(const C *s) const; + bool contains(C s) const; + bool contains(basic_string_view<C> s) const; + static constexpr size_t npos = -1; +}; +typedef basic_string<char> string; + } // namespace std // Check that we detect various common ways to check for membership @@ -453,3 +490,15 @@ void testOperandPermutations(std::map<int, int>& Map) { // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] // CHECK-FIXES: if (!Map.contains(0)) {}; } + +void testStringNops(std::string Str, std::string SubStr) { + if (Str.find("test") == std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!Str.contains("test")) {}; + + if (Str.find('c') != std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Str.contains('c')) {}; + + if (Str.find(SubStr) != std::string::npos) {}; +} >From e42c356bdd679beaf1ee9199afeee566726e9fc9 Mon Sep 17 00:00:00 2001 From: dl8sd11 <gcc...@google.com> Date: Sat, 5 Oct 2024 04:24:41 +0000 Subject: [PATCH 2/4] [clang-tidy] fix string::find(s, 0) --- .../readability/ContainerContainsCheck.cpp | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp index a5e5960eaa6a3c..fc05498b47e2e1 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp @@ -8,6 +8,7 @@ #include "ContainerContainsCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -31,10 +32,15 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { ofClass(cxxRecordDecl(HasContainsMatchingParamType))))) .bind("call"); + const auto Literal0 = integerLiteral(equals(0)); + const auto Literal1 = integerLiteral(equals(1)); + const auto FindCall = cxxMemberCallExpr( anyOf(argumentCountIs(1), - allOf(argumentCountIs(2), hasArgument(1, cxxDefaultArgExpr()))), + allOf(argumentCountIs(2), + hasArgument(1, anyOf(cxxDefaultArgExpr(), + ignoringParenImpCasts(Literal0))))), callee(cxxMethodDecl( hasName("find"), hasParameter(0, hasType(hasUnqualifiedDesugaredType( @@ -50,14 +56,8 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { // before EndCall so 'parameterType' is properly bound. ofClass(cxxRecordDecl(HasContainsMatchingParamType))))); - const auto Literal0 = integerLiteral(equals(0)); - const auto Literal1 = integerLiteral(equals(1)); - - const auto StringLikeClass = cxxRecordDecl( - hasAnyName("::std::basic_string", "::std::basic_string_view", - "::absl::string_view")); - const auto StringNpos = declRefExpr( - to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass)))); + const auto StringNpos = anyOf(declRefExpr(to(varDecl(hasName("npos")))), + memberExpr(member(hasName("npos")))); auto AddSimpleMatcher = [&](auto Matcher) { Finder->addMatcher( @@ -102,7 +102,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall)) .bind("negativeComparison")); - // Find membership tests based on `find() == end() or find() == npos`. + // Find membership tests based on `find() == end()` or `find() == npos`. AddSimpleMatcher( binaryOperator(hasOperatorName("!="), hasOperands(FindCall, anyOf(EndCall, StringNpos))) >From 9d44ee040204e106ecaea4694949693d5f6dce15 Mon Sep 17 00:00:00 2001 From: dl8sd11 <gcc...@google.com> Date: Sat, 5 Oct 2024 04:25:55 +0000 Subject: [PATCH 3/4] [clang-tidy] add string::contains --- .../test/clang-tidy/checkers/Inputs/Headers/string | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string index 0c160bc182b6eb..b1a3f40c10f185 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string @@ -50,12 +50,16 @@ struct basic_string { size_type find(const _Type& str, size_type pos = 0) const; size_type find(const C* s, size_type pos = 0) const; size_type find(const C* s, size_type pos, size_type n) const; + size_type find(C ch, size_type pos = 0) const; size_type rfind(const _Type& str, size_type pos = npos) const; size_type rfind(const C* s, size_type pos, size_type count) const; size_type rfind(const C* s, size_type pos = npos) const; size_type rfind(C ch, size_type pos = npos) const; + bool contains(const C *s) const; + bool contains(C s) const; + _Type& insert(size_type pos, const _Type& str); _Type& insert(size_type pos, const C* s); _Type& insert(size_type pos, const C* s, size_type n); @@ -104,6 +108,9 @@ struct basic_string_view { size_type rfind(const C* s, size_type pos, size_type count) const; size_type rfind(const C* s, size_type pos = npos) const; + bool contains(const C *s) const; + bool contains(C s) const; + constexpr bool starts_with(basic_string_view sv) const noexcept; constexpr bool starts_with(C ch) const noexcept; constexpr bool starts_with(const C* s) const; >From 36252210a2cd6ce53ab3f25d8c7b863807f00b0d Mon Sep 17 00:00:00 2001 From: dl8sd11 <gcc...@google.com> Date: Sat, 5 Oct 2024 04:26:25 +0000 Subject: [PATCH 4/4] [clang-tidy] adopt mock headers and add string_view tests --- .../readability/container-contains.cpp | 62 +++++++------------ 1 file changed, 22 insertions(+), 40 deletions(-) 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 69cc5c88479040..4fa2a378cff2c0 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,4 +1,7 @@ -// RUN: %check_clang_tidy -std=c++20-or-later %s readability-container-contains %t +// RUN: %check_clang_tidy -std=c++20-or-later %s readability-container-contains %t -- \ +// RUN: -- -isystem %clang_tidy_headers + +#include <string> // Some *very* simplified versions of `map` etc. namespace std { @@ -29,42 +32,7 @@ struct multimap { bool contains(const Key &K) const; }; -using size_t = decltype(sizeof(int)); - -// Lightweight standin for std::string_view. -template <typename C> -class basic_string_view { -public: - basic_string_view(); - basic_string_view(const basic_string_view &); - basic_string_view(const C *); - ~basic_string_view(); - int find(basic_string_view s, int pos = 0); - int find(const C *s, int pos = 0); - int find(const C *s, int pos, int n); - int find(char c, int pos = 0); - static constexpr size_t npos = -1; -}; -typedef basic_string_view<char> string_view; - -// Lightweight standin for std::string. -template <typename C> -class basic_string { -public: - basic_string(); - basic_string(const basic_string &); - basic_string(const C *); - ~basic_string(); - int find(basic_string s, int pos = 0); - int find(const C *s, int pos = 0); - int find(const C *s, int pos, int n); - int find(char c, int pos = 0); - bool contains(const C *s) const; - bool contains(C s) const; - bool contains(basic_string_view<C> s) const; - static constexpr size_t npos = -1; -}; -typedef basic_string<char> string; + } // namespace std @@ -491,7 +459,7 @@ void testOperandPermutations(std::map<int, int>& Map) { // CHECK-FIXES: if (!Map.contains(0)) {}; } -void testStringNops(std::string Str, std::string SubStr) { +void testStringNops(std::string Str, std::string SubStr, std::string_view StrView) { if (Str.find("test") == std::string::npos) {}; // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] // CHECK-FIXES: if (!Str.contains("test")) {}; @@ -499,6 +467,20 @@ void testStringNops(std::string Str, std::string SubStr) { if (Str.find('c') != std::string::npos) {}; // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] // CHECK-FIXES: if (Str.contains('c')) {}; - - if (Str.find(SubStr) != std::string::npos) {}; + + if (Str.find('c') != Str.npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Str.contains('c')) {}; + + if (StrView.find("test") == std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!StrView.contains("test")) {}; + + if (StrView.find('c') != std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (StrView.contains('c')) {}; + + if (StrView.find('c') != StrView.npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (StrView.contains('c')) {}; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits