[clang-tools-extra] [clang-tidy] Add check hicpp-ignored-remove-result (PR #73119)

Björn Svensson via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 7 05:31:36 PST 2023


https://github.com/bjosv updated https://github.com/llvm/llvm-project/pull/73119

>From 91cf412abcfd231ab399c3e44c6a9bc14109537c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Tue, 21 Nov 2023 23:30:07 +0100
Subject: [PATCH 1/5] [clang-tidy] Add check hicpp-ignored-remove-result

This check implements the rule 17.5.1 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.

Suppressing issues by casting to `void` is enabled by default, but can be
disabled by setting `AllowCastToVoid` option to `false`.
---
 .../clang-tidy/hicpp/HICPPTidyModule.cpp      | 20 ++++++++
 clang-tools-extra/docs/ReleaseNotes.rst       |  4 ++
 .../checks/hicpp/ignored-remove-result.rst    | 20 ++++++++
 .../docs/clang-tidy/checks/list.rst           |  1 +
 .../checkers/hicpp/ignored-remove-result.cpp  | 47 +++++++++++++++++++
 5 files changed, 92 insertions(+)
 create mode 100644 clang-tools-extra/docs/clang-tidy/checks/hicpp/ignored-remove-result.rst
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp

diff --git a/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp b/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
index 3749796877120..09d15ccab3f29 100644
--- a/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../bugprone/UndelegatedConstructorCheck.h"
+#include "../bugprone/UnusedReturnValueCheck.h"
 #include "../bugprone/UseAfterMoveCheck.h"
 #include "../cppcoreguidelines/AvoidGotoCheck.h"
 #include "../cppcoreguidelines/NoMallocCheck.h"
@@ -41,6 +42,15 @@
 #include "NoAssemblerCheck.h"
 #include "SignedBitwiseCheck.h"
 
+namespace {
+
+// Checked functions for hicpp-ignored-remove-result.
+const llvm::StringRef CheckedFunctions = "::std::remove;"
+                                         "::std::remove_if;"
+                                         "::std::unique;";
+
+} // namespace
+
 namespace clang::tidy {
 namespace hicpp {
 
@@ -64,6 +74,8 @@ class HICPPModule : public ClangTidyModule {
         "hicpp-explicit-conversions");
     CheckFactories.registerCheck<readability::FunctionSizeCheck>(
         "hicpp-function-size");
+    CheckFactories.registerCheck<bugprone::UnusedReturnValueCheck>(
+        "hicpp-ignored-remove-result");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "hicpp-named-parameter");
     CheckFactories.registerCheck<bugprone::UseAfterMoveCheck>(
@@ -107,6 +119,14 @@ class HICPPModule : public ClangTidyModule {
     CheckFactories.registerCheck<cppcoreguidelines::ProTypeVarargCheck>(
         "hicpp-vararg");
   }
+
+  ClangTidyOptions getModuleOptions() override {
+    ClangTidyOptions Options;
+    ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
+    Opts["hicpp-ignored-remove-result.CheckedFunctions"] = CheckedFunctions;
+    Opts["hicpp-ignored-remove-result.AllowCastToVoid"] = "true";
+    return Options;
+  }
 };
 
 // Register the HICPPModule using this statically initialized variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d5f49dc06254..c940025df1c63 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -205,6 +205,10 @@ New check aliases
   <clang-tidy/checks/cppcoreguidelines/macro-to-enum>` to :doc:`modernize-macro-to-enum
   <clang-tidy/checks/modernize/macro-to-enum>` was added.
 
+- New alias :doc:`hicpp-ignored-remove-result
+  <clang-tidy/checks/hicpp/ignored-remove-result>` to :doc:`bugprone-unused-return-value
+  <clang-tidy/checks/bugprone/unused-return-value>` was added.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
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 0000000000000..4b6188b886db1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/hicpp/ignored-remove-result.rst
@@ -0,0 +1,20 @@
+.. 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 an alias of check :doc:`bugprone-unused-return-value <../bugprone/unused-return-value>`
+with a fixed set of functions.
+
+Suppressing issues by casting to ``void`` is enabled by default and can be
+disabled by setting `AllowCastToVoid` option to ``false``.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6f987ba1672e3..417513b2f2aaa 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 0000000000000..86deb96216a68
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s hicpp-ignored-remove-result %t
+
+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&);
+
+} // namespace std
+
+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
+
+  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
+
+  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
+}
+
+void noWarning() {
+
+  auto RemoveRetval = std::remove(nullptr, nullptr, 1);
+
+  auto RemoveIfRetval = std::remove_if(nullptr, nullptr, nullptr);
+
+  auto UniqueRetval = std::unique(nullptr, nullptr);
+
+  // cast to void should allow ignoring the return value
+  (void)std::remove(nullptr, nullptr, 1);
+
+  // no warning on std::find since the checker overrides
+  // bugprone-unused-return-value's checked functions.
+  std::find(nullptr, nullptr, 1);
+}

>From 46179bdf862a531f45b44f03de0803cec089e9b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Thu, 23 Nov 2023 09:49:00 +0100
Subject: [PATCH 2/5] fixup: missing double back-ticks in docs

---
 .../docs/clang-tidy/checks/hicpp/ignored-remove-result.rst      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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
index 4b6188b886db1..ff1d25d31f52c 100644
--- 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
@@ -11,7 +11,7 @@ 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.
+the first operand in a call to ``std::erase``.
 
 This check is an alias of check :doc:`bugprone-unused-return-value <../bugprone/unused-return-value>`
 with a fixed set of functions.

>From c1e05d10c6c68935b7f5894900492fa0f29aaea0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Thu, 23 Nov 2023 17:56:05 +0100
Subject: [PATCH 3/5] fixup: implement as own check (using add_new_check.py)

---
 .../bugprone/UnusedReturnValueCheck.cpp       |  5 ++++
 .../bugprone/UnusedReturnValueCheck.h         |  6 ++--
 .../clang-tidy/hicpp/CMakeLists.txt           |  1 +
 .../clang-tidy/hicpp/HICPPTidyModule.cpp      | 23 ++-------------
 .../hicpp/IgnoredRemoveResultCheck.cpp        | 28 ++++++++++++++++++
 .../hicpp/IgnoredRemoveResultCheck.h          | 29 +++++++++++++++++++
 clang-tools-extra/docs/ReleaseNotes.rst       | 10 ++++---
 .../checks/hicpp/ignored-remove-result.rst    |  3 --
 .../checkers/hicpp/ignored-remove-result.cpp  | 29 +++++++++++++++----
 9 files changed, 100 insertions(+), 34 deletions(-)
 create mode 100644 clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.h

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 6bc9f2dd367dc..a79e5d62eb48e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -133,6 +133,11 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
                                             "::boost::system::error_code"))),
       AllowCastToVoid(Options.get("AllowCastToVoid", false)) {}
 
+UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
+                                               ClangTidyContext *Context,
+                                               std::string CheckedFunctions)
+    : ClangTidyCheck(Name, Context), CheckedFunctions(CheckedFunctions) {}
+
 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 b4356f8379fdc..ca5181f438a2d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -28,10 +28,12 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
     return TK_IgnoreUnlessSpelledInSource;
   }
 
-private:
+protected:
+  UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context,
+                         std::string CheckedFunctions);
   std::string CheckedFunctions;
   const std::vector<StringRef> CheckedReturnTypes;
-  const 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 d12ca275d3964..132fbaccccf8a 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 09d15ccab3f29..daa9f398a740e 100644
--- a/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -10,7 +10,6 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../bugprone/UndelegatedConstructorCheck.h"
-#include "../bugprone/UnusedReturnValueCheck.h"
 #include "../bugprone/UseAfterMoveCheck.h"
 #include "../cppcoreguidelines/AvoidGotoCheck.h"
 #include "../cppcoreguidelines/NoMallocCheck.h"
@@ -38,19 +37,11 @@
 #include "../readability/NamedParameterCheck.h"
 #include "../readability/UppercaseLiteralSuffixCheck.h"
 #include "ExceptionBaseclassCheck.h"
+#include "IgnoredRemoveResultCheck.h"
 #include "MultiwayPathsCoveredCheck.h"
 #include "NoAssemblerCheck.h"
 #include "SignedBitwiseCheck.h"
 
-namespace {
-
-// Checked functions for hicpp-ignored-remove-result.
-const llvm::StringRef CheckedFunctions = "::std::remove;"
-                                         "::std::remove_if;"
-                                         "::std::unique;";
-
-} // namespace
-
 namespace clang::tidy {
 namespace hicpp {
 
@@ -67,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");
@@ -74,8 +67,6 @@ class HICPPModule : public ClangTidyModule {
         "hicpp-explicit-conversions");
     CheckFactories.registerCheck<readability::FunctionSizeCheck>(
         "hicpp-function-size");
-    CheckFactories.registerCheck<bugprone::UnusedReturnValueCheck>(
-        "hicpp-ignored-remove-result");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "hicpp-named-parameter");
     CheckFactories.registerCheck<bugprone::UseAfterMoveCheck>(
@@ -119,14 +110,6 @@ class HICPPModule : public ClangTidyModule {
     CheckFactories.registerCheck<cppcoreguidelines::ProTypeVarargCheck>(
         "hicpp-vararg");
   }
-
-  ClangTidyOptions getModuleOptions() override {
-    ClangTidyOptions Options;
-    ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
-    Opts["hicpp-ignored-remove-result.CheckedFunctions"] = CheckedFunctions;
-    Opts["hicpp-ignored-remove-result.AllowCastToVoid"] = "true";
-    return Options;
-  }
 };
 
 // Register the HICPPModule using this statically initialized variable.
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 0000000000000..d154c42c432f3
--- /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 0000000000000..48354c34a8581
--- /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 c940025df1c63..268040415d5e0 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.
 
@@ -205,10 +211,6 @@ New check aliases
   <clang-tidy/checks/cppcoreguidelines/macro-to-enum>` to :doc:`modernize-macro-to-enum
   <clang-tidy/checks/modernize/macro-to-enum>` was added.
 
-- New alias :doc:`hicpp-ignored-remove-result
-  <clang-tidy/checks/hicpp/ignored-remove-result>` to :doc:`bugprone-unused-return-value
-  <clang-tidy/checks/bugprone/unused-return-value>` was added.
-
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
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
index ff1d25d31f52c..62533f54f28a8 100644
--- 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
@@ -13,8 +13,5 @@ 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 an alias of check :doc:`bugprone-unused-return-value <../bugprone/unused-return-value>`
-with a fixed set of functions.
-
 Suppressing issues by casting to ``void`` is enabled by default and can be
 disabled by setting `AllowCastToVoid` option to ``false``.
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
index 86deb96216a68..b068f08590989 100644
--- 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
@@ -1,4 +1,5 @@
 // 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 {
 
@@ -14,20 +15,36 @@ 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() {
@@ -38,10 +55,12 @@ void noWarning() {
 
   auto UniqueRetval = std::unique(nullptr, nullptr);
 
-  // cast to void should allow ignoring the return value
-  (void)std::remove(nullptr, nullptr, 1);
-
-  // no warning on std::find since the checker overrides
-  // bugprone-unused-return-value's checked functions.
+  // 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();
 }

>From 696d46578886d1501dabeaabdeab60ad20ca9321 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Thu, 7 Dec 2023 01:13:23 +0100
Subject: [PATCH 4/5] fixup: review comments

---
 .../clang-tidy/bugprone/UnusedReturnValueCheck.cpp   | 12 +++++++++++-
 .../clang-tidy/bugprone/UnusedReturnValueCheck.h     | 10 ++++++++--
 .../clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp    |  2 +-
 .../checks/hicpp/ignored-remove-result.rst           |  8 ++++++--
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index a79e5d62eb48e..05012c7df6a97 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -136,7 +136,17 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
 UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
                                                ClangTidyContext *Context,
                                                std::string CheckedFunctions)
-    : ClangTidyCheck(Name, Context), CheckedFunctions(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);
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
index ca5181f438a2d..ab2cc691b894f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -28,11 +28,17 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
     return TK_IgnoreUnlessSpelledInSource;
   }
 
+private:
+  std::string CheckedFunctions;
+  const std::vector<StringRef> CheckedReturnTypes;
+
 protected:
   UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context,
                          std::string CheckedFunctions);
-  std::string CheckedFunctions;
-  const std::vector<StringRef> CheckedReturnTypes;
+  UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context,
+                         std::string CheckedFunctions,
+                         std::vector<StringRef> CheckedReturnTypes,
+                         bool AllowCastToVoid);
   bool AllowCastToVoid;
 };
 
diff --git a/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
index d154c42c432f3..3410559d435f6 100644
--- a/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
@@ -15,7 +15,7 @@ IgnoredRemoveResultCheck::IgnoredRemoveResultCheck(llvm::StringRef Name,
     : UnusedReturnValueCheck(Name, Context,
                              "::std::remove;"
                              "::std::remove_if;"
-                             "::std::unique;") {
+                             "::std::unique") {
   // The constructor for ClangTidyCheck needs to have been called
   // before we can access options via Options.get().
   AllowCastToVoid = Options.get("AllowCastToVoid", true);
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
index 62533f54f28a8..a5d86811157d9 100644
--- 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
@@ -13,5 +13,9 @@ 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``.
 
-Suppressing issues by casting to ``void`` is enabled by default and can be
-disabled by setting `AllowCastToVoid` option to ``false``.
+Options
+-------
+
+.. option:: AllowCastToVoid
+
+   Controls whether casting return values to ``void`` is permitted. Default: `true`.

>From 5f989474b4e4d79047fbd15e7d4aa3b04b0dda78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Thu, 7 Dec 2023 14:27:52 +0100
Subject: [PATCH 5/5] fixup: update docs

---
 .../docs/clang-tidy/checks/hicpp/ignored-remove-result.rst     | 3 +++
 1 file changed, 3 insertions(+)

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
index a5d86811157d9..6ca704ae3e66c 100644
--- 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
@@ -13,6 +13,9 @@ 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
 -------
 



More information about the cfe-commits mailing list