Handle template instantiations better in clang-tidy check

[steveire]

Summary

readability-container-size-empty currently modifies source code based on
AST nodes in template instantiations, which means that it makes
transformations based on substituted types. This can lead to
transforming code to be broken.

Change the matcher implementation to ignore template instantiations
explicitly, and add a matcher to explicitly handle template declatations
instead of instantiations.

[alexfh]

Please fix the typo that results in a compile error.

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

175

This doesn't compile:

llvm-project/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:107:34: error: use of undeclared identifier 'argummentCountIs'; did you mean 'argumentCountIs'?
      cxxUnresolvedConstructExpr(argummentCountIs(0)));
                                 ^~~~~~~~~~~~~~~~
                                 argumentCountIs
/mnt/disks/ssd0/agent/llvm-project/clang/include/clang/ASTMatchers/ASTMatchers.h:4042:27: note: 'argumentCountIs' declared here
AST_POLYMORPHIC_MATCHER_P(argumentCountIs,
                          ^
1 error generated.

https://reviews.llvm.org/harbormaster/build/104600/

[steveire]

Update

[steveire]

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

20

At some point we could could consider adding both of these matchers to ASTMatchers.h. Especially if they are needed in other places at I make more checks handle templates.

[steveire]

Update

[aaron.ballman]

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

26–28

Isn't this true for any non-static member function? e.g., should the matcher be looking at cxxOperatorCallExpr() at all?

61

I take it we can't do hasParent(anyOf(...)) instead?

67

If this matters for var decls, what about field decls in a structure?

69

Same question here.

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

493

The quoting here is unfortunate. It's not part of your patch, but it could be solved by calling Container->getName() and manually quoting the note text if you wanted to hit it in a follow-up (I'd consider that an NFC change, no need to review). You could also address it as part of this patch if you felt like it.

631

Some other test cases that may be of interest:

class C {
  bool B;
public:
  C(const std::vector<int> &C) : B(C.size()) {}
};

struct S {
  std::vector<int> C;
  bool B = C.size();
};

int func(const std::vector<int> &C) {
  return C.size() ? 0 : 1;
}

struct Lazy {
  constexpr unsigned size() const { return 0; }
  constexpr bool empty() const { return true; }
};

constexpr Lazy L;
static_assert(!L.size(), ""); // This one should get converted

struct S {
  void func() noexcept(L.size()); // This one should not
};

[steveire]

Update

[steveire]

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

26–28

I think it's deliberate - https://godbolt.org/z/EYYndv - it's just that we don't consider the aa an argument because the op is a member func.

67

Not sure. What would this look like in a test?

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

493

That will be a follow-up.

[aaron.ballman]

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

56

57

67

I was thinking this one:

struct S {
  std::vector<int> C;
  bool B = C.size();
};

but that seems to be handled properly already.

84

Might as well drop the this from here.

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

493

SGTM, thanks!

[steveire]

Update

[steveire]

Update

[steveire]

Update

[steveire]

Update

[aaron.ballman]

LGTM!

[thakis]

Looks like this breaks tests on windows: http://45.33.8.238/win/30322/step_8.txt

Ptal, and revert for now if it takes a while to fix. (In particular, prefer reverting over disabling the test on win.)

[steveire]

In D91302#2468665, @thakis wrote:

Looks like this breaks tests on windows: http://45.33.8.238/win/30322/step_8.txt

Ptal, and revert for now if it takes a while to fix. (In particular, prefer reverting over disabling the test on win.)

Just so I know - how am I supposed to monitor build health after a push? I usually monitor http://lab.llvm.org:8011/#/changes/7087 but it only shows success. How should this fail on Windows be coming on to my radar?

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

 #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
+  // we skip over it here.
+  BoundNodesTreeBuilder Matches;
+  unsigned ArgIndex = cxxOperatorCallExpr(callee(cxxMethodDecl()))
+                              .matches(Node, Finder, &Matches)
+                          ? 1
+                          : 0;
+  int ParamIndex = 0;
+  for (; ArgIndex < Node.getNumArgs(); ++ArgIndex) {
+    BoundNodesTreeBuilder ArgMatches(*Builder);
+    if (ArgMatcher.matches(*(Node.getArg(ArgIndex)->IgnoreParenCasts()), Finder,
+                           &ArgMatches)) {
+      BoundNodesTreeBuilder ParamMatches(ArgMatches);
+      if (expr(anyOf(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
+                         hasParameter(ParamIndex, ParamMatcher)))),
+                     callExpr(callee(functionDecl(
+                         hasParameter(ParamIndex, ParamMatcher))))))
+              .matches(Node, Finder, &ParamMatches)) {
+        Result.addMatch(ParamMatches);
+        *Builder = std::move(Result);
+        return true;
+      }
+    }
+    ++ParamIndex;
+  }
+  return false;
+}
+
+AST_MATCHER(Expr, usedInBooleanContext) {
+  const char *ExprName = "__booleanContextExpr";
+  auto Result =
+      expr(expr().bind(ExprName),
+           anyOf(hasParent(varDecl(hasType(booleanType()))),
+                 hasParent(cxxConstructorDecl(
+                     hasAnyConstructorInitializer(cxxCtorInitializer(
+                         withInitializer(expr(equalsBoundNode(ExprName))),
+                         forField(hasType(booleanType())))))),
+                 hasParent(fieldDecl(hasType(booleanType()))),
+                 hasParent(stmt(anyOf(
+                     explicitCastExpr(hasDestinationType(booleanType())),
+                     ifStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     doStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     whileStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     forStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     conditionalOperator(
+                         hasCondition(expr(equalsBoundNode(ExprName)))),
+                     parenListExpr(hasParent(varDecl(hasType(booleanType())))),
+                     parenExpr(hasParent(
+                         explicitCastExpr(hasDestinationType(booleanType())))),
+                     returnStmt(forFunction(returns(booleanType()))),
+                     cxxUnresolvedConstructExpr(hasType(booleanType())),
+                     callExpr(hasAnyArgumentWithParam(
+                         expr(equalsBoundNode(ExprName)),
+                         parmVarDecl(hasType(booleanType())))),
+                     binaryOperator(hasAnyOperatorName("&&", "||")),
+                     unaryOperator(hasOperatorName("!")).bind("NegOnSize"))))))
+          .matches(Node, Finder, Builder);
+  Builder->removeBindings([ExprName](const BoundNodesMap &Nodes) {
+    return Nodes.getNode(ExprName).getNodeKind().isNone();
+  });
+  return Result;
+}
+} // namespace ast_matchers
 namespace tidy {
 namespace readability {
 
 using utils::IsBinaryOrTernary;
 
 ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name,
                                                  ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context) {}
 
 void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ValidContainer = qualType(hasUnqualifiedDesugaredType(
-      recordType(hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
-          namedDecl(
-              has(cxxMethodDecl(
-                      isConst(), parameterCountIs(0), isPublic(),
+  const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom(
+          has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
                             hasName("size"),
-                      returns(qualType(isInteger(), unless(booleanType()))))
-                      .bind("size")),
-              has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
-                                hasName("empty"), returns(booleanType()))
-                      .bind("empty")))
-              .bind("container")))))));
+                            returns(qualType(isInteger(), unless(booleanType()),
+                                             unless(elaboratedType()))))
+          .bind("container")));
+
+  const auto ValidContainerNonTemplateType =
+      qualType(hasUnqualifiedDesugaredType(
+          recordType(hasDeclaration(ValidContainerRecord))));
+  const auto ValidContainerTemplateType =
+      qualType(hasUnqualifiedDesugaredType(templateSpecializationType(
+          hasDeclaration(classTemplateDecl(has(ValidContainerRecord))))));
+
+  const auto ValidContainer = qualType(
+      anyOf(ValidContainerNonTemplateType, ValidContainerTemplateType));
 
   const auto WrongUse = traverse(
       TK_AsIs,
       anyOf(
           hasParent(binaryOperator(isComparisonOperator(),
                                    hasEitherOperand(ignoringImpCasts(
                                        anyOf(integerLiteral(equals(1)),
                                              integerLiteral(equals(0))))))
                         .bind("SizeBinaryOp")),
           hasParent(implicitCastExpr(
               hasImplicitDestinationType(booleanType()),
               anyOf(hasParent(
                         unaryOperator(hasOperatorName("!")).bind("NegOnSize")),
                     anything()))),
-          hasParent(explicitCastExpr(hasDestinationType(booleanType())))));
+          usedInBooleanContext()));
 
   Finder->addMatcher(
-      cxxMemberCallExpr(on(expr(anyOf(hasType(ValidContainer),
+      cxxMemberCallExpr(unless(isInTemplateInstantiation()),
+                        on(expr(anyOf(hasType(ValidContainer),
                                       hasType(pointsTo(ValidContainer)),
-                                      hasType(references(ValidContainer))))),
+                                      hasType(references(ValidContainer))))
+                               .bind("MemberCallObject")),
                         callee(cxxMethodDecl(hasName("size"))), WrongUse,
                         unless(hasAncestor(cxxMethodDecl(
                             ofClass(equalsBoundNode("container"))))))
           .bind("SizeCallExpr"),
       this);
 
+  Finder->addMatcher(
+      callExpr(has(cxxDependentScopeMemberExpr(
+                   hasObjectExpression(
+                       expr(anyOf(hasType(ValidContainer),
+                                  hasType(pointsTo(ValidContainer)),
+                                  hasType(references(ValidContainer))))
+                           .bind("MemberCallObject")),
+                   hasMemberName("size"))),
+               WrongUse,
+               unless(hasAncestor(
+                   cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
+          .bind("SizeCallExpr"),
+      this);
+
   // Empty constructor matcher.
   const auto DefaultConstructor = cxxConstructExpr(
           hasDeclaration(cxxConstructorDecl(isDefaultConstructor())));
   // Comparison to empty string or empty constructor.
   const auto WrongComparend = anyOf(
       ignoringImpCasts(stringLiteral(hasSize(0))),
       ignoringImpCasts(cxxBindTemporaryExpr(has(DefaultConstructor))),
       ignoringImplicit(DefaultConstructor),
-      cxxConstructExpr(
-          hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
+      cxxConstructExpr(hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
+                       has(expr(ignoringImpCasts(DefaultConstructor)))),
+      cxxConstructExpr(hasDeclaration(cxxConstructorDecl(isMoveConstructor())),
                        has(expr(ignoringImpCasts(DefaultConstructor)))),
-      cxxConstructExpr(
-          hasDeclaration(cxxConstructorDecl(isMoveConstructor())),
-          has(expr(ignoringImpCasts(DefaultConstructor)))));
+      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"));
   Finder->addMatcher(
       cxxOperatorCallExpr(
+          unless(isInTemplateInstantiation()),
           hasAnyOverloadedOperatorName("==", "!="),
           anyOf(allOf(hasArgument(0, WrongComparend), hasArgument(1, STLArg)),
                 allOf(hasArgument(0, STLArg), hasArgument(1, WrongComparend))),
           unless(hasAncestor(
               cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
           .bind("BinCmp"),
       this);
+  Finder->addMatcher(
+      binaryOperator(hasAnyOperatorName("==", "!="),
+                     anyOf(allOf(hasLHS(WrongComparend), hasRHS(STLArg)),
+                           allOf(hasLHS(STLArg), hasRHS(WrongComparend))),
+                     unless(hasAncestor(
+                         cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
+          .bind("BinCmp"),
+      this);
 }
 
 void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *MemberCall =
-      Result.Nodes.getNodeAs<CXXMemberCallExpr>("SizeCallExpr");
+  const auto *MemberCall = Result.Nodes.getNodeAs<Expr>("SizeCallExpr");
+  const auto *MemberCallObject =
+      Result.Nodes.getNodeAs<Expr>("MemberCallObject");
   const auto *BinCmp = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("BinCmp");
+  const auto *BinCmpTempl = Result.Nodes.getNodeAs<BinaryOperator>("BinCmp");
   const auto *BinaryOp = Result.Nodes.getNodeAs<BinaryOperator>("SizeBinaryOp");
   const auto *Pointee = Result.Nodes.getNodeAs<Expr>("Pointee");
   const auto *E =
-      MemberCall
-          ? MemberCall->getImplicitObjectArgument()
+      MemberCallObject
+          ? MemberCallObject
           : (Pointee ? Pointee : Result.Nodes.getNodeAs<Expr>("STLObject"));
   FixItHint Hint;
   std::string ReplacementText = std::string(
       Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
                            *Result.SourceManager, getLangOpts()));
-  if (BinCmp && IsBinaryOrTernary(E)) {
-    // Not just a DeclRefExpr, so parenthesize to be on the safe side.
+  if (IsBinaryOrTernary(E) || isa<UnaryOperator>(E)) {
     ReplacementText = "(" + ReplacementText + ")";
   }
   if (E->getType()->isPointerType())
     ReplacementText += "->empty()";
   else
     ReplacementText += ".empty()";
 
   if (BinCmp) {
     if (BinCmp->getOperator() == OO_ExclaimEqual) {
       ReplacementText = "!" + ReplacementText;
     }
     Hint =
         FixItHint::CreateReplacement(BinCmp->getSourceRange(), ReplacementText);
-  } else if (BinaryOp) {  // Determine the correct transformation.
+  } else if (BinCmpTempl) {
+    if (BinCmpTempl->getOpcode() == BinaryOperatorKind::BO_NE) {
+      ReplacementText = "!" + ReplacementText;
+    }
+    Hint = FixItHint::CreateReplacement(BinCmpTempl->getSourceRange(),
+                                        ReplacementText);
+  } else if (BinaryOp) { // Determine the correct transformation.
     bool Negation = false;
     const bool ContainerIsLHS =
         !llvm::isa<IntegerLiteral>(BinaryOp->getLHS()->IgnoreImpCasts());
     const auto OpCode = BinaryOp->getOpcode();
     uint64_t Value = 0;
     if (ContainerIsLHS) {
       if (const auto *Literal = llvm::dyn_cast<IntegerLiteral>(
               BinaryOp->getRHS()->IgnoreImpCasts()))
if (const auto *UnaryOp =
             Result.Nodes.getNodeAs<UnaryOperator>("NegOnSize"))
       Hint = FixItHint::CreateReplacement(UnaryOp->getSourceRange(),
                                           ReplacementText);
     else
       Hint = FixItHint::CreateReplacement(MemberCall->getSourceRange(),
                                           "!" + ReplacementText);
   }
 
-  if (MemberCall) {
-    diag(MemberCall->getBeginLoc(),
-         "the 'empty' method should be used to check "
+  auto WarnLoc = MemberCall ? MemberCall->getBeginLoc() : SourceLocation{};
+
+  if (WarnLoc.isValid()) {
+    diag(WarnLoc, "the 'empty' method should be used to check "
                   "for emptiness instead of 'size'")
         << Hint;
   } else {
-    diag(BinCmp->getBeginLoc(),
-         "the 'empty' method should be used to check "
+    WarnLoc = BinCmpTempl ? BinCmpTempl->getBeginLoc()
+                          : (BinCmp ? BinCmp->getBeginLoc() : SourceLocation{});
+    diag(WarnLoc, "the 'empty' method should be used to check "
                   "for emptiness instead of comparing to an empty object")
         << Hint;
   }
 
   const auto *Container = Result.Nodes.getNodeAs<NamedDecl>("container");
   if (const auto *CTS = dyn_cast<ClassTemplateSpecializationDecl>(Container)) {
     // The definition of the empty() method is the same for all implicit
     // instantiations. In order to avoid duplicate or inconsistent warnings
     // (depending on how deduplication is done), we use the same class name

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

 // RUN: %check_clang_tidy %s readability-container-size-empty %t
 
 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;
template <typename T> void f() {
   // 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-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-FIXES: {{^  }}if (v.empty()){{$}}
+  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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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
+  // TypeRequiresSize generally has `.empty()`
+  if (t.size())
+    ;
+
+  if (t == TypeRequiresSize{})
+    ;
+
+  if (t != TypeRequiresSize{})
+    ;
+}
+
+class TypeWithSize {
+public:
+  TypeWithSize();
+  bool operator==(const TypeWithSize &other) const;
+  bool operator!=(const TypeWithSize &other) const;
+
+  unsigned size() const { return 0; }
+  // Does not have `.empty()`
+};
+
+void instantiator() {
+  instantiatedTemplateWithSizeCall<TypeWithSize>();
+  instantiatedTemplateWithSizeCall<std::vector<int>>();
+}
+
+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-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-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-FIXES: {{^  }}return !C.empty() ? 0 : 1;
+}
+
+struct Lazy {
+  constexpr unsigned size() const { return 0; }
+  constexpr bool empty() const { return true; }
+};
+
+constexpr Lazy L;
+static_assert(!L.size(), "");
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
+// CHECK-MESSAGES: :663:18: note: method 'Lazy'::empty() defined here
+// CHECK-FIXES: {{^}}static_assert(L.empty(), "");
+
+struct StructWithLazyNoexcept {
+  void func() noexcept(L.size());
+};