abrahamcd updated this revision to Diff 440742.
abrahamcd added a comment.
Progress Update
Hit a roadblock with determining unused return value,
uploading current state for further work.
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,244 @@
+// 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> v;
+
+ v.empty();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+ bool test = v.empty();
+ //(void) v.empty();
+ // no-warning
+ // NOTE: This test not currently passing.
+ }
+
+ {
+ 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();{{$}}
+ }
+
+ {
+ 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();{{$}}
+ }
+}
+
+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]
+ }
+}
+
+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();{{$}}
+ }
+
+ {
+ 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();{{$}}
+ }
+
+ {
+ 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();{{$}}
+ }
+}
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,30 @@
+.. 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,38 @@
+//===--- 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
Index: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
@@ -0,0 +1,106 @@
+//===--- 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) {
+ using namespace ast_matchers;
+ const auto NonMemberMatcher =
+ callExpr(
+ // unless(hasParent(stmt())),
+ callee(functionDecl(hasName("empty"), unless(returns(voidType())))))
+ .bind("empty");
+ const auto MemberMatcher =
+ expr(ignoringImplicit(ignoringParenImpCasts( cxxMemberCallExpr(hasParent(stmtExpr()),callee(
+ cxxMethodDecl(hasName("empty"), unless(returns(voidType()))))))))
+ .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);
+
+ CXXRecordDecl::method_range Methods =
+ MemberCall->getRecordDecl()->methods();
+ DeclContext::specific_decl_iterator<CXXMethodDecl> 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);
+
+ CXXRecordDecl *ArgRecordDecl = Arg->getType()->getAsCXXRecordDecl();
+ if (ArgRecordDecl == NULL)
+ return;
+ CXXRecordDecl::method_range Methods = ArgRecordDecl->methods();
+ DeclContext::specific_decl_iterator<CXXMethodDecl> 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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
