Bugprone-default-lambda-capture

on-behalf-of: @amd <[email protected]>

Author: jjmarr-amd

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>(

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

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

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

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
 ^^^^^^^^^^^^^^^^^
 

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: ``[]``

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>`,

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>();
+}