diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 6bc9f2dd367dc..05012c7df6a97 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 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 b4356f8379fdc..ab2cc691b894f 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 CheckedReturnTypes; - const bool AllowCastToVoid; + +protected: + UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context, + std::string CheckedFunctions); + UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context, + std::string CheckedFunctions, + std::vector 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 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 3749796877120..daa9f398a740e 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( "hicpp-exception-baseclass"); + CheckFactories.registerCheck( + "hicpp-ignored-remove-result"); CheckFactories.registerCheck( "hicpp-multiway-paths-covered"); CheckFactories.registerCheck("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 0000000000000..3410559d435f6 --- /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 9262f9bbfe62a..6d91748e4cef1 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 + ` 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 ` 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 0000000000000..6ca704ae3e66c --- /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 `_. + +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 e6c02fe48fbf8..31f0e090db1d7 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 `, :doc:`google-upgrade-googletest-case `, "Yes" :doc:`hicpp-exception-baseclass `, + :doc:`hicpp-ignored-remove-result `, :doc:`hicpp-multiway-paths-covered `, :doc:`hicpp-no-assembler `, :doc:`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..b068f08590989 --- /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 +ForwardIt remove(ForwardIt, ForwardIt, const T &); + +template +ForwardIt remove_if(ForwardIt, ForwardIt, UnaryPredicate); + +template +ForwardIt unique(ForwardIt, ForwardIt); + +template +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(); +}