https://github.com/MitraGulla updated https://github.com/llvm/llvm-project/pull/234905
>From 3f15bbd283f9b67494ef810ec93edc4cd11bd017 Mon Sep 17 00:00:00 2001 From: MitraGulla <[email protected]> Date: Fri, 23 Oct 2026 15:40:16 +0530 Subject: [PATCH 1/2] [clang-tidy] Avoid replacing match_results length() with empty() --- .../readability/ContainerSizeEmptyCheck.cpp | 22 +++- .../readability/ContainerSizeEmptyCheck.h | 1 + .../readability/container-size-empty.md | 7 ++ .../checkers/Inputs/Headers/std/regex | 23 ++++ .../readability/container-size-empty.cpp | 115 ++++++++++++++++++ 5 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp index 2db5385052843f..ea7a79a55a07d2 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -135,10 +135,14 @@ using utils::isBinaryOrTernary; ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), + ExcludedLengthTypes(utils::options::parseStringList( + Options.get("ExcludedLengthTypes", "::std::match_results"))), ExcludedComparisonTypes(utils::options::parseStringList( Options.get("ExcludedComparisonTypes", "::std::array"))) {} void ContainerSizeEmptyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "ExcludedLengthTypes", + utils::options::serializeStringList(ExcludedLengthTypes)); Options.store(Opts, "ExcludedComparisonTypes", utils::options::serializeStringList(ExcludedComparisonTypes)); } @@ -168,6 +172,15 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { const auto ValidContainer = qualType( anyOf(ValidContainerNonTemplateType, ValidContainerTemplateType)); + const auto ExcludedLengthTypesMatcher = qualType( + anyOf(hasDeclaration(cxxRecordDecl( + matchers::matchesAnyListedRegexName(ExcludedLengthTypes))), + hasCanonicalType(hasDeclaration(cxxRecordDecl( + matchers::matchesAnyListedRegexName(ExcludedLengthTypes)))))); + const auto ExcludedLengthExprType = qualType( + anyOf(ExcludedLengthTypesMatcher, pointsTo(ExcludedLengthTypesMatcher), + references(ExcludedLengthTypesMatcher))); + const auto WrongUse = anyOf(hasParent(binaryOperator( isComparisonOperator(), @@ -189,7 +202,9 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { argumentCountIs(0), on(ValidContainerExpr), callee( cxxMethodDecl(hasAnyName("size", "length")).bind("SizeMethod")), - WrongUse, NotInEmptyMethodOfContainer) + WrongUse, NotInEmptyMethodOfContainer, + unless(allOf(callee(cxxMethodDecl(hasName("length"))), + on(expr(hasType(ExcludedLengthExprType)))))) .bind("SizeCallExpr"), this); @@ -198,7 +213,10 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { has(mapAnyOf(memberExpr, cxxDependentScopeMemberExpr) .with(hasObjectExpression(ValidContainerExpr), anyOf(matchMemberName("size"), - matchMemberName("length"))) + matchMemberName("length")), + unless(allOf(matchMemberName("length"), + hasObjectExpression(expr(hasType( + ExcludedLengthExprType)))))) .bind("MemberExpr")), WrongUse, NotInEmptyMethodOfContainer) .bind("SizeCallExpr"), diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h index 6d984d29ada66b..0d968c31ec7885 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h @@ -37,6 +37,7 @@ class ContainerSizeEmptyCheck : public ClangTidyCheck { } private: + std::vector<StringRef> ExcludedLengthTypes; std::vector<StringRef> ExcludedComparisonTypes; }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.md b/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.md index 89b5f9fbbc886d..418349034c3576 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.md +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.md @@ -33,3 +33,10 @@ objects of the same type. If a class is listed here, the check will not suggest using `empty()` instead of such comparisons for objects of that class. Default is `::std::array`. ``` + +```{option} ExcludedLengthTypes +A semicolon-separated list of regular expressions matching class names for +which the check will not suggest replacing calls to `length()` with +`empty()`. This is useful for types where `length()` and `empty()` have +different semantics. Default is `::std::match_results`. +``` diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex new file mode 100644 index 00000000000000..f293fde4ee3dce --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex @@ -0,0 +1,23 @@ +#ifndef TEST_REGEX +#define TEST_REGEX + +namespace std { + +template <typename BidirectionalIterator> +class match_results { +public: + using size_type = unsigned long; + + size_type length(size_type sub = 0) const; + size_type size() const; + bool empty() const; +}; + +using smatch = match_results<const char *>; +using cmatch = match_results<const char *>; +using wsmatch = match_results<const wchar_t *>; +using wcmatch = match_results<const wchar_t *>; + +} // namespace std + +#endif diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp index 79c42c213002f5..e1a63e64527536 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp @@ -3,6 +3,7 @@ // RUN: -- -fno-delayed-template-parsing #include <string> #include <memory> +#include <regex> namespace std { template <typename T> struct vector { @@ -35,6 +36,120 @@ auto empty(const C& c) -> decltype(c.empty()); } +void testRegexMatchResults(std::smatch &sm, std::cmatch &cm, + std::wsmatch &wsm, std::wcmatch &wcm) { + if (sm.length()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + if (!sm.length()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + + if (sm.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!sm.empty()) {} + + if (sm.empty()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + + if (cm.length()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + if (cm.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!cm.empty()) {} + if (cm.empty()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + + if (wsm.length()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + if (wsm.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!wsm.empty()) {} + if (wsm.empty()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + + if (wcm.length()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + if (wcm.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!wcm.empty()) {} + if (wcm.empty()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + + // length() must also be excluded when compared with zero. + if (sm.length() == 0) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + if (sm.length() != 0) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + + // size() remains safe to replace with empty(). + if (!sm.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (sm.empty()) {} + + if (sm.size() == 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (sm.empty()) {} + + if (sm.size() != 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!sm.empty()) {} +} + +void testOrdinaryContainers(std::vector<int> &vect, std::string &str) { + if (vect.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!vect.empty()) {} + + if (!vect.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (vect.empty()) {} + + if (vect.size() == 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (vect.empty()) {} + + if (vect.size() != 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!vect.empty()) {} + + if (vect.empty()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + + if (str.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!str.empty()) {} + + if (!str.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (str.empty()) {} + + if (str.length()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty] + // CHECK-FIXES: if (!str.empty()) {} + + if (!str.length()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty] + // CHECK-FIXES: if (str.empty()) {} + + if (str.size() == 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (str.empty()) {} + + if (str.length() == 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty] + // CHECK-FIXES: if (str.empty()) {} + + if (str.size() != 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] + // CHECK-FIXES: if (!str.empty()) {} + + if (str.length() != 0) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty] + // CHECK-FIXES: if (!str.empty()) {} + + if (str.empty()) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: +} + template <typename T> class TemplatedContainer { public: >From 9c8e767bd44a67efa6513734a3f52bc8a5c405b0 Mon Sep 17 00:00:00 2001 From: MitraGulla <[email protected]> Date: Fri, 23 Oct 2026 21:06:40 +0530 Subject: [PATCH 2/2] [clang-tidy] Improve match_results regression tests --- clang-tools-extra/docs/ReleaseNotes.md | 5 ++++ .../checkers/Inputs/Headers/std/regex | 7 ++--- .../checkers/Inputs/Headers/std/string | 1 + .../readability/container-size-empty.cpp | 26 +++++++++---------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index b48662c1934423..2b29524309034f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -340,6 +340,11 @@ infrastructure are described first, followed by tool-specific sections. fixing a crash when checking a const-qualified method declared with the `lifetimebound` attribute. +- Improved {doc}`readability-container-size-empty + <clang-tidy/checks/readability/container-size-empty>` check to avoid + suggesting `empty()` for `std::match_results::length()`, whose return value + can be zero even when the match results object is not empty. + - Improved {doc}`readability-enum-initial-value <clang-tidy/checks/readability/enum-initial-value>` check by adding the {option}`AllowReferencedInitialValues` to support the diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex index f293fde4ee3dce..add9bbc4549d93 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex @@ -7,15 +7,16 @@ template <typename BidirectionalIterator> class match_results { public: using size_type = unsigned long; + using difference_type = long; - size_type length(size_type sub = 0) const; + difference_type length(size_type sub = 0) const; size_type size() const; bool empty() const; }; -using smatch = match_results<const char *>; +using smatch = match_results<string::const_iterator>; using cmatch = match_results<const char *>; -using wsmatch = match_results<const wchar_t *>; +using wsmatch = match_results<wstring::const_iterator>; using wcmatch = match_results<const wchar_t *>; } // namespace std diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string index 766f240c655fbc..bd29f72359a1e2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string @@ -19,6 +19,7 @@ struct basic_string_view; template <typename C, typename T = char_traits<C>, typename A = allocator<C>> struct basic_string { typedef size_t size_type; + typedef const C *const_iterator; typedef basic_string<C, T, A> _Type; basic_string(); basic_string(const basic_string &); diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp index e1a63e64527536..0d844f86055c42 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp @@ -39,46 +39,46 @@ auto empty(const C& c) -> decltype(c.empty()); void testRegexMatchResults(std::smatch &sm, std::cmatch &cm, std::wsmatch &wsm, std::wcmatch &wcm) { if (sm.length()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (!sm.length()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:8: warning: if (sm.size()) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] // CHECK-FIXES: if (!sm.empty()) {} if (sm.empty()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (cm.length()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (cm.size()) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] // CHECK-FIXES: if (!cm.empty()) {} if (cm.empty()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (wsm.length()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (wsm.size()) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] // CHECK-FIXES: if (!wsm.empty()) {} if (wsm.empty()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (wcm.length()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (wcm.size()) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] // CHECK-FIXES: if (!wcm.empty()) {} if (wcm.empty()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: // length() must also be excluded when compared with zero. if (sm.length() == 0) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (sm.length() != 0) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: // size() remains safe to replace with empty(). if (!sm.size()) {} @@ -112,7 +112,7 @@ void testOrdinaryContainers(std::vector<int> &vect, std::string &str) { // CHECK-FIXES: if (!vect.empty()) {} if (vect.empty()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: if (str.size()) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] @@ -147,7 +147,7 @@ void testOrdinaryContainers(std::vector<int> &vect, std::string &str) { // CHECK-FIXES: if (!str.empty()) {} if (str.empty()) {} - // CHECK-MESSAGES-NOT: :[[@LINE-1]]:{{[0-9]+}}: warning: + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:7: warning: } template <typename T> _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
