[clang-tidy] Add readability-avoid-default-lambda-capture

clang-tools-extra/clang-tidy/readability/AvoidDefaultLambdaCaptureCheck.cpp

@@ -0,0 +1,94 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "AvoidDefaultLambdaCaptureCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/Lambda.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+AST_MATCHER(LambdaExpr, hasDefaultCapture) {
+  return Node.getCaptureDefault() != LCD_None;
+}
+
+std::optional<std::string>
+generateImplicitCaptureText(const LambdaCapture &Capture) {
+  if (Capture.capturesThis()) {
+    return Capture.getCaptureKind() == LCK_StarThis ? "*this" : "this";
+  }
+
+  if (Capture.capturesVariable()) {
+    std::string Result;
+    if (Capture.getCaptureKind() == LCK_ByRef) {
+      Result += "&";
+    }
+    Result += Capture.getCapturedVar()->getName().str();
+    return Result;
+  }
+
+  if (Capture.capturesVLAType()) {
+    // VLA captures are rare and complex - for now we skip them
+    // A full implementation would need to handle the VLA type properly
+    return std::nullopt;
+  }
+
+  return std::nullopt;
+}
+
+} // namespace
+
+void AvoidDefaultLambdaCaptureCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(lambdaExpr(hasDefaultCapture()).bind("lambda"), this);
+}
+
+void AvoidDefaultLambdaCaptureCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+  assert(Lambda);
+
+  const SourceLocation DefaultCaptureLoc = Lambda->getCaptureDefaultLoc();
+  if (DefaultCaptureLoc.isInvalid())
+    return;
+
+  auto Diag = diag(DefaultCaptureLoc,
+                   "lambda default captures are discouraged; "
+                   "prefer to capture specific variables explicitly");
+
+  std::vector<std::string> AllCaptures;
+
+  for (const auto &Capture : Lambda->explicit_captures()) {
+    if (const auto CaptureText = generateImplicitCaptureText(Capture)) {
+      AllCaptures.push_back(CaptureText.value());
+    }
+  }
+
+  for (const auto &Capture : Lambda->implicit_captures()) {
+    if (const auto CaptureText = generateImplicitCaptureText(Capture)) {
+      AllCaptures.push_back(CaptureText.value());
+    }
+  }
+
+  // Replace with new capture list
+  std::string ReplacementText;
+  if (AllCaptures.empty()) {
+    ReplacementText = "[]";
+  } else {
+    ReplacementText = "[" + llvm::join(AllCaptures, ", ") + "]";
+  }
+
+  SourceRange IntroducerRange = Lambda->getIntroducerRange();
+  if (IntroducerRange.isValid()) {
+    Diag << FixItHint::CreateReplacement(IntroducerRange, ReplacementText);
+  }
+}
+
+} // namespace clang::tidy::readability

clang-tools-extra/clang-tidy/readability/AvoidDefaultLambdaCaptureCheck.h

@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_READABILITY_AVOIDDEFAULTLAMBDACAPTURECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDDEFAULTLAMBDACAPTURECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Flags lambdas that use default capture modes
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-default-lambda-capture.html
+class AvoidDefaultLambdaCaptureCheck : public ClangTidyCheck {
+public:
+  AvoidDefaultLambdaCaptureCheck(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::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDDEFAULTLAMBDACAPTURECHECK_H

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
 add_clang_library(clangTidyReadabilityModule STATIC
   AmbiguousSmartptrResetCallCheck.cpp
   AvoidConstParamsInDecls.cpp
+  AvoidDefaultLambdaCaptureCheck.cpp
   AvoidNestedConditionalOperatorCheck.cpp
   AvoidReturnWithVoidValueCheck.cpp
   AvoidUnconditionalPreprocessorIfCheck.cpp

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "AmbiguousSmartptrResetCallCheck.h"
 #include "AvoidConstParamsInDecls.h"
+#include "AvoidDefaultLambdaCaptureCheck.h"
 #include "AvoidNestedConditionalOperatorCheck.h"
 #include "AvoidReturnWithVoidValueCheck.h"
 #include "AvoidUnconditionalPreprocessorIfCheck.h"
@@ -93,6 +94,8 @@ class ReadabilityModule : public ClangTidyModule {
         "readability-container-size-empty");
     CheckFactories.registerCheck<ConvertMemberFunctionsToStatic>(
         "readability-convert-member-functions-to-static");
+    CheckFactories.registerCheck<AvoidDefaultLambdaCaptureCheck>(
+        "readability-avoid-default-lambda-capture");
     CheckFactories.registerCheck<DeleteNullPointerCheck>(
         "readability-delete-null-pointer");
     CheckFactories.registerCheck<DuplicateIncludeCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -203,6 +203,11 @@ New checks
   Finds virtual function overrides with different visibility than the function
   in the base class.
 
+- New :doc:`readability-avoid-default-lambda-capture
+  <clang-tidy/checks/readability/avoid-default-lambda-capture>` check.
+
+  Warns on default lambda captures (e.g. ``[&](){ ... }``, ``[=](){ ... }``).
+
 - New :doc:`readability-redundant-parentheses
   <clang-tidy/checks/readability/redundant-parentheses>` check.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -377,6 +377,7 @@ Clang-Tidy Checks
    :doc:`readability-container-data-pointer <readability/container-data-pointer>`, "Yes"
    :doc:`readability-container-size-empty <readability/container-size-empty>`, "Yes"
    :doc:`readability-convert-member-functions-to-static <readability/convert-member-functions-to-static>`, "Yes"
+   :doc:`readability-avoid-default-lambda-capture <readability/avoid-default-lambda-capture>`,
    :doc:`readability-delete-null-pointer <readability/delete-null-pointer>`, "Yes"
    :doc:`readability-duplicate-include <readability/duplicate-include>`, "Yes"
    :doc:`readability-else-after-return <readability/else-after-return>`, "Yes"

clang-tools-extra/docs/clang-tidy/checks/readability/avoid-default-lambda-capture.rst

@@ -0,0 +1,53 @@
+.. title:: clang-tidy - readability-avoid-default-lambda-capture
+
+readability-avoid-default-lambda-capture
+========================================
+
+Warns on default lambda captures (e.g. ``[&](){ ... }``, ``[=](){ ... }``).
+
+Captures can lead to subtle bugs including dangling references and unnecessary
+copies. Writing out the name of the variables being captured reminds programmers
+and reviewers to know what is being captured. And knowing is half the battle.
+
+Coding guidelines that recommend against defaulted lambda captures include:
+
+* Item 31 of Effective Modern C++ by Scott Meyers
+
+Example
+-------
+
+.. code-block:: c++
+
+  #include <iostream>
+
+  class Widget {
+    std::vector<std::function<void(int)>> callbacks;
+    int                                   widgetId;
+    void addCallback(int factoryId) {
+      callbacks.emplace_back(
+        [&](){
+          std::cout << "Widget " << widgetId << " made in factory " << factoryId;
+        }
+      );
+    }
+  }
+
+When ``callbacks`` is executed, ``factoryId`` will dangle. Writing the name of
+``factoryId`` in the capture list reminds the reader that it is being captured,
+which will hopefully lead to the bug being fixed during code review.
+
+.. code-block:: c++
+
+  #include <iostream>
+
+  class Widget {
+    std::vector<std::function<void(int)>> callbacks;
+    int                                   widgetId;
+    void addCallback(int factoryId) {
+      callbacks.emplace_back(
+        [&factoryId, &widgetId](){ // Why isn't factoryId captured by value??
+          std::cout << "Widget " << widgetId << " made in factory " << factoryId;
+        }
+      );
+    }
+  }

clang-tools-extra/test/clang-tidy/checkers/readability/avoid-default-lambda-capture.cpp

@@ -0,0 +1,110 @@
+// RUN: %check_clang_tidy %s readability-avoid-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 default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+  // CHECK-FIXES: auto lambda1 = [value](int x) { return value + x; };
+
+  auto lambda2 = [&](int x) { return value + x; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+  // CHECK-FIXES: auto lambda2 = [&value](int x) { return value + x; };
+
+  auto lambda3 = [=, &another](int x) { return value + another + x; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+  // CHECK-FIXES: auto lambda3 = [&another, value](int x) { return value + another + x; };
+
+  auto lambda4 = [&, value](int x) { return value + another + x; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+  // CHECK-FIXES: auto lambda4 = [value, &another](int x) { return value + another + x; };
+}
+
+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 default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+    // CHECK-FIXES: auto outer = [outer_var, middle_var, inner_var]() {
+
+    auto inner = [&](int x) { return outer_var + middle_var + inner_var + x; };
+    // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: lambda default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+    // CHECK-FIXES: auto inner = [&outer_var, &middle_var, &inner_var](int x) { return outer_var + middle_var + inner_var + x; };
+
+    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 default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+    // CHECK-FIXES: auto create_adder = [](int x) {
+    return [x](int y) { return x + y; }; // Inner lambda is fine - explicit capture
+  };
+
+  auto func1 = [&]() { return a; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: lambda default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+  // CHECK-FIXES: auto func1 = [&a]() { return a; };
+
+  auto func2 = [=]() { return b; };
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: lambda default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+  // CHECK-FIXES: auto func2 = [b]() { return b; };
+}
+
+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 default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+    // CHECK-FIXES: auto lambda1 = [this, local]() { return member + local; };
+
+    auto lambda2 = [&]() { return member + local; };
+    // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: lambda default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+    // CHECK-FIXES: auto lambda2 = [this, &local]() { return member + local; };
+
+    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 default captures are discouraged; prefer to capture specific variables explicitly [readability-avoid-default-lambda-capture]
+  // CHECK-FIXES: auto lambda = [](T x) { return value + x; };
+}
+
+void instantiate_templates() {
+  test_template_lambdas<int>();
+  test_template_lambdas<double>();
+}