[clang-tools-extra] 3ed940a - [clang-tidy] Add check hicpp-ignored-remove-result (#73119)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 7 11:55:21 PST 2023
Author: Björn Svensson
Date: 2023-12-07T20:55:17+01:00
New Revision: 3ed940ac3dac03d044a8d1e51005cec84dd128f9
URL: https://github.com/llvm/llvm-project/commit/3ed940ac3dac03d044a8d1e51005cec84dd128f9
DIFF: https://github.com/llvm/llvm-project/commit/3ed940ac3dac03d044a8d1e51005cec84dd128f9.diff
LOG: [clang-tidy] Add check hicpp-ignored-remove-result (#73119)
This check implements the [rule
17.5.1](https://www.perforce.com/resources/qac/high-integrity-cpp-coding-standard/standard-library)
of the HICPP standard which states:
- Do not ignore the result of std::remove, std::remove_if or std::unique
The mutating algorithms std::remove, std::remove_if and both overloads
of std::unique operate by swapping or moving elements of the range they
are operating over. On completion, they return an iterator to the last
valid element. In the majority of cases the correct behavior is to use
this result as the first operand in a call to std::erase.
This check is based on `bugprone-unused-return-value` but with a fixed
set of functions.
Suppressing issues by casting to `void` is enabled by default, but can
be disabled by setting `AllowCastToVoid` option to `false`.
Added:
clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.h
clang-tools-extra/docs/clang-tidy/checks/hicpp/ignored-remove-result.rst
clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt
clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 6bc9f2dd367dcb..05012c7df6a975 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -133,6 +133,21 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
"::boost::system::error_code"))),
AllowCastToVoid(Options.get("AllowCastToVoid", false)) {}
+UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
+ ClangTidyContext *Context,
+ std::string CheckedFunctions)
+ : UnusedReturnValueCheck(Name, Context, std::move(CheckedFunctions), {},
+ false) {}
+
+UnusedReturnValueCheck::UnusedReturnValueCheck(
+ llvm::StringRef Name, ClangTidyContext *Context,
+ std::string CheckedFunctions, std::vector<StringRef> CheckedReturnTypes,
+ bool AllowCastToVoid)
+ : ClangTidyCheck(Name, Context),
+ CheckedFunctions(std::move(CheckedFunctions)),
+ CheckedReturnTypes(std::move(CheckedReturnTypes)),
+ AllowCastToVoid(AllowCastToVoid) {}
+
void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "CheckedFunctions", CheckedFunctions);
Options.store(Opts, "CheckedReturnTypes",
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
index b4356f8379fdc8..ab2cc691b894f7 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -31,7 +31,15 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
private:
std::string CheckedFunctions;
const std::vector<StringRef> CheckedReturnTypes;
- const bool AllowCastToVoid;
+
+protected:
+ UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context,
+ std::string CheckedFunctions);
+ UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context,
+ std::string CheckedFunctions,
+ std::vector<StringRef> CheckedReturnTypes,
+ bool AllowCastToVoid);
+ bool AllowCastToVoid;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt b/clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt
index d12ca275d39647..132fbaccccf8a9 100644
--- a/clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyHICPPModule
ExceptionBaseclassCheck.cpp
HICPPTidyModule.cpp
+ IgnoredRemoveResultCheck.cpp
MultiwayPathsCoveredCheck.cpp
NoAssemblerCheck.cpp
SignedBitwiseCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp b/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
index 3749796877120e..daa9f398a740ed 100644
--- a/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -37,6 +37,7 @@
#include "../readability/NamedParameterCheck.h"
#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "ExceptionBaseclassCheck.h"
+#include "IgnoredRemoveResultCheck.h"
#include "MultiwayPathsCoveredCheck.h"
#include "NoAssemblerCheck.h"
#include "SignedBitwiseCheck.h"
@@ -57,6 +58,8 @@ class HICPPModule : public ClangTidyModule {
"hicpp-deprecated-headers");
CheckFactories.registerCheck<ExceptionBaseclassCheck>(
"hicpp-exception-baseclass");
+ CheckFactories.registerCheck<IgnoredRemoveResultCheck>(
+ "hicpp-ignored-remove-result");
CheckFactories.registerCheck<MultiwayPathsCoveredCheck>(
"hicpp-multiway-paths-covered");
CheckFactories.registerCheck<SignedBitwiseCheck>("hicpp-signed-bitwise");
diff --git a/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
new file mode 100644
index 00000000000000..3410559d435f63
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
@@ -0,0 +1,28 @@
+//===--- IgnoredRemoveResultCheck.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 "IgnoredRemoveResultCheck.h"
+
+namespace clang::tidy::hicpp {
+
+IgnoredRemoveResultCheck::IgnoredRemoveResultCheck(llvm::StringRef Name,
+ ClangTidyContext *Context)
+ : UnusedReturnValueCheck(Name, Context,
+ "::std::remove;"
+ "::std::remove_if;"
+ "::std::unique") {
+ // The constructor for ClangTidyCheck needs to have been called
+ // before we can access options via Options.get().
+ AllowCastToVoid = Options.get("AllowCastToVoid", true);
+}
+
+void IgnoredRemoveResultCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "AllowCastToVoid", AllowCastToVoid);
+}
+
+} // namespace clang::tidy::hicpp
diff --git a/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.h b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.h
new file mode 100644
index 00000000000000..48354c34a8581a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.h
@@ -0,0 +1,29 @@
+//===--- IgnoredRemoveResultCheck.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_HICPP_IGNOREDREMOVERESULTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_IGNOREDREMOVERESULTCHECK_H
+
+#include "../bugprone/UnusedReturnValueCheck.h"
+
+namespace clang::tidy::hicpp {
+
+/// Ensure that the result of std::remove, std::remove_if and std::unique
+/// are not ignored according to rule 17.5.1.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/hicpp/ignored-remove-result.html
+class IgnoredRemoveResultCheck : public bugprone::UnusedReturnValueCheck {
+public:
+ IgnoredRemoveResultCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+};
+
+} // namespace clang::tidy::hicpp
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_IGNOREDREMOVERESULTCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9262f9bbfe62a7..6d91748e4cef18 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -174,6 +174,12 @@ New checks
Flags coroutines that suspend while a lock guard is in scope at the
suspension point.
+- New :doc:`hicpp-ignored-remove-result
+ <clang-tidy/checks/hicpp/ignored-remove-result>` check.
+
+ Ensure that the result of ``std::remove``, ``std::remove_if`` and
+ ``std::unique`` are not ignored according to rule 17.5.1.
+
- New :doc:`misc-coroutine-hostile-raii
<clang-tidy/checks/misc/coroutine-hostile-raii>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/hicpp/ignored-remove-result.rst b/clang-tools-extra/docs/clang-tidy/checks/hicpp/ignored-remove-result.rst
new file mode 100644
index 00000000000000..6ca704ae3e66c7
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/hicpp/ignored-remove-result.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - hicpp-ignored-remove-result
+
+hicpp-ignored-remove-result
+===========================
+
+Ensure that the result of ``std::remove``, ``std::remove_if`` and ``std::unique``
+are not ignored according to
+`rule 17.5.1 <https://www.perforce.com/resources/qac/high-integrity-cpp-coding-standard/standard-library>`_.
+
+The mutating algorithms ``std::remove``, ``std::remove_if`` and both overloads
+of ``std::unique`` operate by swapping or moving elements of the range they are
+operating over. On completion, they return an iterator to the last valid
+element. In the majority of cases the correct behavior is to use this result as
+the first operand in a call to ``std::erase``.
+
+This check is a subset of :doc:`bugprone-unused-return-value <../bugprone/unused-return-value>`
+and depending on used options it can be superfluous to enable both checks.
+
+Options
+-------
+
+.. option:: AllowCastToVoid
+
+ Controls whether casting return values to ``void`` is permitted. Default: `true`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index e6c02fe48fbf86..31f0e090db1d7d 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -226,6 +226,7 @@ Clang-Tidy Checks
:doc:`google-runtime-operator <google/runtime-operator>`,
:doc:`google-upgrade-googletest-case <google/upgrade-googletest-case>`, "Yes"
:doc:`hicpp-exception-baseclass <hicpp/exception-baseclass>`,
+ :doc:`hicpp-ignored-remove-result <hicpp/ignored-remove-result>`,
:doc:`hicpp-multiway-paths-covered <hicpp/multiway-paths-covered>`,
:doc:`hicpp-no-assembler <hicpp/no-assembler>`,
:doc:`hicpp-signed-bitwise <hicpp/signed-bitwise>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp b/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp
new file mode 100644
index 00000000000000..b068f085909893
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s hicpp-ignored-remove-result %t
+// RUN: %check_clang_tidy -check-suffixes=NOCAST %s hicpp-ignored-remove-result %t -- -config='{CheckOptions: {hicpp-ignored-remove-result.AllowCastToVoid: false}}'
+
+namespace std {
+
+template <typename ForwardIt, typename T>
+ForwardIt remove(ForwardIt, ForwardIt, const T &);
+
+template <typename ForwardIt, typename UnaryPredicate>
+ForwardIt remove_if(ForwardIt, ForwardIt, UnaryPredicate);
+
+template <typename ForwardIt>
+ForwardIt unique(ForwardIt, ForwardIt);
+
+template <class InputIt, class T>
+InputIt find(InputIt, InputIt, const T&);
+
+class error_code {
+};
+
+} // namespace std
+
+std::error_code errorFunc() {
+ return std::error_code();
+}
+
+void warning() {
+ std::remove(nullptr, nullptr, 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES-NOCAST: [[@LINE-3]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+
+ std::remove_if(nullptr, nullptr, nullptr);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES-NOCAST: [[@LINE-3]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+
+ std::unique(nullptr, nullptr);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES-NOCAST: [[@LINE-3]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+}
+
+void optionalWarning() {
+ // No warning unless AllowCastToVoid=false
+ (void)std::remove(nullptr, nullptr, 1);
+ // CHECK-MESSAGES-NOCAST: [[@LINE-1]]:9: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+}
+
+void noWarning() {
+
+ auto RemoveRetval = std::remove(nullptr, nullptr, 1);
+
+ auto RemoveIfRetval = std::remove_if(nullptr, nullptr, nullptr);
+
+ auto UniqueRetval = std::unique(nullptr, nullptr);
+
+ // Verify that other checks in the baseclass are not used.
+ // - no warning on std::find since the checker overrides
+ // bugprone-unused-return-value's checked functions.
+ std::find(nullptr, nullptr, 1);
+ // - no warning on return types since the checker disable
+ // bugprone-unused-return-value's checked return types.
+ errorFunc();
+ (void) errorFunc();
+}
More information about the cfe-commits
mailing list