[clang-tools-extra] [clang-tidy] introduce a must use check (PR #76101)

Tyler Rockwood via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 21 14:31:59 PST 2023


https://github.com/rockwotj updated https://github.com/llvm/llvm-project/pull/76101

>From 5afeaab9f148b10d951e37fd27cb32687f310a9c Mon Sep 17 00:00:00 2001
From: Tyler Rockwood <rockwood at redpanda.com>
Date: Thu, 21 Dec 2023 16:31:12 -0600
Subject: [PATCH] clang-tidy/bugprone: introduce
 unused-local-non-trivial-variable check

Signed-off-by: Tyler Rockwood <rockwood at redpanda.com>
---
 .../bugprone/BugproneTidyModule.cpp           |  3 +
 .../clang-tidy/bugprone/CMakeLists.txt        |  1 +
 .../UnusedLocalNonTrivialVariableCheck.cpp    | 89 +++++++++++++++++++
 .../UnusedLocalNonTrivialVariableCheck.h      | 44 +++++++++
 clang-tools-extra/docs/ReleaseNotes.rst       | 13 +++
 .../unused-local-non-trivial-variable.rst     | 29 ++++++
 .../docs/clang-tidy/checks/list.rst           |  1 +
 .../unused-local-non-trivial-variable.cpp     | 79 ++++++++++++++++
 8 files changed, 259 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.h
 create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-local-non-trivial-variable.rst
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp

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..2c61078e1d1cc1
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedLocalNonTrivialVariableCheck.cpp
@@ -0,0 +1,89 @@
+//===--- 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";
+
+AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); }
+AST_MATCHER(VarDecl, isReferenced) { return Node.isReferenced(); }
+AST_MATCHER(Type, isReferenceType) { return Node.isReferenceType(); }
+} // namespace
+
+UnusedLocalNonTrivialVariableCheck::UnusedLocalNonTrivialVariableCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IncludeTypeRegex(utils::options::parseStringList(
+          Options.get("IncludeTypeRegex", DefaultIncludeTypeRegex))),
+      ExcludeTypeRegex(utils::options::parseStringList(
+          Options.get("ExcludeTypeRegex", ""))) {}
+
+void UnusedLocalNonTrivialVariableCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeTypeRegex",
+                utils::options::serializeStringList(IncludeTypeRegex));
+  Options.store(Opts, "ExcludeTypeRegex",
+                utils::options::serializeStringList(ExcludeTypeRegex));
+}
+
+void UnusedLocalNonTrivialVariableCheck::registerMatchers(MatchFinder *Finder) {
+  if (IncludeTypeRegex.empty())
+    return;
+
+  Finder->addMatcher(
+      varDecl(
+          isLocalVarDecl(), unless(isReferenced()),
+          unless(isExpansionInSystemHeader()), unless(isImplicit()),
+          unless(isExceptionVariable()), hasLocalStorage(), isDefinition(),
+          unless(hasType(isReferenceType())),
+          hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+              recordDecl(matchesAnyListedName(IncludeTypeRegex)))))),
+          unless(hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+              recordDecl(matchesAnyListedName(ExcludeTypeRegex))))))))
+          .bind("var"),
+      this);
+}
+
+void UnusedLocalNonTrivialVariableCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("var");
+  const auto Type = MatchedDecl->getType();
+
+  if (Type.isTrivialType(*Result.Context) ||
+      Type.isTriviallyCopyableType(*Result.Context))
+    return;
+
+  diag(MatchedDecl->getLocation(), "unused local variable %0 of type %1")
+      << MatchedDecl << Type;
+}
+
+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..5e7778d229aa82
--- /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:
+///   - 'IncludeTypeRegex': a semicolon-separated list of regular expressions
+///                         matching types to ensure must be used.
+///   - 'ExcludeTypeRegex': a semicolon-separated list of regular expressions
+///                         matching types that are excluded from the
+///                         'IncludeTypeRegex' 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> IncludeTypeRegex;
+  const std::vector<StringRef> ExcludeTypeRegex;
+};
+
+} // 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..2e0a9f9aeae75e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -168,6 +168,19 @@ 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. By
+  default std::.*mutex and std::future are included.
+  
+  The check supports these options:
+    - 'IncludeTypeRegex': a semicolon-separated list of regular expressions
+                          matching types to ensure must be used.
+    - 'ExcludeTypeRegex': a semicolon-separated list of regular expressions
+                          matching types that are excluded from the
+                          'IncludeTypeRegex' matches.
+
 - 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..59c50419657ec6
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-local-non-trivial-variable.rst
@@ -0,0 +1,29 @@
+.. 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.
+
+In the following 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:: IncludeTypeRegex
+
+   Semicolon-separated list of regular expressions matching types of variables to check.
+   By default it 'std::.*mutex;std::future'.
+
+.. option:: ExcludeTypeRegex
+
+   A semicolon-separated list of regular expressions matching types that are excluded from the
+  'IncludeTypeRegex' 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..65b23defd73fa0 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>`, "Yes"
    :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..c6e45378b9c3e2
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s bugprone-unused-local-non-trivial-variable %t -- \
+// RUN:       -config="{CheckOptions: {bugprone-unused-local-non-trivial-variable.IncludeTypeRegex: '::async::Future'}}"
+
+
+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;
+};
+
+
+} // 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();
+    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;
+    return Num * Num3;
+}



More information about the cfe-commits mailing list