From 0097fd2b068e374b02571b391bd4a0a84e8ed3e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?= <60141446+felix642@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:05:06 -0500 Subject: [PATCH] [clang-tidy] bugprone-unused-return-value config now supports regexes (#82952) The parameter `CheckedFunctions` now supports regexes Fixes #63107 --- .../bugprone/UnusedReturnValueCheck.cpp | 202 +++++++++--------- .../bugprone/UnusedReturnValueCheck.h | 6 +- .../hicpp/IgnoredRemoveResultCheck.cpp | 8 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/bugprone/unused-return-value.rst | 5 +- .../bugprone/unused-return-value-custom.cpp | 2 +- 6 files changed, 117 insertions(+), 110 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp index 05012c7df6a975..b4bf85c912c3ca 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp @@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher, UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - CheckedFunctions(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;" - // 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;" - "::ttyname")), + 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;" + // 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;" + // 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;" + "::ttyname"))), CheckedReturnTypes(utils::options::parseStringList( Options.get("CheckedReturnTypes", "::std::error_code;" "::std::error_condition;" @@ -133,36 +133,36 @@ 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::UnusedReturnValueCheck( + llvm::StringRef Name, ClangTidyContext *Context, + std::vector CheckedFunctions) : UnusedReturnValueCheck(Name, Context, std::move(CheckedFunctions), {}, false) {} UnusedReturnValueCheck::UnusedReturnValueCheck( llvm::StringRef Name, ClangTidyContext *Context, - std::string CheckedFunctions, std::vector CheckedReturnTypes, - bool AllowCastToVoid) + std::vector 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, "CheckedFunctions", + utils::options::serializeStringList(CheckedFunctions)); Options.store(Opts, "CheckedReturnTypes", utils::options::serializeStringList(CheckedReturnTypes)); Options.store(Opts, "AllowCastToVoid", AllowCastToVoid); } void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) { - auto FunVec = utils::options::parseStringList(CheckedFunctions); - auto MatchedDirectCallExpr = expr(callExpr(callee(functionDecl( // Don't match void overloads of checked functions. unless(returns(voidType())), - anyOf(isInstantiatedFrom(hasAnyName(FunVec)), + anyOf(isInstantiatedFrom(matchers::matchesAnyListedName( + CheckedFunctions)), returns(hasCanonicalType(hasDeclaration( namedDecl(matchers::matchesAnyListedName( CheckedReturnTypes))))))))) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h index ab2cc691b894f7..d65a567e1c468a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h @@ -29,14 +29,14 @@ class UnusedReturnValueCheck : public ClangTidyCheck { } private: - std::string CheckedFunctions; + const std::vector CheckedFunctions; const std::vector CheckedReturnTypes; protected: UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context, - std::string CheckedFunctions); + std::vector CheckedFunctions); UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context, - std::string CheckedFunctions, + std::vector CheckedFunctions, std::vector 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 3410559d435f63..8020f8cd062510 100644 --- a/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp @@ -13,9 +13,11 @@ namespace clang::tidy::hicpp { IgnoredRemoveResultCheck::IgnoredRemoveResultCheck(llvm::StringRef Name, ClangTidyContext *Context) : UnusedReturnValueCheck(Name, Context, - "::std::remove;" - "::std::remove_if;" - "::std::unique") { + { + "::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); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0d2467210fc664..70ae23ba185af4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -140,6 +140,10 @@ Changes in existing checks ` check by ignoring local variable with ``[maybe_unused]`` attribute. +- Improved :doc:`bugprone-unused-return-value + ` check by updating the + parameter `CheckedFunctions` to support regexp. + - Improved :doc:`cppcoreguidelines-missing-std-forward ` check by no longer giving false positives for deleted functions. 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 1e3c8a3268222e..8d5eddbe215cb8 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 @@ -10,8 +10,9 @@ Options .. option:: CheckedFunctions - Semicolon-separated list of functions to check. The function is checked if - the name and scope matches, with any arguments. + Semicolon-separated list of functions to check. + 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, 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 d3650b210ab02b..3035183573ccd7 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,7 @@ // 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::memFun;::ns::ClassTemplate::staticFun"}}' \ +// RUN: "::fun;::ns::Outer::Inner::memFun;::ns::Type::staticFun;::ns::ClassTemplate::(mem|static)Fun"}}' \ // RUN: -- namespace std {