abrahamcd updated this revision to Diff 440286.
abrahamcd added a comment.
Removed old test file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128372/new/
https://reviews.llvm.org/D128372
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,202 @@
+// 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>
+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();
+};
+
+template <class T>
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template <class T>
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+ std::vector<int> s;
+
+ s.empty();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+ std::vector_with_void_empty<int> m;
+
+ m.empty();
+ // no-warning
+
+ std::vector_with_clear<int> v;
+
+ v.empty();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}v.clear();{{$}}
+
+ std::vector_with_int_empty<int> w;
+
+ w.empty();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}w.clear();{{$}}
+
+ absl::string x;
+
+ x.empty();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+ absl::string_with_void_empty y;
+
+ y.empty();
+ // no-warning
+
+ absl::string_with_clear z;
+
+ z.empty();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}z.clear();{{$}}
+
+ absl::string_with_int_empty a;
+
+ a.empty();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}a.clear();{{$}}
+}
+
+int test_qualified_empty() {
+ absl::string_with_clear v;
+
+ std::empty(v);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: 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]]:3: 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 w;
+
+ std::empty(w);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+}
+
+int test_unqualified_empty() {
+ std::vector<int> v;
+
+ empty(v);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+ std::vector_with_void_empty<int> w;
+
+ empty(w);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}w.clear();{{$}}
+
+ std::vector_with_clear<int> x;
+
+ empty(x);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}x.clear();{{$}}
+
+ std::vector_with_int_empty<int> y;
+
+ empty(y);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}y.clear();{{$}}
+
+ absl::string z;
+
+ empty(z);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+
+ absl::string_with_void_empty a;
+
+ empty(a);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}a.clear();{{$}}
+
+ absl::string_with_clear b;
+
+ empty(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}b.clear();{{$}}
+
+ absl::string_with_int_empty c;
+
+ empty(c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }}c.clear();{{$}}
+
+ {
+ using std::empty;
+ empty(v);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+ }
+
+ {
+ using std::empty;
+ empty(w);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }} w.clear();{{$}}
+ }
+
+ {
+ using absl::empty;
+ empty(z);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+ }
+
+ {
+ using absl::empty;
+ empty(a);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+ // CHECK-FIXES: {{^ }} a.clear();{{$}}
+ }
+
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -114,6 +114,7 @@
`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>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
@@ -0,0 +1,31 @@
+.. 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();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,6 +122,11 @@
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.
Index: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
@@ -0,0 +1,35 @@
+//===--- 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) {}
+ 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
Index: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
@@ -0,0 +1,103 @@
+//===--- 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/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/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void StandaloneEmptyCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+ const auto NonMemberMatcher =
+ ast_matchers::callExpr(ast_matchers::callee(ast_matchers::functionDecl(
+ ast_matchers::hasName("empty"),
+ ast_matchers::unless(ast_matchers::returns(
+ ast_matchers::asString("void"))))))
+ .bind("empty");
+ const auto MemberMatcher =
+ ast_matchers::cxxMemberCallExpr(
+ ast_matchers::callee(ast_matchers::cxxMethodDecl(
+ ast_matchers::hasName("empty"),
+ ast_matchers::unless(
+ ast_matchers::returns(ast_matchers::asString("void"))))))
+ .bind("empty");
+
+ Finder->addMatcher(MemberMatcher, this);
+ Finder->addMatcher(NonMemberMatcher, this);
+}
+
+void StandaloneEmptyCheck::check(
+ const ast_matchers::MatchFinder::MatchResult &Result) {
+ if (const auto *MemberCall =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("empty")) {
+ SourceLocation MemberLoc = MemberCall->getBeginLoc();
+ SourceLocation ReplacementLoc = MemberCall->getExprLoc();
+ SourceRange ReplacementRange = SourceRange(ReplacementLoc, ReplacementLoc);
+
+ auto Methods = MemberCall->getRecordDecl()->methods();
+ auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+ return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+ });
+
+ if (Clear != Methods.end()) {
+ diag(MemberLoc,
+ "ignoring the result of 'empty()', did you mean 'clear()'? ")
+ << FixItHint::CreateReplacement(ReplacementRange, "clear");
+ } else {
+ diag(MemberLoc, "ignoring the result of 'empty()'");
+ }
+
+ } else if (const auto *NonMemberCall =
+ Result.Nodes.getNodeAs<CallExpr>("empty")) {
+ SourceLocation NonMemberLoc = NonMemberCall->getExprLoc();
+ SourceLocation NonMemberEndLoc = NonMemberCall->getEndLoc();
+
+ const Expr *Arg = NonMemberCall->getArg(0);
+
+ auto Methods = Arg->getType()->getAsCXXRecordDecl()->methods();
+ auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+ return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+ });
+
+ if (Clear != Methods.end()) {
+ 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);
+ } else {
+ diag(NonMemberLoc, "ignoring the result of '%0'")
+ << llvm::dyn_cast<NamedDecl>(NonMemberCall->getCalleeDecl())
+ ->getQualifiedNameAsString();
+ }
+ }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -44,6 +44,7 @@
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
SpuriouslyWakeUpFunctionsCheck.cpp
+ StandaloneEmptyCheck.cpp
StringConstructorCheck.cpp
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -48,6 +48,7 @@
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
#include "SpuriouslyWakeUpFunctionsCheck.h"
+#include "StandaloneEmptyCheck.h"
#include "StringConstructorCheck.h"
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
@@ -156,6 +157,8 @@
"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>(
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
