Skip to content

[clang-tidy] introduce a unused local non trival variable check #76101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ add_clang_library(clangTidyBugproneModule
UnhandledSelfAssignmentCheck.cpp
UniquePtrArrayMismatchCheck.cpp
UnsafeFunctionsCheck.cpp
UnusedLocalNonTrivialVariableCheck.cpp
UnusedRaiiCheck.cpp
UnusedReturnValueCheck.cpp
UseAfterMoveCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -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]
}