[clang-tidy] Add check hicpp-ignored-remove-result

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",

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

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

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");

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

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

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.
 

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`.

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>`,

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();
+}