Author: Baranov Victor Date: 2026-05-10T13:54:30+03:00 New Revision: c522ad0a41c73ef2a2bdbae401a9c480c697548e
URL: https://github.com/llvm/llvm-project/commit/c522ad0a41c73ef2a2bdbae401a9c480c697548e DIFF: https://github.com/llvm/llvm-project/commit/c522ad0a41c73ef2a2bdbae401a9c480c697548e.diff LOG: [clang-tidy] Fix FP in readability-container-size-empty with compairing to unrelated type (#190535) Fixes https://github.com/llvm/llvm-project/issues/162287. Added: Modified: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp clang-tools-extra/docs/ReleaseNotes.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 2101fd2248e8a..cbad3d244d841 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -144,15 +144,18 @@ void ContainerSizeEmptyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { } void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { - const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom(namedDecl( - has(cxxMethodDecl( - isConst(), parameterCountIs(0), isPublic(), - hasAnyName("size", "length"), - returns(qualType(isIntegralType(), unless(booleanType())))) - .bind("size")), - has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(), - hasName("empty"), returns(booleanType())) - .bind("empty"))))); + const auto ValidContainerRecord = + cxxRecordDecl( + isSameOrDerivedFrom(namedDecl( + has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(), + hasAnyName("size", "length"), + returns(qualType(isIntegralType(), + unless(booleanType())))) + .bind("size")), + has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(), + hasName("empty"), returns(booleanType())) + .bind("empty"))))) + .bind("ContainerDecl"); const auto ValidContainerNonTemplateType = qualType(hasUnqualifiedDesugaredType( @@ -235,6 +238,13 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( binaryOperation( hasAnyOperatorName("==", "!="), hasOperands(WrongComparend, STLArg), + unless(hasEitherOperand(cxxConstructExpr( + argumentCountIs(0), + unless(hasType(qualType(hasCanonicalType(hasDeclaration( + // 'equalsBoundNode' needs the 'ContainerDecl' binding + // from 'STLArg' to already exist, so this constraint must + // appear after 'hasOperands' matcher + namedDecl(equalsBoundNode("ContainerDecl")))))))))), unless(allOf(hasLHS(hasType(ExcludedComparisonTypesMatcher)), hasRHS(hasType(SameExcludedComparisonTypesMatcher)))), NotInEmptyMethodOfContainer) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index be5315ac3c181..0d3adedeea0f8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -541,6 +541,9 @@ Changes in existing checks - Reduce verbosity by removing the note indicating source location of the ``empty`` function. + - Fixed a false positive with suggesting ``empty`` when comparing a container + to a default-constructed object of an unrelated type. + - Improved :doc:`readability-convert-member-functions-to-static <clang-tidy/checks/readability/convert-member-functions-to-static>` check: 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 2b8b3261ac765..cd2eebb16138b 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 @@ -954,3 +954,77 @@ struct DestructorUser { } }; } + +namespace GH162287 { + +struct Label { + virtual ~Label(); +}; +bool operator==(std::string, const Label&); +bool operator==(std::string, std::vector<char>); +bool operator==(const Label&, std::string); + +void testUnrelatedType() { + std::string s{"aa"}; + if (s == Label{}) + ; + if (s == Label()) + ; + if (s == std::vector<char>{}) + ; + if (Label() == s) + ; +} + +void testValid() { + std::string s{"aa"}; + std::vector<int> v; + Container c; + + // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: if (s.empty()) + if (s == std::string{}) + ; + // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: if (s.empty()) + if (std::string() == s) + ; + // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: if (c.empty()) + if (c == Container()) + ; + Container *p = nullptr; + // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: if (p->empty()) + if (*p == Container()) + ; + using MyString = std::string; + MyString ms{"aa"}; + // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: if (ms.empty()) + if (ms == std::string()) + ; + bool b1 = s == Label(); + // CHECK-MESSAGES: :[[@LINE+2]]:13: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: bool b2 = c.empty(); + bool b2 = c == Container(); + // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: if (v.empty()) + if (v == std::vector<int>()) + ; +} + +template <typename T> +bool testUnrelatedInTemplate(std::string s) { + return s == Label{}; +} +template bool testUnrelatedInTemplate<int>(std::string); + +template <typename T> +bool testDependentValidContainer(TemplatedContainer<T> c) { + return c == TemplatedContainer<T>(); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object + // CHECK-FIXES: return c.empty(); +} +template bool testDependentValidContainer<int>(TemplatedContainer<int>); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
