llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: None (maflcko) <details> <summary>Changes</summary> The `bugprone-unused-return-value.AllowCastToVoid` default value is `false`. This is fine and was intentionally introduced after https://github.com/llvm/llvm-project/issues/66570#issue-1899312407, since it can be disabled via `NOLINT(bugprone-unused-return-value)`. However, there are normal code-patterns where a return value is explicitly discarded. Putting a NOLINT after such normal code, which often already comes with a cast-to-void, is verbose. C++17 allows to write `[[maybe_unused]] auto _{get_result(a, b)};`. However, this is equally verbose and not fully supported before C++26. Finally, C++26 introduces the `_` placeholder, which seems ideal to use over all alternatives: `(void)`, or `[[maybe_unused]]`, or `NOLINT`. Thus, switch the default for AllowCastToVoid, so that C++23 (and earlier) allow cast-to-void, and C++26 (and later) disallow it, promoting the use of the modern `_` placeholder. Personally I think the dynamic default value is ideal, because it allows a clean path toward the C++26 `_` identifier. Also, there is one doc-fixup commit. --- Full diff: https://github.com/llvm/llvm-project/pull/171618.diff 6 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp (+4-1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst (+27-23) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp (+2-1) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx23.cpp (+16) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx26-or-later.cpp (+15) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 6fbd3922b532d..2cb7e17ce1082 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -11,6 +11,7 @@ #include "../utils/OptionsUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/LangStandard.h" #include "clang/Basic/OperatorKinds.h" using namespace clang::ast_matchers; @@ -145,7 +146,9 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, "^::std::errc$;" "^::std::expected$;" "^::boost::system::error_code$"))), - AllowCastToVoid(Options.get("AllowCastToVoid", false)) {} + AllowCastToVoid( + Options.get("AllowCastToVoid", Context->getLangOpts().LangStd < + LangStandard::lang_cxx26)) {} UnusedReturnValueCheck::UnusedReturnValueCheck( llvm::StringRef Name, ClangTidyContext *Context, diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6d3da6183c86a..875157646d324 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -153,6 +153,12 @@ Improvements to clang-tidy `-header-filter='.*'`. To disable warnings from non-system, set `-header-filter` to an empty string. +- :doc:`bugprone-unused-return-value <clang-tidy/checks/bugprone/unused-return-value>` + now defaults ``AllowCastToVoid`` to ``false`` when running with C++26 or newer + language modes, while older standards keep the previous ``true`` default. + With C++26, the ``_`` placeholder should be used to mark unused return + values. + - :program:`clang-tidy` no longer attempts to analyze code from system headers by default, greatly improving performance. This behavior is disabled if the `SystemHeaders` option is enabled. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst index 725403a6eb818..f8ecb05bb9bc0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst @@ -17,26 +17,26 @@ Options This parameter supports regexp. The function is checked if the name and scope matches, with any arguments. By default the following functions are checked: - ``^::std::async$, ^::std::launder$, ^::std::remove$, ^::std::remove_if$, - ^::std::unique$, ^::std::unique_ptr::release$, ^::std::basic_string::empty$, - ^::std::vector::empty$, ^::std::back_inserter$, ^::std::distance$, - ^::std::find$, ^::std::find_if$, ^::std::inserter$, ^::std::lower_bound$, - ^::std::make_pair$, ^::std::map::count$, ^::std::map::find$, - ^::std::map::lower_bound$, ^::std::multimap::equal_range$, - ^::std::multimap::upper_bound$, ^::std::set::count$, ^::std::set::find$, - ^::std::setfill$, ^::std::setprecision$, ^::std::setw$, ^::std::upper_bound$, - ^::std::vector::at$, ^::bsearch$, ^::ferror$, ^::feof$, ^::isalnum$, - ^::isalpha$, ^::isblank$, ^::iscntrl$, ^::isdigit$, ^::isgraph$, ^::islower$, - ^::isprint$, ^::ispunct$, ^::isspace$, ^::isupper$, ^::iswalnum$, - ^::iswprint$, ^::iswspace$, ^::isxdigit$, ^::memchr$, ^::memcmp$, ^::strcmp$, - ^::strcoll$, ^::strncmp$, ^::strpbrk$, ^::strrchr$, ^::strspn$, ^::strstr$, - ^::wcscmp$, ^::access$, ^::bind$, ^::connect$, ^::difftime$, ^::dlsym$, - ^::fnmatch$, ^::getaddrinfo$, ^::getopt$, ^::htonl$, ^::htons$, - ^::iconv_open$, ^::inet_addr$, isascii$, isatty$, ^::mmap$, ^::newlocale$, - ^::openat$, ^::pathconf$, ^::pthread_equal$, ^::pthread_getspecific$, - ^::pthread_mutex_trylock$, ^::readdir$, ^::readlink$, ^::recvmsg$, - ^::regexec$, ^::scandir$, ^::semget$, ^::setjmp$, ^::shm_open$, ^::shmget$, - ^::sigismember$, ^::strcasecmp$, ^::strsignal$, ^::ttyname$`` + ``^::std::async$; ^::std::launder$; ^::std::remove$; ^::std::remove_if$; + ^::std::unique$; ^::std::unique_ptr::release$; ^::std::basic_string::empty$; + ^::std::vector::empty$; ^::std::back_inserter$; ^::std::distance$; + ^::std::find$; ^::std::find_if$; ^::std::inserter$; ^::std::lower_bound$; + ^::std::make_pair$; ^::std::map::count$; ^::std::map::find$; + ^::std::map::lower_bound$; ^::std::multimap::equal_range$; + ^::std::multimap::upper_bound$; ^::std::set::count$; ^::std::set::find$; + ^::std::setfill$; ^::std::setprecision$; ^::std::setw$; ^::std::upper_bound$; + ^::std::vector::at$; ^::bsearch$; ^::ferror$; ^::feof$; ^::isalnum$; + ^::isalpha$; ^::isblank$; ^::iscntrl$; ^::isdigit$; ^::isgraph$; ^::islower$; + ^::isprint$; ^::ispunct$; ^::isspace$; ^::isupper$; ^::iswalnum$; + ^::iswprint$; ^::iswspace$; ^::isxdigit$; ^::memchr$; ^::memcmp$; ^::strcmp$; + ^::strcoll$; ^::strncmp$; ^::strpbrk$; ^::strrchr$; ^::strspn$; ^::strstr$; + ^::wcscmp$; ^::access$; ^::bind$; ^::connect$; ^::difftime$; ^::dlsym$; + ^::fnmatch$; ^::getaddrinfo$; ^::getopt$; ^::htonl$; ^::htons$; + ^::iconv_open$; ^::inet_addr$; isascii$; isatty$; ^::mmap$; ^::newlocale$; + ^::openat$; ^::pathconf$; ^::pthread_equal$; ^::pthread_getspecific$; + ^::pthread_mutex_trylock$; ^::readdir$; ^::readlink$; ^::recvmsg$; + ^::regexec$; ^::scandir$; ^::semget$; ^::setjmp$; ^::shm_open$; ^::shmget$; + ^::sigismember$; ^::strcasecmp$; ^::strsignal$; ^::ttyname$`` - ``std::async()``. Not using the return value makes the call synchronous. - ``std::launder()``. Not using the return value usually means that the @@ -58,12 +58,16 @@ Options Semicolon-separated list of function return types to check. By default the following function return types are checked: - `^::std::error_code$`, `^::std::error_condition$`, `^::std::errc$`, - `^::std::expected$`, `^::boost::system::error_code$` + ``^::std::error_code$; ^::std::error_condition$; ^::std::errc$; + ^::std::expected$; ^::boost::system::error_code$`` .. option:: AllowCastToVoid - Controls whether casting return values to ``void`` is permitted. Default: `false`. + Controls whether casting return values to ``void`` is permitted. Default: + ``true`` for pre-C++26 language modes and ``false`` when the language standard + is C++26 or newer. + With C++26, the ``_`` placeholder should be used to mark unused return + values. :doc:`cert-err33-c <../cert/err33-c>` is an alias of this check that checks a fixed and large set of standard library functions. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp index 3035183573ccd..ac59cbe341de0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp @@ -1,7 +1,8 @@ // RUN: %check_clang_tidy %s bugprone-unused-return-value %t \ // RUN: -config='{CheckOptions: \ // RUN: {bugprone-unused-return-value.CheckedFunctions: \ -// RUN: "::fun;::ns::Outer::Inner::memFun;::ns::Type::staticFun;::ns::ClassTemplate::(mem|static)Fun"}}' \ +// RUN: "::fun;::ns::Outer::Inner::memFun;::ns::Type::staticFun;::ns::ClassTemplate::(mem|static)Fun", \ +// RUN: bugprone-unused-return-value.AllowCastToVoid: false}}' \ // RUN: -- namespace std { diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx23.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx23.cpp new file mode 100644 index 0000000000000..9ac00b0b8e750 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx23.cpp @@ -0,0 +1,16 @@ +// RUN: %check_clang_tidy -std=c++23 %s bugprone-unused-return-value %t + +namespace std { +struct future {}; +template <typename Function, typename... Args> +future async(Function &&, Args &&...); +} // namespace std + +int foo(); + +void allowCastToVoidInCxx23() { + std::async(foo); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors + (void)std::async(foo); + [[maybe_unused]] auto _ = std::async(foo); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx26-or-later.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx26-or-later.cpp new file mode 100644 index 0000000000000..51ed82e0e7333 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-cxx26-or-later.cpp @@ -0,0 +1,15 @@ +// RUN: %check_clang_tidy -std=c++26-or-later %s bugprone-unused-return-value %t + +namespace std { +struct future {}; +template <typename Function, typename... Args> +future async(Function &&, Args &&...); +} // namespace std + +int baz(); + +void disallowCastToVoidInCxx26() { + (void)std::async(baz); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors + auto _ = std::async(baz); +} `````````` </details> https://github.com/llvm/llvm-project/pull/171618 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
