-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Bugprone-default-lambda-capture #158747
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
Bugprone-default-lambda-capture #158747
Conversation
on-behalf-of: @amd [email protected]
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
be253b6
to
bf8c6bc
Compare
Not actually ready for main yet. |
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: JJ Marr (jjmarr-amd) Changeson-behalf-of: @AMD <[email protected]> Full diff: https://github.com/llvm/llvm-project/pull/158747.diff 8 Files Affected:
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<DanglingHandleCheck>(
"bugprone-dangling-handle");
+ CheckFactories.registerCheck<DefaultLambdaCaptureCheck>(
+ "bugprone-default-lambda-capture");
CheckFactories.registerCheck<DynamicStaticInitializersCheck>(
"bugprone-dynamic-static-initializers");
CheckFactories.registerCheck<EasilySwappableParametersCheck>(
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<LambdaExpr>("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<TraversalKind> 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
+ <clang-tidy/checks/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 <bugprone/copy-constructor-init>`, "Yes"
:doc:`bugprone-crtp-constructor-accessibility <bugprone/crtp-constructor-accessibility>`, "Yes"
:doc:`bugprone-dangling-handle <bugprone/dangling-handle>`,
+ :doc:`bugprone-default-lambda-capture <bugprone/default-lambda-capture>`,
:doc:`bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers>`,
:doc:`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters>`,
:doc:`bugprone-empty-catch <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<typename T>
+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<int>();
+ test_template_lambdas<double>();
+}
|
on-behalf-of: @amd [email protected]