Author: Aayush Shrivastava Date: 2026-06-03T21:56:24+03:00 New Revision: 517308ad15ee9480edfb9aa6dcc2c628c43da20d
URL: https://github.com/llvm/llvm-project/commit/517308ad15ee9480edfb9aa6dcc2c628c43da20d DIFF: https://github.com/llvm/llvm-project/commit/517308ad15ee9480edfb9aa6dcc2c628c43da20d.diff LOG: [clang-tidy] Extend readability-container-size-empty to std::size() (#201231) Fixes #198494 Extend the check to warn when the non-member `std::size()` free function is used in a boolean context or compared to 0/1, and suggest using .empty instead. Added: Modified: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp index cbad3d244d841..33fe48048b356 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -179,13 +179,14 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { const auto NotInEmptyMethodOfContainer = unless( forCallable(cxxMethodDecl(hasCanonicalDecl(equalsBoundNode("empty"))))); + const auto ValidContainerExpr = + expr(anyOf(hasType(ValidContainer), hasType(pointsTo(ValidContainer)), + hasType(references(ValidContainer)))) + .bind("MemberCallObject"); + Finder->addMatcher( cxxMemberCallExpr( - argumentCountIs(0), - on(expr(anyOf(hasType(ValidContainer), - hasType(pointsTo(ValidContainer)), - hasType(references(ValidContainer)))) - .bind("MemberCallObject")), + argumentCountIs(0), on(ValidContainerExpr), callee( cxxMethodDecl(hasAnyName("size", "length")).bind("SizeMethod")), WrongUse, NotInEmptyMethodOfContainer) @@ -193,18 +194,23 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { this); Finder->addMatcher( - callExpr( - argumentCountIs(0), - has(mapAnyOf(memberExpr, cxxDependentScopeMemberExpr) - .with( - hasObjectExpression( - expr(anyOf(hasType(ValidContainer), - hasType(pointsTo(ValidContainer)), - hasType(references(ValidContainer)))) - .bind("MemberCallObject")), - anyOf(matchMemberName("size"), matchMemberName("length"))) - .bind("MemberExpr")), - WrongUse, NotInEmptyMethodOfContainer) + callExpr(argumentCountIs(0), + has(mapAnyOf(memberExpr, cxxDependentScopeMemberExpr) + .with(hasObjectExpression(ValidContainerExpr), + anyOf(matchMemberName("size"), + matchMemberName("length"))) + .bind("MemberExpr")), + WrongUse, NotInEmptyMethodOfContainer) + .bind("SizeCallExpr"), + this); + + // Match non-member std::size(container) used in boolean context or compared + // with 0/1. + Finder->addMatcher( + callExpr(argumentCountIs(1), + callee(functionDecl(hasName("::std::size")).bind("SizeMethod")), + hasArgument(0, ValidContainerExpr), WrongUse, + NotInEmptyMethodOfContainer) .bind("SizeCallExpr"), this); @@ -406,7 +412,7 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) { "for emptiness instead of %0"); if (const auto *SizeMethod = Result.Nodes.getNodeAs<NamedDecl>("SizeMethod")) - Diag << SizeMethod; + Diag << SizeMethod->getDeclName(); else if (const auto *DependentExpr = Result.Nodes.getNodeAs<CXXDependentScopeMemberExpr>( "MemberExpr")) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index fa53072be1eac..0fb29f1b1ff22 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -683,6 +683,10 @@ Changes in existing checks - Fixed a false positive with suggesting ``empty`` when comparing a container to a default-constructed object of an unrelated type. + - Extended to warn when the non-member ``std::size()`` function is used + in a Boolean context or compared to ``0`` or ``1``, consistent with the + existing ``size()``/``length()`` member call detection. + - Improved :doc:`readability-convert-member-functions-to-static <clang-tidy/checks/readability/convert-member-functions-to-static>` check: diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst index cc012fdcd7649..70b05f498d804 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst @@ -4,15 +4,15 @@ readability-container-size-empty ================================ -Checks whether a call to the ``size()``/``length()`` method can be replaced -with a call to ``empty()``. +Checks whether a call to the ``size()``/``length()`` method or the +``std::size()`` free function can be replaced with a call to ``empty()``. The emptiness of a container should be checked using the ``empty()`` method -instead of the ``size()``/``length()`` method. It shows clearer intent to use -``empty()``. Furthermore some containers (for example, a ``std::forward_list``) -may implement the ``empty()`` method but not implement the ``size()`` or -``length()`` method. Using ``empty()`` whenever possible makes it easier to -switch to another container in the future. +instead of the ``size()``/``length()`` method or ``std::size()``. It shows +clearer intent to use ``empty()``. Furthermore some containers (for example, a +``std::forward_list``) may implement the ``empty()`` method but not implement +the ``size()`` or ``length()`` method. Using ``empty()`` whenever possible +makes it easier to switch to another container in the future. The check issues warning if a container has ``empty()`` and ``size()`` or ``length()`` methods matching following signatures: 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 cd2eebb16138b..79c42c213002f 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 @@ -27,6 +27,12 @@ namespace string_literals{ string operator""s(const char *, size_t); } +template <class C> +auto size(const C& c) -> decltype(c.size()); + +template <class C> +auto empty(const C& c) -> decltype(c.empty()); + } template <typename T> @@ -924,6 +930,62 @@ class ReportInTemplateContainerNonEmptyMethod { } }; +void testStdSize(const std::string &str, std::vector<int> vect) { + if (std::size(vect)) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (!vect.empty()) + + if (!std::size(vect)) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (vect.empty()) + + if (std::size(vect) == 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (vect.empty()) + + if (std::size(vect) != 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (!vect.empty()) + + if (std::size(vect) > 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (!vect.empty()) + + if (std::size(str)) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (!str.empty()) + + if (std::size(str) == 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (str.empty()) + + // No warning when std::empty is already used. + if (std::empty(vect)) {} + + // using declaration resolves to ::std::size, should also warn. + using std::size; + if (size(str) == 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (str.empty()) +} + +#define SIZE std::size +void testStdSizeMacro(const std::string &str) { + if (SIZE(str) == 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' + // CHECK-FIXES: if (str.empty()) +} +#undef SIZE + class ReportInContainerNonEmptyMethodCompare { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
