[clang-tools-extra] [clang-tidy] Avoid replacing match_results length() with empty() (PR #234905)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 23 03:33:51 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-tools-extra
Author: MitraGulla
<details>
<summary>Changes</summary>
## Summary
The check can suggest replacing `length()` with `empty()` for `std::match_results`, but these two calls don't always mean the same thing. This change prevents the check from suggesting `empty()` when `length()` is called on `std::match_results`.
Added tests for its different aliases and kept the existing suggestions for other containers.
## Testing
- Ran `clang-format` on the modified C++ files.
- Ran the `container-size-empty.cpp` lit test successfully.
---
Full diff: https://github.com/llvm/llvm-project/pull/234905.diff
5 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp (+20-2)
- (modified) clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h (+1)
- (modified) clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.md (+7)
- (added) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/regex (+23)
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp (+115)
``````````diff
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:
``````````
</details>
https://github.com/llvm/llvm-project/pull/234905
More information about the cfe-commits
mailing list