[clang-tidy] Fix false-positive in readability-container-size-empty

[PiotrZSL]

Summary

Ignoring std::array type when matching 'std:array == std::array()'.
In such case we shouldn't propose to use empty().

Fixes: https://github.com/llvm/llvm-project/issues/48286

[PiotrZSL]

I may consider tomorrow moving std::array into into some dedicated option, like IgnoredContainersInComparisonRegexp

[PiotrZSL]

Add configuration option. Release Notes + documentation for new option powered by ChatGPT.

[PiotrZSL]

Ready for review

[carlosgalvezp]

Looks good in general, thanks for the fix! I have minor comments.

Also, I found the comment on the Github issue interesting:

Perhaps, this can even be generalized to all types whose size() and empty() are constexpr.

Would that be a more scalable approach than maintaining a list of classes to ignore?

clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst

30

For better readability, please spell out this acronym.

clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

723

Missing a test case to test that the option works as expected, e.g. set it to a non-default value.

760

Empty

[PiotrZSL]

In D144217#4143540, @carlosgalvezp wrote:

Perhaps, this can even be generalized to all types whose size() and empty() are constexpr.

Problem is that you can mark function constexpr, but it doesnt need to be constexpr evaluated:

struct C
{
    constexpr C(int v) : v(v) {}
    constexpr int size() const { return v; };
    constexpr bool empty() const { return v == 0; };
    constexpr void setSize(int vv) { v = vv; };
    int v;
};

constexpr C value(6);
C non_const(6);

I would need to check if it's constexpr evaluated, but here we don't evaluate it. Alternative would be to check if method use only template arguments, but then it could use other const static or something...
This is why I decided to go with config. And still you may have other user provided classes that does not have empty/size constexpr.

[carlosgalvezp]

In D144217#4143554, @PiotrZSL wrote:

In D144217#4143540, @carlosgalvezp wrote:

Perhaps, this can even be generalized to all types whose size() and empty() are constexpr.

Problem is that you can mark function constexpr, but it doesnt need to be constexpr evaluated:

struct C
{
    constexpr C(int v) : v(v) {}
    constexpr int size() const { return v; };
    constexpr bool empty() const { return v == 0; };
    constexpr void setSize(int vv) { v = vv; };
    int v;
};

constexpr C value(6);
C non_const(6);

I would need to check if it's constexpr evaluated, but here we don't evaluate it. Alternative would be to check if method use only template arguments, but then it could use other const static or something...
This is why I decided to go with config. And still you may have other user provided classes that does not have empty/size constexpr.

Ok, good point!

clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp

106

I also realize that in other checks, this is typically implemented as a list (comma or semicolon-separated) instead of a regex. Would it make sense to do that here as well, for consistency? As a user I would also find it easier and more intuitive to type a list than to have to google how to create a list using regex :)

[PiotrZSL]

clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp

106

I usually prefer regexp, as it allow more flexibility (like matching templates), but I can change this to use semicolon separated list, in this case it would also do a job.

[PiotrZSL]

Review fixes + Rebase

[PiotrZSL]

Removed include

[PiotrZSL]

Rebase + Used existing matcher instead of creating new one

[carlosgalvezp]

Thanks for the fix! Looks good, have a couple minor comments.

clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp

88–89

While this improves readability, we typically prefer these changes to be done in a separate NFC patch to reduce review noise (and thereby improving review speed).

180

This is no longer just StdArray, it can be any excluded type - please rename accordingly.

clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst

34

Only std::array is currently part of the default.

clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

658

What was the motivation for this change? Similarly to my previous comment this seems unrelated to the main scope of the patch.

[PiotrZSL]

Fix review comments

clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h

 //===--- ContainerSizeEmptyCheck.h - clang-tidy -----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTYCHECK_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTYCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include <vector>
 
 namespace clang::tidy::readability {
 
 /// Checks whether a call to the `size()` method can be replaced with a call to
 /// `empty()`.
 ///
 /// The emptiness of a container should be checked using the `empty()` method
 /// instead of the `size()` method. It is not guaranteed that `size()` is a
 /// constant-time function, and it is generally more efficient and also shows
 /// clearer intent to use `empty()`. Furthermore some containers may implement
 /// the `empty()` method but not implement the `size()` method. Using `empty()`
 /// whenever possible makes it easier to switch to another container in the
 /// future.
 class ContainerSizeEmptyCheck : public ClangTidyCheck {
 public:
   ContainerSizeEmptyCheck(StringRef Name, ClangTidyContext *Context);
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus;
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   std::optional<TraversalKind> getCheckTraversalKind() const override {
     return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  std::vector<llvm::StringRef> ExcludedComparisonTypes;
 };
 
 } // namespace clang::tidy::readability
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTYCHECK_H

clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp

 //===--- ContainerSizeEmptyCheck.cpp - clang-tidy -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "ContainerSizeEmptyCheck.h"
 #include "../utils/ASTUtils.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace ast_matchers {
+
 AST_POLYMORPHIC_MATCHER_P2(hasAnyArgumentWithParam,
                            AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
                                                            CXXConstructExpr),
                            internal::Matcher<Expr>, ArgMatcher,
                            internal::Matcher<ParmVarDecl>, ParamMatcher) {
   BoundNodesTreeBuilder Result;
   // The first argument of an overloaded member operator is the implicit object
   // argument of the method which should not be matched against a parameter, so
auto Result =
                      binaryOperator(hasAnyOperatorName("&&", "||")),
                      unaryOperator(hasOperatorName("!")).bind("NegOnSize"))))))
           .matches(Node, Finder, Builder);
   Builder->removeBindings([ExprName](const BoundNodesMap &Nodes) {
     return Nodes.getNode(ExprName).getNodeKind().isNone();
   });
   return Result;
 }
+
 AST_MATCHER(CXXConstructExpr, isDefaultConstruction) {
   return Node.getConstructor()->isDefaultConstructor();
 }
+
 AST_MATCHER(QualType, isIntegralType) {
   return Node->isIntegralType(Finder->getASTContext());
 }
+
 } // namespace ast_matchers
 namespace tidy::readability {
 
 using utils::isBinaryOrTernary;
 
 ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name,
                                                  ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context) {}
+    : ClangTidyCheck(Name, Context),
+      ExcludedComparisonTypes(utils::options::parseStringList(
+          Options.get("ExcludedComparisonTypes", "::std::array"))) {}
+
+void ContainerSizeEmptyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ExcludedComparisonTypes",
+                utils::options::serializeStringList(ExcludedComparisonTypes));
+}
 
 void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
   const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom(
       namedDecl(
           has(cxxMethodDecl(
                   isConst(), parameterCountIs(0), isPublic(), hasName("size"),
                   returns(qualType(isIntegralType(), unless(booleanType()))))
                   .bind("size")),
const auto WrongComparend = anyOf(
       cxxUnresolvedConstructExpr(argumentCountIs(0)));
   // Match the object being compared.
   const auto STLArg =
       anyOf(unaryOperator(
                 hasOperatorName("*"),
                 hasUnaryOperand(
                     expr(hasType(pointsTo(ValidContainer))).bind("Pointee"))),
             expr(hasType(ValidContainer)).bind("STLObject"));
+
+  const auto ExcludedComparisonTypesMatcher = qualType(anyOf(
+      hasDeclaration(
+          cxxRecordDecl(matchers::matchesAnyListedName(ExcludedComparisonTypes))
+              .bind("excluded")),
+      hasCanonicalType(hasDeclaration(
+          cxxRecordDecl(matchers::matchesAnyListedName(ExcludedComparisonTypes))
+              .bind("excluded")))));
+  const auto SameExcludedComparisonTypesMatcher =
+      qualType(anyOf(hasDeclaration(cxxRecordDecl(equalsBoundNode("excluded"))),
+                     hasCanonicalType(hasDeclaration(
+                         cxxRecordDecl(equalsBoundNode("excluded"))))));
+
   Finder->addMatcher(
-      binaryOperation(hasAnyOperatorName("==", "!="),
-                      hasOperands(WrongComparend,
-                                  STLArg),
-                          unless(hasAncestor(cxxMethodDecl(
-                              ofClass(equalsBoundNode("container"))))))
+      binaryOperation(
+          hasAnyOperatorName("==", "!="), hasOperands(WrongComparend, STLArg),
+          unless(allOf(hasLHS(hasType(ExcludedComparisonTypesMatcher)),
+                       hasRHS(hasType(SameExcludedComparisonTypesMatcher)))),
+          unless(hasAncestor(
+              cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
           .bind("BinCmp"),
       this);
 }
 
 void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MemberCall = Result.Nodes.getNodeAs<Expr>("SizeCallExpr");
   const auto *MemberCallObject =
       Result.Nodes.getNodeAs<Expr>("MemberCallObject");

clang-tools-extra/docs/ReleaseNotes.rst

- Deprecated check-local options `HeaderFileExtensions`
   in :doc:`misc-unused-using-decls
   <clang-tidy/checks/misc/unused-using-decls>` check.
   Global options of the same name should be used instead.
 
 - Fixed reading `HungarianNotation.CString.*` options in
   :doc:`readability-identifier-naming
   <clang-tidy/checks/readability/identifier-naming>` check.
 
+- Fixed a false positive in :doc:`readability-container-size-empty
+  <clang-tidy/checks/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
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst

 matching following signatures:
 
 .. code-block:: c++
 
   size_type size() const;
   bool empty() const;
 
 `size_type` can be any kind of integer type.
+
+.. option:: ExcludedComparisonTypes
+
+    A semicolon-separated list of class names for which the check will ignore
+    comparisons of objects with default-constructed 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 value is: `::std::array`.

clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

-// RUN: %check_clang_tidy %s readability-container-size-empty %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
+// RUN: -config="{CheckOptions: [{key: readability-container-size-empty.ExcludedComparisonTypes , value: '::std::array;::IgnoredDummyType'}]}" \
+// RUN: -- -fno-delayed-template-parsing
 
 namespace std {
 template <typename T> struct vector {
   vector();
   bool operator==(const vector<T>& other) const;
   bool operator!=(const vector<T>& other) const;
   unsigned long size() const;
   bool empty() const;
bool returnsBool() {
   (void)(str.size() + 0);
   (void)(str.size() - 0);
   (void)(0 + str.size());
   (void)(0 - str.size());
   if (intSet.size() == 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
   if (intSet == std::set<int>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
   if (s_func() == "")
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (s_func().empty()){{$}}
   if (str.size() == 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
bool returnsBool() {
   // CHECK-FIXES: {{^  }}return !derived.empty();
 }
 
 class ConstructWithBoolField {
   bool B;
 public:
   ConstructWithBoolField(const std::vector<int> &C) : B(C.size()) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:57: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}ConstructWithBoolField(const std::vector<int> &C) : B(!C.empty()) {}
 };
 
 struct StructWithNSDMI {
   std::vector<int> C;
   bool B = C.size();
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}bool B = !C.empty();
 };
 
 int func(const std::vector<int> &C) {
   return C.size() ? 0 : 1;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}return !C.empty() ? 0 : 1;
 }
 
 constexpr Lazy L;
 static_assert(!L.size(), "");
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :101:18: note: method 'Lazy'::empty() defined here
+// CHECK-MESSAGES: :103:18: note: method 'Lazy'::empty() defined here
 // CHECK-FIXES: {{^}}static_assert(L.empty(), "");
 
 struct StructWithLazyNoexcept {
   void func() noexcept(L.size());
 };
 
 #define CHECKSIZE(x) if (x.size()) {}
 // CHECK-FIXES: #define CHECKSIZE(x) if (x.size()) {}
 
 template <typename T> void f() {
   std::vector<T> v;
   if (v.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v == std::vector<T>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(v);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(v);
 
   TemplatedContainer<T> templated_container;
   if (templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer<T>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(templated_container);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(templated_container);
 }
 
 void g() {
   f<int>();
   f<double>();
   f<char *>();
 }
 
 template <typename T>
 bool neverInstantiatedTemplate() {
   std::vector<T> v;
   if (v.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
 
   if (v == std::vector<T>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   if (v.size() == 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
   if (v.size() != 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v.size() < 1)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
   if (v.size() > 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v.size() == 1)
     ;
   if (v.size() != 1)
     ;
   if (v.size() == 2)
     ;
   if (v.size() != 2)
     ;
 
   if (static_cast<bool>(v.size()))
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (static_cast<bool>(!v.empty())){{$}}
   if (v.size() && false)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty() && false){{$}}
   if (!v.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
 
   TemplatedContainer<T> templated_container;
   if (templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer<T>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   while (templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty()){{$}}
 
   do {
   }
   while (templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty());
 
   for (; templated_container.size();)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}for (; !templated_container.empty();){{$}}
 
   if (true && templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (true && !templated_container.empty()){{$}}
 
   if (true || templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (true || !templated_container.empty()){{$}}
 
   if (!templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
 
   bool b1 = templated_container.size();
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}bool b1 = !templated_container.empty();
 
   bool b2(templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}bool b2(!templated_container.empty());
 
   auto b3 = static_cast<bool>(templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}auto b3 = static_cast<bool>(!templated_container.empty());
 
   auto b4 = (bool)templated_container.size();
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}auto b4 = (bool)!templated_container.empty();
 
   auto b5 = bool(templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}auto b5 = bool(!templated_container.empty());
 
   takesBool(templated_container.size());
   // We don't detect this one because we don't know the parameter of takesBool
   // until the type of templated_container.size() is known
 
   return templated_container.size();
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}return !templated_container.empty();
 }
 
 template <typename TypeRequiresSize>
 void instantiatedTemplateWithSizeCall() {
   TypeRequiresSize t;
   // The instantiation of the template with std::vector<int> should not
   // result in changing the template, because we don't know that
struct unique_ptr {
   T *operator->() const;
   T &operator*() const;
 };
 } // namespace std
 
 bool call_through_unique_ptr(const std::unique_ptr<std::vector<int>> &ptr) {
   return ptr->size() > 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}return !ptr->empty();
 }
 
 bool call_through_unique_ptr_deref(const std::unique_ptr<std::vector<int>> &ptr) {
   return (*ptr).size() > 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}return !(*ptr).empty();
 }
 
 struct TypedefSize {
   typedef int size_type;
   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 <typename T, unsigned long N> 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];
+};
+
+}
+
+struct DummyType {
+  bool operator==(const DummyType&) const;
+  unsigned long size() const;
+  bool empty() const;
+};
+
+struct IgnoredDummyType {
+  bool operator==(const IgnoredDummyType&) const;
+  unsigned long size() const;
+  bool empty() const;
+};
+
+typedef std::array<int, 10U> 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: :744:8: note: method 'array'::empty() defined here
+}
+
+bool testArrayCompareToEmpty(const Array& value) {
+  return value == std::array<int, 10U>();
+}
+
+bool testDummyType(const DummyType& value) {
+  return value == DummyType();
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object [readability-container-size-empty]
+// CHECK-FIXES: {{^  }}return value.empty();{{$}}
+// CHECK-MESSAGES: :754:8: note: method 'DummyType'::empty() defined here
+}
+
+bool testIgnoredDummyType(const IgnoredDummyType& value) {
+  return value == IgnoredDummyType();
+}