diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTYCHECK_H #include "../ClangTidyCheck.h" +#include namespace clang::tidy::readability { @@ -31,9 +32,13 @@ } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; std::optional getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } + +private: + std::string IgnoreComparisonForTypesRegexp; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -96,7 +96,14 @@ ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + IgnoreComparisonForTypesRegexp( + Options.get("IgnoreComparisonForTypesRegexp", "^::std::array")) {} + +void ContainerSizeEmptyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreComparisonForTypesRegexp", + IgnoreComparisonForTypesRegexp); +} void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom( @@ -164,12 +171,25 @@ hasUnaryOperand( expr(hasType(pointsTo(ValidContainer))).bind("Pointee"))), expr(hasType(ValidContainer)).bind("STLObject")); + + const auto StdArrayType = qualType(anyOf( + hasDeclaration(cxxRecordDecl(matchesName(IgnoreComparisonForTypesRegexp)) + .bind("array")), + hasCanonicalType(hasDeclaration( + cxxRecordDecl(matchesName(IgnoreComparisonForTypesRegexp)) + .bind("array"))))); + const auto SameStdArrayType = + qualType(anyOf(hasDeclaration(cxxRecordDecl(equalsBoundNode("array"))), + hasCanonicalType(hasDeclaration( + cxxRecordDecl(equalsBoundNode("array")))))); + Finder->addMatcher( binaryOperation(hasAnyOperatorName("==", "!="), - hasOperands(WrongComparend, - STLArg), - unless(hasAncestor(cxxMethodDecl( - ofClass(equalsBoundNode("container")))))) + hasOperands(WrongComparend, STLArg), + unless(allOf(hasLHS(hasType(StdArrayType)), + hasRHS(hasType(SameStdArrayType)))), + unless(hasAncestor(cxxMethodDecl( + ofClass(equalsBoundNode("container")))))) .bind("BinCmp"), this); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -130,6 +130,11 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Fixed a false positive in :doc:`readability-container-size-empty + ` check when comparing + ``std::array`` objects to default constructed ones. The behavior for this and + other relevant classes can now be configured with a new option. + Removed checks ^^^^^^^^^^^^^^ 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 --- 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 @@ -24,3 +24,11 @@ bool empty() const; `size_type` can be any kind of integer type. + +.. option:: IgnoreComparisonForTypesRegexp + + Excludes certain classes from the check using a regular expression of ERE + syntax. If excluded, the check won't suggest replacing the comparison of + an object with a default constructed object of the same type using + ``empty()``. This option is useful if the check incorrectly flags such + comparisons for certain classes. Default value is `^::std::array`. 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 --- 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 @@ -724,10 +724,37 @@ size_type size() const; bool empty() const; }; -void test() { + +void testTypedefSize() { TypedefSize ts; if (ts.size() == 0) ; // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used // CHECK-FIXES: {{^ }}if (ts.empty()){{$}} } + +namespace std { + +template struct array { + bool operator==(const array& other) const; + bool operator!=(const array& other) const; + unsigned long size() const { return N; } + bool empty() const { return N != 0U; } + + T data[N]; +}; + +} + +typedef std::array Array; + +bool testArraySize(const Array& value) { + return value.size() == 0U; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty] +// CHECK-FIXES: {{^ }}return value.empty();{{$}} +// CHECK-MESSAGES: :742:8: note: method 'array'::empty() defined here +} + +bool testArrayCompareToEmptye(const Array& value) { + return value == std::array(); +}