[clang-tools-extra] [clang-tidy]avoid bugprone-unused-return-value false positive for function with the same prefix as the default argument (PR #84333)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 7 22:10:34 PST 2024
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/84333
>From aaa9ea1db21b5de5d8be454ce1b1d05b219cccfc Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Thu, 7 Mar 2024 23:28:40 +0800
Subject: [PATCH 1/2] [clang-tidy]avoid bugprone-unused-return-value false
negative for function with the same prefix as the default argument
string without `$` end will cause incorrectly match results.
Fixes: #84314.
---
.../bugprone/UnusedReturnValueCheck.cpp | 184 +++++++++---------
clang-tools-extra/docs/ReleaseNotes.rst | 4 +-
.../checkers/bugprone/unused-return-value.cpp | 14 ++
3 files changed, 109 insertions(+), 93 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index b4bf85c912c3ca..1252b2f23805a1 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -34,102 +34,102 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
CheckedFunctions(utils::options::parseStringList(
- Options.get("CheckedFunctions", "::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;"
+ Options.get("CheckedFunctions", "::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$;"
// C standard library
- "::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;"
+ "::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$;"
// POSIX
- "::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;"
+ "::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"))),
CheckedReturnTypes(utils::options::parseStringList(
- Options.get("CheckedReturnTypes", "::std::error_code;"
- "::std::error_condition;"
- "::std::errc;"
- "::std::expected;"
+ Options.get("CheckedReturnTypes", "::std::error_code$;"
+ "::std::error_condition$;"
+ "::std::errc$;"
+ "::std::expected$;"
"::boost::system::error_code"))),
AllowCastToVoid(Options.get("AllowCastToVoid", false)) {}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d98c4ff9a75044..b5f025ce467a15 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -152,7 +152,9 @@ Changes in existing checks
- Improved :doc:`bugprone-unused-return-value
<clang-tidy/checks/bugprone/unused-return-value>` check by updating the
- parameter `CheckedFunctions` to support regexp.
+ parameter `CheckedFunctions` to support regexp and avoiding false postive for
+ function with the same prefix as the default argument, e.g. ``std::unique_ptr``
+ and ``std::unique``.
- Improved :doc:`bugprone-use-after-move
<clang-tidy/checks/bugprone/use-after-move>` check to also handle
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
index 5c6ce1e4bf1fd2..e784c9b85172cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
@@ -30,6 +30,11 @@ struct default_delete;
template <typename T, typename Deleter = std::default_delete<T>>
struct unique_ptr {
+ unique_ptr();
+ unique_ptr(unique_ptr const&);
+ unique_ptr(unique_ptr &&);
+ unique_ptr& operator=(unique_ptr const&);
+ unique_ptr& operator=(unique_ptr &&);
T *release() noexcept;
};
@@ -254,3 +259,12 @@ void noWarning() {
({ std::async(increment, 42); });
auto StmtExprRetval = ({ std::async(increment, 42); });
}
+
+namespace gh84314 {
+
+extern std::unique_ptr<int> alloc();
+void f1(std::unique_ptr<int> &foo) {
+ foo = alloc();
+}
+
+} // namespace gh84314
\ No newline at end of file
>From e9505c3bd3fe5fc0805183e8df0bb7e9ce3e4d54 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Fri, 8 Mar 2024 14:09:01 +0800
Subject: [PATCH 2/2] update docs
---
.../checks/bugprone/unused-return-value.rst | 33 ++++++++++---------
1 file changed, 17 insertions(+), 16 deletions(-)
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 8d5eddbe215cb8..9c01ef50b53814 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
@@ -14,22 +14,23 @@ 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
More information about the cfe-commits
mailing list