[Clang-Tidy] Empty Check

[denik]

Summary

Adds a clang-tidy check for the incorrect use of empty() on a
container when the result of the call is ignored.

Authored-by: Abraham Corea Diaz <[e-mail]>
Co-authored-by: Denis Nikitin <[e-mail]>

[cjdb]

A great start, thanks!

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

52–58

Note to other reviewers: I suggested this because we couldn't work out a better way to identify if .clear() exists (Abraham can best expand on the issues regarding how a matcher wasn't working).

76–79

It looks as though this is unconditionally suggesting clear (even if it doesn't exist), and doesn't suggest it in the message.

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h

19

Please fix this :)

clang-tools-extra/docs/ReleaseNotes.rst

128

• s/container/range
• This should probably mention it suggests clear() if it's a member function.

clang-tools-extra/docs/clang-tidy/checks/list.rst

363

I don't think the changes in this file were intetional?

clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

5–9 ↗

(On Diff #439131)

Please remove clear from vector, and add the following vector_with_clear and vector_with_void_empty and vector_with_int_empty.

We should be testing that clear() is only a suggested fix if it's present (but x.empty() is flagged either way), and only when x.empty() returns an integral result.

17–21 ↗

(On Diff #439131)

As above. Please also rename absl::vector to something like absl::string (and drop the template altogether).

[tschuett]

Just for reference:
https://reviews.llvm.org/D128267

[cjdb]

In D128372#3602881, @tschuett wrote:

Just for reference:
https://reviews.llvm.org/D128267

Ack. I still think this CL is useful, given that not every library will have [[nodiscard]], and because it can suggest appropriate alternatives.

[cjdb]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

62

I wonder if we should add a fixit hint that suggests x = T{}; in the event x.clear() isn't available (e.g. std::stack).

[Eugene.Zelenko]

Please add documentation for check.

clang-tools-extra/docs/ReleaseNotes.rst

126

Path to documentation was changed recently.

clang-tools-extra/docs/clang-tidy/checks/list.rst

363

Please revert all changes made because of incorrect rebase.

[Eugene.Zelenko]

Tests locations were changed recently too.

[LegalizeAdulthood]

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto main:HEAD and:

• fold your changes into the appropriate subdirs, stripping the module prefix from new files.
• make the target check-clang-extra to validate your tests
• make the target docs-clang-tools-html to validate any docs changes

[abrahamcd]

Added functionality to check if member function clear() exists before
suggesting it as a fix, including updated tests to reflect that. Fixed
path change and rebase errors from the initial revision.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

56

Please don't use auto unless type is explicitly stated in same statement or iterator.

77

Please don't use auto unless type is explicitly stated in same statement or iterator.

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h

23

Please fix URL.

30

Please define isLanguageVersionSupported().

clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst

6

Excessive newline.

10

Boolean.

[LegalizeAdulthood]

clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

1 ↗

(On Diff #439938)

This file should be removed from the changelist

[abrahamcd]

Removed old test file.

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

52–58

Yes, I wrote a matcher for the declaration of a .clear() member function and another matcher for a call to .empty(), but it seemed like the check() function runs multiple times to match the different matchers. It would find .clear() on one run of check() and find .empty() on another, and I couldn't find a way to see if .clear() had already been found on a previous run of check().

[abrahamcd]

Addressed review edits and clarity feedback.

[denik]

Thanks for adding a check! Please check my comments.

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

77

Check getAsCXXRecordDecl() for null and add a test case.

clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

62

I think you also need no-warning test cases when the empty() result is used.

[cjdb]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

77

We should also have a test to ensure empty(0) doesn't segfault.

[abrahamcd]

Progress Update

Hit a roadblock with determining unused return value,
uploading current state for further work.

[abrahamcd]

Stuck on determining unused return value, uploaded current state for @denik and @cjdb to take a look.

[abrahamcd]

Adds functionality for only warning on the case of unused return value
of the call to empty() and removes using-directive.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

147

nullptr.

[cjdb]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

129

Let's eliminate some of these else-clauses by using early exits.

[abrahamcd]

Adds argument 0 test case and refactoring based on feedback.

[cjdb]

Thanks for working on this! I'll merge this on your behalf once there's maintainer approval.

[denik]

Thanks Abraham!

[abrahamcd]

Gentle ping!

[abrahamcd]

Hi all, I just wanted to check in on this again and see if any more feedback or progress could be made. Thank you!

[rsmith]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

24

150

This only looks for direct members, not ones found in a base class. Eg, this will not find clear() in llvm::SmallVector<T, N>. If you have a Sema object here, you could do a real name lookup; otherwise, you can use CXXRecordDecl::lookupInBases to do the lookup. You should be able to find some examples by searching for existing calls to that function.

[abrahamcd]

Checks for clear() in base classes as well.

[abrahamcd]

Adds non-dependent base class test case.

[njames93]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

58–73

These using declarations are annoying, the common convention in tidy checks is to just use a global using namespace ast_matchers at the top of the file.

121–122

We also need to check qualifiers. If there exists a clear method but its unavailable due to it not being const when our member is const. The fix would result in a compile error.

124

dyn_cast isn't needed here as we have already checked we are dealing with a CXXMethodDecl.

132

The 'did you mean' diagnostics in clang all use a semicolon ; instead of a comma between the message and suggestion.

177–178

Diagnostics can accept args as const NamedDecl *, so you can omit the call to getQualifiedNameAsString().
The use of dyn_cast here is suspicious. If the CalleeDecl isn't a NamedDecl, this would assert when you try and call getQualifiedNameAsString, but equally I can't see any case why the CalleeDecl wouldn't be a NamedDecl. @rsmith Can you think of any situation where this could happen?

[abrahamcd]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

177–178

Seems that without getQualifiedNameAsString() I get longer less-readable versions of the name, e.g. 'empty<std::vector<int> &>' instead of just 'std::empty'. Do you think the extra information is helpful here?

[cjdb]

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

58–73

Adding them just after the namespaces are opened would be good.

I've had issues with refactoring llvm-project files in the past that have global using-directives, so I'd prefer it if we stuck with the using-declarations please. If they're a part of the preamble, they become additional lines that will be scrolled over.

121–122

Good catch. We probably shouldn't be suggesting clear() on a const object anyway.

177–178

I'm partial to 'std::empty' because that's presumably what the user will type, and so it'll be easier for them to understand. 'empty<std::vector<int> &>' isn't useful because we almost always defer to type deduction.

[abrahamcd]

Adds check for the compatibility of qualifiers on the clear method and the object it is called on.

[abrahamcd]

Minor formatting/naming fixes.

[abrahamcd]

Hi, checking in on this again, thanks!

[cjdb]

@njames93, @LegalizeAdulthood, are there any further concerns regarding this CL?

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

127

Please synchronize with first statement in documentation.

clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

630

Excessive newline.

[abrahamcd]

Formatting fixes and synchronization of documentation.

[cjdb]

This patch has been open for weeks without maintainer input, despite repeated requests, and @abrahamcd finishes his internship this Friday. We would very much appreciate direction on whether or not D128372 is ready to be merged.

[cjdb]

Ping.

[MaskRay]

Drive-by comments: this patch may need a review from common reviewers like @njames93.

I assume that @LegalizeAdulthood's comment (about the file hierarchy since there was a big reorganization few months ago) has been addressed. If you are concerned with unable to contact @LegalizeAdulthood, I think folks like @njames93 can decide moving forward the patch despite the "Request Changes" state. That said, it'll always be good to give @LegalizeAdulthood some time when the patch is in a ready-to-submit state.

[denik]

Any updates from the reviewers?

[denik]

@LegalizeAdulthood, @njames93, is there anything else we should address in this change?

[denik]

@LegalizeAdulthood, @njames93, friendly ping.

[cjdb]

@LegalizeAdulthood @njames93 is there anything actionable that's left for this patch?

[denik]

@Eugene.Zelenko , could you please stamp the patch if you don't have any other concerns?

@njames93 , if you don't have spare cycles could you please suggest other reviewers?

[Eugene.Zelenko]

In D128372#3883214, @denik wrote:

@Eugene.Zelenko , could you please stamp the patch if you don't have any other concerns?

Please wait for developers approval. I look only for trivial issues.

[cjdb]

@njames93 if you don't have any further concerns, I am going to merge this on Friday afternoon, as it will have been four months since there has been a maintainer's input.

[carlosgalvezp]

In D128372#3957982, @cjdb wrote:

@njames93 if you don't have any further concerns, I am going to merge this on Friday afternoon, as it will have been four months since there has been a maintainer's input.

@cjdb I have raised a concern about the lack of reviewer feedback in clang-tidy here:
https://discourse.llvm.org/t/rfc-improve-code-review-process-for-clang-tidy/66740

If more people express their concerns perhaps it can be escalated to the point where action is taken?

[MaskRay]

I just took a glance and the code looks reasonable.

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

132

delete blank line

161

delete blank line

clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

162

unneeded blank line

ditto below

They just make tests less compact without good values.

178

[[@LINE-1]] is deprecated lit syntax. Use [[#@LINE-1]]. Don't use the prevailing style in clang-tidy tests.

[denik]

Format changes.

Removed blank lines and fixed test types.

[denik]

Replaced deprecated [[@LINE-1]]

[denik]

In D128372#3965162, @MaskRay wrote:

I just took a glance and the code looks reasonable.

@MaskRay thanks for the review! I have updated the patch.

[MyDeveloperDay]

Ironically as an aside... looking back at the review because of the discource article.

The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years ago was to cover exactly this use case...and many others like it where users have a function which returns a boolean and takes no arguments are pointer or reference (empty() included)

https://reviews.llvm.org/D55433

Shouldn't people be using this checker to add nodiscard and let the compiler to the heavy lifting. Was there a use case as to why that solution doesn't solve this?

[Eugene.Zelenko]

Please take a loon on https://github.com/llvm/llvm-project/issues/59487.

[cjdb]

In D128372#3991516, @MyDeveloperDay wrote:

Ironically as an aside... looking back at the review because of the discource article.

The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years ago was to cover exactly this use case...and many others like it where users have a function which returns a boolean and takes no arguments are pointer or reference (empty() included)

https://reviews.llvm.org/D55433

Shouldn't people be using this checker to add nodiscard and let the compiler to the heavy lifting. Was there a use case as to why that solution doesn't solve this?

That check is for library authors: this one is for library users. There are a non-insignificant number of cases where an unused call to empty pops up, hence the desire for this check.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
+#include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "StringviewNullptrCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
 #include "SuspiciousIncludeCheck.h"
 #include "SuspiciousMemoryComparisonCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<SignedCharMisuseCheck>(
         "bugprone-signed-char-misuse");
     CheckFactories.registerCheck<SizeofContainerCheck>(
         "bugprone-sizeof-container");
     CheckFactories.registerCheck<SizeofExpressionCheck>(
         "bugprone-sizeof-expression");
     CheckFactories.registerCheck<SpuriouslyWakeUpFunctionsCheck>(
         "bugprone-spuriously-wake-up-functions");
+    CheckFactories.registerCheck<StandaloneEmptyCheck>(
+        "bugprone-standalone-empty");
     CheckFactories.registerCheck<StringConstructorCheck>(
         "bugprone-string-constructor");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "bugprone-string-integer-assignment");
     CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
         "bugprone-string-literal-with-embedded-nul");
     CheckFactories.registerCheck<StringviewNullptrCheck>(
         "bugprone-stringview-nullptr");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   ReservedIdentifierCheck.cpp
   SharedPtrArrayMismatchCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
   SignalHandlerCheck.cpp
   SignedCharMisuseCheck.cpp
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
+  StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
   StringviewNullptrCheck.cpp
   SuspiciousEnumUsageCheck.cpp
   SuspiciousIncludeCheck.cpp
   SuspiciousMemoryComparisonCheck.cpp
   SuspiciousMemsetUsageCheck.cpp

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h

+//===--- StandaloneEmptyCheck.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_BUGPRONE_STANDALONEEMPTYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STANDALONEEMPTYCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Checks for ignored calls to `empty()` on a range and suggests `clear()`
+/// as an alternative if it is an existing member function.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/standalone-empty.html
+class StandaloneEmptyCheck : public ClangTidyCheck {
+public:
+  StandaloneEmptyCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, 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;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STANDALONEEMPTYCHECK_H

clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp

+//===--- StandaloneEmptyCheck.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 "StandaloneEmptyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+using ast_matchers::BoundNodes;
+using ast_matchers::MatchFinder;
+
+const Expr *getCondition(const BoundNodes &Nodes, const StringRef NodeId) {
+  const auto *If = Nodes.getNodeAs<IfStmt>(NodeId);
+  if (If != nullptr)
+    return If->getCond();
+
+  const auto *For = Nodes.getNodeAs<ForStmt>(NodeId);
+  if (For != nullptr)
+    return For->getCond();
+
+  const auto *While = Nodes.getNodeAs<WhileStmt>(NodeId);
+  if (While != nullptr)
+    return While->getCond();
+
+  const auto *Do = Nodes.getNodeAs<DoStmt>(NodeId);
+  if (Do != nullptr)
+    return Do->getCond();
+
+  const auto *Switch = Nodes.getNodeAs<SwitchStmt>(NodeId);
+  if (Switch != nullptr)
+    return Switch->getCond();
+
+  return nullptr;
+}
+
+void StandaloneEmptyCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  using ast_matchers::callee;
+  using ast_matchers::callExpr;
+  using ast_matchers::cxxMemberCallExpr;
+  using ast_matchers::cxxMethodDecl;
+  using ast_matchers::expr;
+  using ast_matchers::functionDecl;
+  using ast_matchers::hasName;
+  using ast_matchers::hasParent;
+  using ast_matchers::ignoringImplicit;
+  using ast_matchers::ignoringParenImpCasts;
+  using ast_matchers::optionally;
+  using ast_matchers::returns;
+  using ast_matchers::stmt;
+  using ast_matchers::stmtExpr;
+  using ast_matchers::unless;
+  using ast_matchers::voidType;
+
+  const auto NonMemberMatcher = expr(ignoringImplicit(ignoringParenImpCasts(
+      callExpr(
+          hasParent(stmt(optionally(hasParent(stmtExpr().bind("stexpr"))))
+                        .bind("parent")),
+          callee(functionDecl(hasName("empty"), unless(returns(voidType())))))
+          .bind("empty"))));
+  const auto MemberMatcher =
+      expr(ignoringImplicit(ignoringParenImpCasts(cxxMemberCallExpr(
+               hasParent(stmt(optionally(hasParent(stmtExpr().bind("stexpr"))))
+                             .bind("parent")),
+               callee(cxxMethodDecl(hasName("empty"),
+                                    unless(returns(voidType()))))))))
+          .bind("empty");
+
+  Finder->addMatcher(MemberMatcher, this);
+  Finder->addMatcher(NonMemberMatcher, this);
+}
+
+void StandaloneEmptyCheck::check(const MatchFinder::MatchResult &Result) {
+  // Skip if the parent node is Expr.
+  if (Result.Nodes.getNodeAs<Expr>("parent"))
+    return;
+
+  const auto PParentStmtExpr = Result.Nodes.getNodeAs<Expr>("stexpr");
+  const auto ParentCompStmt = Result.Nodes.getNodeAs<CompoundStmt>("parent");
+  const auto *ParentCond = getCondition(Result.Nodes, "parent");
+
+  if (const auto *MemberCall =
+          Result.Nodes.getNodeAs<CXXMemberCallExpr>("empty")) {
+    // Skip if it's a condition of the parent statement.
+    if (ParentCond == MemberCall->getExprStmt())
+      return;
+    // Skip if it's the last statement in the GNU extension
+    // statement expression.
+    if (PParentStmtExpr && ParentCompStmt &&
+        ParentCompStmt->body_back() == MemberCall->getExprStmt())
+      return;
+
+    SourceLocation MemberLoc = MemberCall->getBeginLoc();
+    SourceLocation ReplacementLoc = MemberCall->getExprLoc();
+    SourceRange ReplacementRange = SourceRange(ReplacementLoc, ReplacementLoc);
+
+    ASTContext &Context = MemberCall->getRecordDecl()->getASTContext();
+    DeclarationName Name =
+        Context.DeclarationNames.getIdentifier(&Context.Idents.get("clear"));
+
+    auto Candidates =
+        MemberCall->getRecordDecl()->lookupDependentName(Name, [](const NamedDecl *ND) {
+          return isa<CXXMethodDecl>(ND) &&
+                 llvm::dyn_cast<CXXMethodDecl>(ND)->getMinRequiredArguments() ==
+                     0;
+        });
+
+    bool HasClear = !Candidates.empty();
+
+    if (HasClear) {
+      diag(MemberLoc,
+           "ignoring the result of 'empty()', did you mean 'clear()'? ")
+          << FixItHint::CreateReplacement(ReplacementRange, "clear");
+      return;
+    }
+
+    diag(MemberLoc, "ignoring the result of 'empty()'");
+
+  } else if (const auto *NonMemberCall =
+                 Result.Nodes.getNodeAs<CallExpr>("empty")) {
+    if (ParentCond == NonMemberCall->getExprStmt())
+      return;
+    if (PParentStmtExpr && ParentCompStmt &&
+        ParentCompStmt->body_back() == NonMemberCall->getExprStmt())
+      return;
+
+    SourceLocation NonMemberLoc = NonMemberCall->getExprLoc();
+    SourceLocation NonMemberEndLoc = NonMemberCall->getEndLoc();
+
+    const Expr *Arg = NonMemberCall->getArg(0);
+
+    CXXRecordDecl *ArgRecordDecl = Arg->getType()->getAsCXXRecordDecl();
+    if (ArgRecordDecl == nullptr)
+      return;
+
+    ASTContext &Context = ArgRecordDecl->getASTContext();
+    DeclarationName Name =
+        Context.DeclarationNames.getIdentifier(&Context.Idents.get("clear"));
+
+    auto Candidates =
+        ArgRecordDecl->lookupDependentName(Name, [](const NamedDecl *ND) {
+          return isa<CXXMethodDecl>(ND) &&
+                 llvm::dyn_cast<CXXMethodDecl>(ND)->getMinRequiredArguments() ==
+                     0;
+        });
+
+    bool HasClear = !Candidates.empty();
+
+    if (HasClear) {
+      std::string ReplacementText =
+          std::string(Lexer::getSourceText(
+              CharSourceRange::getTokenRange(Arg->getSourceRange()),
+              *Result.SourceManager, getLangOpts())) +
+          ".clear()";
+      SourceRange ReplacementRange = SourceRange(NonMemberLoc, NonMemberEndLoc);
+      diag(NonMemberLoc, "ignoring the result of '%0', did you mean 'clear()'?")
+          << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl())
+                 ->getQualifiedNameAsString()
+          << FixItHint::CreateReplacement(ReplacementRange, ReplacementText);
+      return;
+    }
+
+    diag(NonMemberLoc, "ignoring the result of '%0'")
+        << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl())
+               ->getQualifiedNameAsString();
+  }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 
 New checks
 ^^^^^^^^^^
 
 - New :doc:`bugprone-shared-ptr-array-mismatch <clang-tidy/checks/bugprone/shared-ptr-array-mismatch>` check.
 
   Finds initializations of C++ shared pointers to non-array type that are initialized with an array.
 
+- New :doc:`bugprone-standalone-empty <clang-tidy/checks/bugprone/standalone-empty>` check.
+
+  Warns when `empty()` is used on a range and the result is ignored. Suggests `clear()` as an alternative
+  if it is an existing member function.
+
 - New :doc:`bugprone-unchecked-optional-access
   <clang-tidy/checks/bugprone/unchecked-optional-access>` check.
 
   Warns when the code is unwrapping a `std::optional<T>`, `absl::optional<T>`,
   or `base::Optional<T>` object without assuring that it contains a value.
 
 - New :doc:`misc-confusable-identifiers <clang-tidy/checks/misc/confusable-identifiers>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst

+.. title:: clang-tidy - bugprone-standalone-empty
+
+bugprone-standalone-empty
+=========================
+
+Checks for inaccurate use of the ``empty()`` method.
+
+The ``empty()`` method on several common ranges returns a Boolean indicating
+whether or not the range is empty, but is often mistakenly interpreted as
+a way to clear the contents of a range. Some ranges offer a ``clear()``
+method for this purpose. This check warns when a call to empty returns a
+result that is ignored, and suggests replacing it with a call to ``clear()``
+if it is available as a member function of the range.
+
+For example, the following code could be used to indicate whether a range
+is empty or not, but the result is ignored:
+
+.. code-block:: c++
+
+  std::vector<int> v;
+  ...
+  v.empty();
+
+A call to ``clear()`` would appropriately clear the contents of the range:
+
+.. code-block:: c++
+
+  std::vector<int> v;
+  ...
+  v.clear();

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `bugprone-redundant-branch-condition <bugprone/redundant-branch-condition.html>`_, "Yes"
    `bugprone-reserved-identifier <bugprone/reserved-identifier.html>`_, "Yes"
    `bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch.html>`_, "Yes"
    `bugprone-signal-handler <bugprone/signal-handler.html>`_,
    `bugprone-signed-char-misuse <bugprone/signed-char-misuse.html>`_,
    `bugprone-sizeof-container <bugprone/sizeof-container.html>`_,
    `bugprone-sizeof-expression <bugprone/sizeof-expression.html>`_,
    `bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions.html>`_,
+   `bugprone-standalone-empty <bugprone/standalone-empty.html>`_, "Yes"
    `bugprone-string-constructor <bugprone/string-constructor.html>`_, "Yes"
    `bugprone-string-integer-assignment <bugprone/string-integer-assignment.html>`_, "Yes"
    `bugprone-string-literal-with-embedded-nul <bugprone/string-literal-with-embedded-nul.html>`_,
    `bugprone-stringview-nullptr <bugprone/stringview-nullptr.html>`_, "Yes"
    `bugprone-suspicious-enum-usage <bugprone/suspicious-enum-usage.html>`_,
    `bugprone-suspicious-include <bugprone/suspicious-include.html>`_,
    `bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison.html>`_,
    `bugprone-suspicious-memset-usage <bugprone/suspicious-memset-usage.html>`_, "Yes"
.. csv-table::
    `readability-simplify-subscript-expr <readability/simplify-subscript-expr.html>`_, "Yes"
    `readability-static-accessed-through-instance <readability/static-accessed-through-instance.html>`_, "Yes"
    `readability-static-definition-in-anonymous-namespace <readability/static-definition-in-anonymous-namespace.html>`_, "Yes"
    `readability-string-compare <readability/string-compare.html>`_, "Yes"
    `readability-suspicious-call-argument <readability/suspicious-call-argument.html>`_,
    `readability-uniqueptr-delete-release <readability/uniqueptr-delete-release.html>`_, "Yes"
    `readability-uppercase-literal-suffix <readability/uppercase-literal-suffix.html>`_, "Yes"
    `readability-use-anyofallof <readability/use-anyofallof.html>`_,
    `zircon-temporary-objects <zircon/temporary-objects.html>`_,
 
 
 .. csv-table:: Aliases..
    :header: "Name", "Redirect", "Offers fixes"
 
    `bugprone-narrowing-conversions <bugprone/narrowing-conversions.html>`_, `cppcoreguidelines-narrowing-conversions <cppcoreguidelines/narrowing-conversions.html>`_,
    `cert-con36-c <cert/con36-c.html>`_, `bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions.html>`_,
    `cert-con54-cpp <cert/con54-cpp.html>`_, `bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions.html>`_,

clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template <typename T>
+struct vector {
+  bool empty();
+};
+
+template <typename T>
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template <typename T>
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template <typename T>
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template <typename T>
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template <typename T>
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template <typename T>
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template <class T>
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template <class T>
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template <typename T>
+struct base_vector {
+    void clear();
+};
+
+template <typename T>
+struct base_vector_clear_with_args {
+    void clear(int i);
+};
+
+template <typename T>
+struct base_vector_clear_variable {
+    int clear;
+};
+
+struct base_vector_non_dependent {
+    void clear();
+};
+
+template <typename T>
+struct vector : base_vector<T> {
+    bool empty();
+};
+
+template <typename T>
+struct vector_clear_with_args : base_vector_clear_with_args<T> {
+    bool empty();
+};
+
+template <typename T>
+struct vector_clear_variable : base_vector_clear_variable<T> {
+    bool empty();
+};
+
+template <typename T>
+struct vector_non_dependent : base_vector_non_dependent {
+    bool empty();
+};
+
+template <typename T>
+bool empty(T &&);
+
+}
+
+int test_member_empty() {
+  {
+    std::vector<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+    std::vector_with_void_empty<int> v;
+
+    v.empty();
+    // no-warning
+  }
+
+  {
+    std::vector_with_clear<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    std::vector_with_int_empty<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    std::vector_with_clear_args<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+    std::vector_with_clear_variable<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+    absl::string s;
+
+    s.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+    absl::string_with_void_empty s;
+
+    s.empty();
+    // no-warning
+  }
+
+  {
+    absl::string_with_clear s;
+
+    s.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+    absl::string_with_int_empty s;
+
+    s.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+    absl::string_with_clear_args s;
+
+    s.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+    absl::string_with_clear_variable s;
+
+    s.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+}
+
+int test_qualified_empty() {
+  {
+    absl::string_with_clear v;
+
+    std::empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+    absl::empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+    test::empty(v);
+    // no-warning
+  }
+
+  {
+    absl::string s;
+
+    std::empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    std::empty(0);
+    // no-warning
+
+    absl::empty(nullptr);
+    // no-warning
+  }
+}
+
+int test_unqualified_empty() {
+  {
+    std::vector<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    std::vector_with_void_empty<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    std::vector_with_clear<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    std::vector_with_int_empty<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+  
+  {
+    std::vector_with_clear_args<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    std::vector_with_clear_variable<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    absl::string s;
+
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    absl::string_with_void_empty s;
+
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+    absl::string_with_clear s;
+
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+    absl::string_with_int_empty s;
+
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+    absl::string_with_clear_args s;
+
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    absl::string_with_clear_variable s;
+
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    std::vector<int> v;
+    using std::empty;
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    std::vector_with_clear<int> v;
+    using std::empty;
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    absl::string s;
+    using absl::empty;
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    absl::string_with_clear s;
+    using absl::empty;
+    empty(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+}
+
+int test_empty_method_expressions() {
+  std::vector<int> v;
+
+  bool EmptyReturn(v.empty());
+  // no-warning
+
+  (void)v.empty();
+  // no-warning
+
+  // Don't warn in the if condition.
+  if (v.empty()) v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  // Don't warn in the for condition.
+  for(v.empty();v.empty();v.empty()) v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  // CHECK-MESSAGES: :[[@LINE-3]]:38: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  // Don't warn in the while condition.
+  while(v.empty()) v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  // Don't warn in the do-while condition.
+  do v.empty(); while(v.empty());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  // Don't warn in the switch expression.
+  switch(v.empty()) {
+    // no-warning
+    case true:
+      v.empty();
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  // Don't warn in the return expression, which is the last statement.
+  bool StmtExprReturn = ({v.empty(); v.empty();});
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+}
+
+int test_empty_expressions() {
+  absl::string s;
+
+  bool test(std::empty(s));
+  // no-warning
+
+  (void)std::empty(s);
+  // no-warning
+
+  if (std::empty(s)) std::empty(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  for(std::empty(s);std::empty(s);std::empty(s)) std::empty(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-MESSAGES: :[[@LINE-2]]:35: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-MESSAGES: :[[@LINE-3]]:50: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  while(std::empty(s)) std::empty(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  do std::empty(s); while(std::empty(s));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  switch(std::empty(s)) {
+    // no-warning
+    case true:
+      std::empty(s);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  bool StmtExprReturn = ({std::empty(s); std::empty(s);});
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+}
+
+int test_clear_in_base_class() {
+
+  {
+    base::vector<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    base::vector_non_dependent<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    base::vector_clear_with_args<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+    base::vector_clear_variable<int> v;
+
+    v.empty();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+    base::vector<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'base::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    base::vector_non_dependent<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'base::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+    // CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+    base::vector_clear_with_args<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'base::empty' [bugprone-standalone-empty]
+  }
+
+  {
+    base::vector_clear_variable<int> v;
+
+    empty(v);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'base::empty' [bugprone-standalone-empty]
+  }
+
+}