diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 491de6acea2b7..db99d57c511b8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -23,6 +23,7 @@ #include "CopyConstructorInitCheck.h" #include "CrtpConstructorAccessibilityCheck.h" #include "DanglingHandleCheck.h" +#include "DefaultLambdaCaptureCheck.h" #include "DynamicStaticInitializersCheck.h" #include "EasilySwappableParametersCheck.h" #include "EmptyCatchCheck.h" @@ -134,6 +135,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-copy-constructor-init"); CheckFactories.registerCheck( "bugprone-dangling-handle"); + CheckFactories.registerCheck( + "bugprone-default-lambda-capture"); CheckFactories.registerCheck( "bugprone-dynamic-static-initializers"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 46bc8efd44bc5..66125c9d22c1c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -19,6 +19,7 @@ add_clang_library(clangTidyBugproneModule STATIC CopyConstructorInitCheck.cpp CrtpConstructorAccessibilityCheck.cpp DanglingHandleCheck.cpp + DefaultLambdaCaptureCheck.cpp DynamicStaticInitializersCheck.cpp EasilySwappableParametersCheck.cpp EmptyCatchCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/DefaultLambdaCaptureCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/DefaultLambdaCaptureCheck.cpp new file mode 100644 index 0000000000000..6c95fbc984c5a --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/DefaultLambdaCaptureCheck.cpp @@ -0,0 +1,35 @@ +#include "DefaultLambdaCaptureCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +void DefaultLambdaCaptureCheck::registerMatchers(MatchFinder *Finder) { + // Match any lambda expression + Finder->addMatcher(lambdaExpr().bind("lambda"), this); +} + +void DefaultLambdaCaptureCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Lambda = Result.Nodes.getNodeAs("lambda"); + if (!Lambda) + return; + + // Check if lambda has a default capture + if (Lambda->getCaptureDefault() == LCD_None) + return; + + SourceLocation DefaultCaptureLoc = Lambda->getCaptureDefaultLoc(); + if (DefaultCaptureLoc.isInvalid()) + return; + + const char *CaptureKind = + (Lambda->getCaptureDefault() == LCD_ByCopy) ? "by-copy" : "by-reference"; + + diag(DefaultCaptureLoc, "lambda %0 default capture is discouraged; " + "prefer to capture specific variables explicitly") + << CaptureKind; +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/DefaultLambdaCaptureCheck.h b/clang-tools-extra/clang-tidy/bugprone/DefaultLambdaCaptureCheck.h new file mode 100644 index 0000000000000..ac47c866cfccd --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/DefaultLambdaCaptureCheck.h @@ -0,0 +1,26 @@ +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DEFAULT_LAMBDA_CAPTURE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DEFAULT_LAMBDA_CAPTURE_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/** Flags lambdas that use default capture modes + * + * For the user-facing documentation see: + * http://clang.llvm.org/extra/clang-tidy/checks/bugprone/default-lambda-capture.html + */ +class DefaultLambdaCaptureCheck : public ClangTidyCheck { +public: + DefaultLambdaCaptureCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DEFAULT_LAMBDA_CAPTURE_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7cdff86beeec6..30d0bffb78f39 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -194,6 +194,15 @@ New checks Finds virtual function overrides with different visibility than the function in the base class. +- New :doc:`bugprone-default-lambda-capture + ` check. + + Finds lambda expressions that use default capture modes (``[=]`` or ``[&]``) + and suggests using explicit captures instead. Default captures can lead to + subtle bugs including dangling references with ``[&]``, unnecessary copies + with ``[=]``, and make code less maintainable by hiding which variables are + actually being captured. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/default-lambda-capture.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/default-lambda-capture.rst new file mode 100644 index 0000000000000..026acf9f1894b --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/default-lambda-capture.rst @@ -0,0 +1,42 @@ +.. title:: clang-tidy - bugprone-default-lambda-capture + +bugprone-default-lambda-capture +=============================== + +Warns when lambda expressions use default capture modes (``[=]`` or ``[&]``) +instead of explicitly capturing specific variables. Default captures can make +code less readable and more error-prone by making it unclear which variables +are being captured and how. + +Implements Item 31 of Effective Modern C++ by Scott Meyers. + +Example +------- + +.. code-block:: c++ + + void example() { + int x = 1; + int y = 2; + + // Bad - default capture by copy + auto lambda1 = [=]() { return x + y; }; + + // Bad - default capture by reference + auto lambda2 = [&]() { return x + y; }; + + // Good - explicit captures + auto lambda3 = [x, y]() { return x + y; }; + auto lambda4 = [&x, &y]() { return x + y; }; + } + +The check will warn on: + +- Default capture by copy: ``[=]`` +- Default capture by reference: ``[&]`` +- Mixed captures with defaults: ``[=, &x]`` or ``[&, x]`` + +The check will not warn on: + +- Explicit captures: ``[x]``, ``[&x]``, ``[x, y]``, ``[this]`` +- Empty capture lists: ``[]`` diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c490d2ece2e0a..cb6254ee36f42 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -91,6 +91,7 @@ Clang-Tidy Checks :doc:`bugprone-copy-constructor-init `, "Yes" :doc:`bugprone-crtp-constructor-accessibility `, "Yes" :doc:`bugprone-dangling-handle `, + :doc:`bugprone-default-lambda-capture `, :doc:`bugprone-dynamic-static-initializers `, :doc:`bugprone-easily-swappable-parameters `, :doc:`bugprone-empty-catch `, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/default-lambda-capture.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/default-lambda-capture.cpp new file mode 100644 index 0000000000000..f820b4c9b8a0e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/default-lambda-capture.cpp @@ -0,0 +1,98 @@ +// RUN: %check_clang_tidy %s bugprone-default-lambda-capture %t + +void test_default_captures() { + int value = 42; + int another = 10; + + auto lambda1 = [=](int x) { return value + x; }; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda by-copy default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + auto lambda2 = [&](int x) { return value + x; }; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda by-reference default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + auto lambda3 = [=, &another](int x) { return value + another + x; }; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda by-copy default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + auto lambda4 = [&, value](int x) { return value + another + x; }; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda by-reference default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] +} + +void test_acceptable_captures() { + int value = 42; + int another = 10; + + auto lambda1 = [value](int x) { return value + x; }; + auto lambda2 = [&value](int x) { return value + x; }; + auto lambda3 = [value, another](int x) { return value + another + x; }; + auto lambda4 = [&value, &another](int x) { return value + another + x; }; + + auto lambda5 = [](int x, int y) { return x + y; }; + + struct S { + int member = 5; + void foo() { + auto lambda = [this]() { return member; }; + } + }; +} + +void test_nested_lambdas() { + int outer_var = 1; + int middle_var = 2; + int inner_var = 3; + + auto outer = [=]() { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: lambda by-copy default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + auto inner = [&](int x) { return outer_var + middle_var + inner_var + x; }; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda by-reference default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + return inner(10); + }; +} + +void test_lambda_returns() { + int a = 1, b = 2, c = 3; + + auto create_adder = [=](int x) { + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: lambda by-copy default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + return [x](int y) { return x + y; }; // Inner lambda is fine - explicit capture + }; + + auto func1 = [&]() { return a; }; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: lambda by-reference default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + auto func2 = [=]() { return b; }; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: lambda by-copy default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] +} + +class TestClass { + int member = 42; + +public: + void test_member_function_lambdas() { + int local = 10; + + auto lambda1 = [=]() { return member + local; }; + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: lambda by-copy default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + auto lambda2 = [&]() { return member + local; }; + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: lambda by-reference default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] + + auto lambda3 = [this, local]() { return member + local; }; + auto lambda4 = [this, &local]() { return member + local; }; + } +}; + +template +void test_template_lambdas() { + T value{}; + + auto lambda = [=](T x) { return value + x; }; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: lambda by-copy default capture is discouraged; prefer to capture specific variables explicitly [bugprone-default-lambda-capture] +} + +void instantiate_templates() { + test_template_lambdas(); + test_template_lambdas(); +}