Author: Tyler Rockwood Date: 2023-12-25T12:19:53+01:00 New Revision: 952d344f3e25a352bc5f2f1f5f611e96bd7acb91
URL: https://github.com/llvm/llvm-project/commit/952d344f3e25a352bc5f2f1f5f611e96bd7acb91 DIFF: https://github.com/llvm/llvm-project/commit/952d344f3e25a352bc5f2f1f5f611e96bd7acb91.diff LOG: [clang-tidy] introduce a unused local non trival variable check (#76101) Introduce a new (off by default) clang tidy check to ensure that variables of a specific type are always used even if -Wunused-variables wouldn't generate a warning. This check has already caught a couple of different bugs on the codebase I work on, where not handling a future means that lifetimes may not be kept alive properly as an async chunk of code may run after a class has been destroyed, etc. I would like to upstream it because I believe there could be other applications of this check that would be useful in different contexts. --------- Signed-off-by: Tyler Rockwood <rockw...@redpanda.com> Added: clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.cpp clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.h clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-local-non-trivial-variable.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp Modified: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 7a910037368c83..435cb1e3fbcff3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -83,6 +83,7 @@ #include "UnhandledSelfAssignmentCheck.h" #include "UniquePtrArrayMismatchCheck.h" #include "UnsafeFunctionsCheck.h" +#include "UnusedLocalNonTrivialVariableCheck.h" #include "UnusedRaiiCheck.h" #include "UnusedReturnValueCheck.h" #include "UseAfterMoveCheck.h" @@ -235,6 +236,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-unique-ptr-array-mismatch"); CheckFactories.registerCheck<UnsafeFunctionsCheck>( "bugprone-unsafe-functions"); + CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>( + "bugprone-unused-local-non-trivial-variable"); CheckFactories.registerCheck<UnusedRaiiCheck>("bugprone-unused-raii"); CheckFactories.registerCheck<UnusedReturnValueCheck>( "bugprone-unused-return-value"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index d443fd8d1452f1..70e7fbc7ec0c14 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -79,6 +79,7 @@ add_clang_library(clangTidyBugproneModule UnhandledSelfAssignmentCheck.cpp UniquePtrArrayMismatchCheck.cpp UnsafeFunctionsCheck.cpp + UnusedLocalNonTrivialVariableCheck.cpp UnusedRaiiCheck.cpp UnusedReturnValueCheck.cpp UseAfterMoveCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.cpp new file mode 100644 index 00000000000000..ee7f365320ff9c --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.cpp @@ -0,0 +1,91 @@ +//===--- UnusedLocalNonTrivialVariableCheck.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 "UnusedLocalNonTrivialVariableCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h" + +using namespace clang::ast_matchers; +using namespace clang::tidy::matchers; + +namespace clang::tidy::bugprone { + +namespace { +static constexpr StringRef DefaultIncludeTypeRegex = + "::std::.*mutex;::std::future;::std::basic_string;::std::basic_regex;" + "::std::base_istringstream;::std::base_stringstream;::std::bitset;" + "::std::filesystem::path"; + +AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); } +AST_MATCHER(VarDecl, isReferenced) { return Node.isReferenced(); } +AST_MATCHER(Type, isReferenceType) { return Node.isReferenceType(); } +AST_MATCHER(QualType, isTrivial) { + return Node.isTrivialType(Finder->getASTContext()) || + Node.isTriviallyCopyableType(Finder->getASTContext()); +} +} // namespace + +UnusedLocalNonTrivialVariableCheck::UnusedLocalNonTrivialVariableCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeTypes(utils::options::parseStringList( + Options.get("IncludeTypes", DefaultIncludeTypeRegex))), + ExcludeTypes( + utils::options::parseStringList(Options.get("ExcludeTypes", ""))) {} + +void UnusedLocalNonTrivialVariableCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeTypes", + utils::options::serializeStringList(IncludeTypes)); + Options.store(Opts, "ExcludeTypes", + utils::options::serializeStringList(ExcludeTypes)); +} + +void UnusedLocalNonTrivialVariableCheck::registerMatchers(MatchFinder *Finder) { + if (IncludeTypes.empty()) + return; + + Finder->addMatcher( + varDecl(isLocalVarDecl(), unless(isReferenced()), + unless(isExceptionVariable()), hasLocalStorage(), isDefinition(), + unless(hasType(isReferenceType())), unless(hasType(isTrivial())), + hasType(hasUnqualifiedDesugaredType( + anyOf(recordType(hasDeclaration(namedDecl( + matchesAnyListedName(IncludeTypes), + unless(matchesAnyListedName(ExcludeTypes))))), + templateSpecializationType(hasDeclaration(namedDecl( + matchesAnyListedName(IncludeTypes), + unless(matchesAnyListedName(ExcludeTypes))))))))) + .bind("var"), + this); +} + +void UnusedLocalNonTrivialVariableCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("var"); + diag(MatchedDecl->getLocation(), "unused local variable %0 of type %1") + << MatchedDecl << MatchedDecl->getType(); +} + +bool UnusedLocalNonTrivialVariableCheck::isLanguageVersionSupported( + const LangOptions &LangOpts) const { + return LangOpts.CPlusPlus; +} + +std::optional<TraversalKind> +UnusedLocalNonTrivialVariableCheck::getCheckTraversalKind() const { + return TK_IgnoreUnlessSpelledInSource; +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.h new file mode 100644 index 00000000000000..e79b803a2158b6 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.h @@ -0,0 +1,44 @@ +//===--- UnusedLocalNonTrivialVariableCheck.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_BUGPRONE_UNUSEDLOCALNONTRIVIALVARIABLECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNUSEDLOCALNONTRIVIALVARIABLECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Warns when a local non trivial variable is unused within a function. By +/// default std::.*mutex and std::future are included. +/// +/// The check supports these options: +/// - 'IncludeTypes': a semicolon-separated list of regular expressions +/// matching types to ensure must be used. +/// - 'ExcludeTypes': a semicolon-separated list of regular expressions +/// matching types that are excluded from the +/// 'IncludeTypes' matches. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unused-local-non-trivial-variable.html +class UnusedLocalNonTrivialVariableCheck : public ClangTidyCheck { +public: + UnusedLocalNonTrivialVariableCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override; + std::optional<TraversalKind> getCheckTraversalKind() const override; + +private: + const std::vector<StringRef> IncludeTypes; + const std::vector<StringRef> ExcludeTypes; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNUSEDLOCALNONTRIVIALVARIABLECHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6d91748e4cef18..6e7554e0433c25 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -168,6 +168,11 @@ New checks extracted from an optional-like type and then used to create a new instance of the same optional-like type. +- New :doc:`bugprone-unused-local-non-trivial-variable + <clang-tidy/checks/bugprone/unused-local-non-trivial-variable>` check. + + Warns when a local non trivial variable is unused within a function. + - New :doc:`cppcoreguidelines-no-suspend-with-lock <clang-tidy/checks/cppcoreguidelines/no-suspend-with-lock>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-local-non-trivial-variable.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-local-non-trivial-variable.rst new file mode 100644 index 00000000000000..7a72a08d8f3aa8 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-local-non-trivial-variable.rst @@ -0,0 +1,56 @@ +.. title:: clang-tidy - bugprone-unused-local-non-trivial-variable + +bugprone-unused-local-non-trivial-variable +========================================== + +Warns when a local non trivial variable is unused within a function. +The following types of variables are excluded from this check: + +* trivial and trivially copyable +* references and pointers +* exception variables in catch clauses +* static or thread local +* structured bindings + +This check can be configured to warn on all non-trivial variables by setting +`IncludeTypes` to `.*`, and excluding specific types using `ExcludeTypes`. + +In the this example, `my_lock` would generate a warning that it is unused. + +.. code-block:: c++ + + std::mutex my_lock; + // my_lock local variable is never used + +In the next example, `future2` would generate a warning that it is unused. + +.. code-block:: c++ + + std::future<MyObject> future1; + std::future<MyObject> future2; + // ... + MyObject foo = future1.get(); + // future2 is not used. + +Options +------- + +.. option:: IncludeTypes + + Semicolon-separated list of regular expressions matching types of variables + to check. + By default the following types are checked: + + * `::std::.*mutex` + * `::std::future` + * `::std::string` + * `::std::basic_regex` + * `::std::basic_istringstream` + * `::std::basic_stringstream` + * `::std::bitset` + * `::std::path` + +.. option:: ExcludeTypes + + A semicolon-separated list of regular expressions matching types that are + excluded from the `IncludeTypes` matches. By default it is an empty list. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 31f0e090db1d7d..39d8b490d927cc 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -149,6 +149,7 @@ Clang-Tidy Checks :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`, :doc:`bugprone-unique-ptr-array-mismatch <bugprone/unique-ptr-array-mismatch>`, "Yes" :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`, + :doc:`bugprone-unused-local-non-trivial-variable <bugprone/unused-local-non-trivial-variable>`, :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes" :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`, :doc:`bugprone-use-after-move <bugprone/use-after-move>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp new file mode 100644 index 00000000000000..9bbf3d116885f3 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp @@ -0,0 +1,118 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-unused-local-non-trivial-variable %t -- \ +// RUN: -config="{CheckOptions: {bugprone-unused-local-non-trivial-variable.IncludeTypes: '::async::Future;::async::Foo.*', bugprone-unused-local-non-trivial-variable.ExcludeTypes: '::async::FooBar'}}" + + +namespace async { +template <typename T> +class Ptr { + public: + explicit Ptr(T Arg) : Underlying(new T(Arg)) {} + T& operator->() { + return Underlying; + } + ~Ptr() { + delete Underlying; + } + private: + T* Underlying; +}; + +template<typename T> +class Future { +public: + T get() { + return Pending; + } + ~Future(); +private: + T Pending; +}; + +class FooBar { + public: + ~FooBar(); + private: + Future<int> Fut; +}; + +class FooQux { + public: + ~FooQux(); + private: + Future<int> Fut; +}; + +class FizzFoo { + public: + ~FizzFoo(); + private: + Future<int> Fut; +}; + +} // namespace async + +// Warning is still emitted if there are type aliases. +namespace a { +template<typename T> +using Future = async::Future<T>; +} // namespace a + +void releaseUnits(); +struct Units { + ~Units() { + releaseUnits(); + } +}; +a::Future<Units> acquireUnits(); + +template<typename T> +T qux(T Generic) { + async::Future<Units> PendingA = acquireUnits(); + auto PendingB = acquireUnits(); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: unused local variable 'PendingB' of type 'a::Future<Units>' (aka 'Future<Units>') [bugprone-unused-local-non-trivial-variable] + async::Future<Units> MustBeUsed; + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: unused local variable 'MustBeUsed' of type 'async::Future<Units>' [bugprone-unused-local-non-trivial-variable] + PendingA.get(); + async::Future<T> TemplateType; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: unused local variable 'TemplateType' of type 'async::Future<T>' [bugprone-unused-local-non-trivial-variable] + a::Future<T> AliasTemplateType; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: unused local variable 'AliasTemplateType' of type 'a::Future<T>' (aka 'Future<type-parameter-0-0>') [bugprone-unused-local-non-trivial-variable] + return Generic; +} + +async::Future<int> Global; + +int bar(int Num) { + a::Future<Units> PendingA = acquireUnits(); + a::Future<Units> PendingB = acquireUnits(); // not used at all, unused variable not fired because of destructor side effect + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: unused local variable 'PendingB' of type 'a::Future<Units>' (aka 'Future<Units>') [bugprone-unused-local-non-trivial-variable] + auto Num2 = PendingA.get(); + auto Num3 = qux(Num); + async::Ptr<a::Future<Units>> Shared = async::Ptr<a::Future<Units>>(acquireUnits()); + static auto UnusedStatic = async::Future<Units>(); + thread_local async::Future<Units> UnusedThreadLocal; + auto Captured = acquireUnits(); + Num3 += [Captured]() { + return 1; + }(); + a::Future<Units> Referenced = acquireUnits(); + a::Future<Units>* Pointer = &Referenced; + a::Future<Units>& Reference = Referenced; + const a::Future<Units>& ConstReference = Referenced; + try { + } catch (a::Future<Units> Fut) { + } + struct Holder { + a::Future<Units> Fut; + }; + Holder H; + auto [fut] = H; + return Num * Num3; +} + +void exclusion() { + async::FizzFoo A; + async::FooBar B; + async::FooQux C; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unused local variable 'C' of type 'async::FooQux' [bugprone-unused-local-non-trivial-variable] +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits