[clang-tidy] add check for capturing lambda coroutines

[PiotrZSL]

Summary

Signed-off-by: Noah Watkins <[e-mail]>

[ChuanqiXu]

This is helpful. I feel it is OK to add the warning in the clang too. Are you interested in that?

[dotnwat]

In D137514#3911046, @ChuanqiXu wrote:

This is helpful. I feel it is OK to add the warning in the clang too. Are you interested in that?

Yes, but unfortunately I'm very time constrained until the end of the year. Is that something that would need to be added to this diff, or could be separate? (fyi, brand new to clang development)

[ChuanqiXu]

In D137514#3912626, @dotnwat wrote:

In D137514#3911046, @ChuanqiXu wrote:

This is helpful. I feel it is OK to add the warning in the clang too. Are you interested in that?

Yes, but unfortunately I'm very time constrained until the end of the year. Is that something that would need to be added to this diff, or could be separate? (fyi, brand new to clang development)

Oh, I am not blocking this patch and this patch looks good to me and self-contained. That's just a simple suggestion.

[carlosgalvezp]

Looking good! Some minor comments

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp

41

Perhaps it would be good with a diagnostic that gives more actionable feedback? For example "avoid capturing coroutines in a lambda"

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h

19

Start the documentation describing "what" the check warns for (here you explain the "why"). Make sure this text is in sync with the .rst documentation.

29

Limit to C++20 using isLanguageVersionSupported

clang-tools-extra/docs/ReleaseNotes.rst

126

C++ Core Guidelines

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-capturing-lambda-coroutines.rst

23 ↗

(On Diff #473522)

Mention that the check implements rule CP.51 of C++ Core Guidelines, and add a link to the that guideline (see other C++ Core Guidelines checks as an example).

24 ↗

(On Diff #473522)

does

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

182

Remove, since this check does not provide fix-its.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-capturing-lambda-coroutines.cpp

1–2 ↗

(On Diff #473522)

I think it's easier to read and reason about if you keep it in one line.

2 ↗

(On Diff #473522)

It's strange to have this test depend on input data from another module - create a Inputs folder inside the cppcoreguidelines folder and copy coroutines.h there.

[carlosgalvezp]

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp

41

I meant to remove this comment since it's rather nitpick, please ignore :)

[ChuanqiXu]

Is there any new progress on this?

[PiotrZSL]

Rebase + Move files to other locations due to structure change

[PiotrZSL]

Making tests passing on this diff

[ChuanqiXu]

LGTM. Thanks! BTW, in such cases, you can commandeer the revision generally.

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h

+//===--- AvoidCapturingLambdaCoroutinesCheck.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_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// The normal usage of captures in lambdas are problematic when the lambda is a
+/// coroutine because the captures are destroyed after the first suspension
+/// point. Using the captures after this point is a use-after-free issue.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-capturing-lambda-coroutines.html
+class AvoidCapturingLambdaCoroutinesCheck : public ClangTidyCheck {
+public:
+  AvoidCapturingLambdaCoroutinesCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp

+//===--- AvoidCapturingLambdaCoroutinesCheck.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 "AvoidCapturingLambdaCoroutinesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void AvoidCapturingLambdaCoroutinesCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(lambdaExpr().bind("lambda"), this);
+}
+
+void AvoidCapturingLambdaCoroutinesCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+  if (!Lambda) {
+    return;
+  }
+
+  const auto *Body = dyn_cast<CoroutineBodyStmt>(Lambda->getBody());
+  if (!Body) {
+    return;
+  }
+
+  if (Lambda->captures().empty()) {
+    return;
+  }
+
+  diag(Lambda->getBeginLoc(), "found capturing coroutine lambda");
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

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

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   AvoidCaptureDefaultWhenCapturingThisCheck.cpp
+  AvoidCapturingLambdaCoroutinesCheck.cpp
   AvoidConstOrRefDataMembersCheck.cpp
   AvoidDoWhileCheck.cpp
   AvoidGotoCheck.cpp
   AvoidNonConstGlobalVariablesCheck.cpp
   AvoidReferenceCoroutineParametersCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InitVariablesCheck.cpp
   InterfacesGlobalInitCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidCaptureDefaultWhenCapturingThisCheck.h"
+#include "AvoidCapturingLambdaCoroutinesCheck.h"
 #include "AvoidConstOrRefDataMembersCheck.h"
 #include "AvoidDoWhileCheck.h"
 #include "AvoidGotoCheck.h"
 #include "AvoidNonConstGlobalVariablesCheck.h"
 #include "AvoidReferenceCoroutineParametersCheck.h"
 #include "InitVariablesCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "MacroUsageCheck.h"

 namespace cppcoreguidelines {
 
 /// A module containing checks of the C++ Core Guidelines
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidCaptureDefaultWhenCapturingThisCheck>(
         "cppcoreguidelines-avoid-capture-default-when-capturing-this");
+    CheckFactories.registerCheck<AvoidCapturingLambdaCoroutinesCheck>(
+        "cppcoreguidelines-avoid-capturing-lambda-coroutines");
     CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
         "cppcoreguidelines-avoid-c-arrays");
     CheckFactories.registerCheck<AvoidConstOrRefDataMembersCheck>(
         "cppcoreguidelines-avoid-const-or-ref-data-members");
     CheckFactories.registerCheck<AvoidDoWhileCheck>(
         "cppcoreguidelines-avoid-do-while");
     CheckFactories.registerCheck<AvoidGotoCheck>(
         "cppcoreguidelines-avoid-goto");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`cppcoreguidelines-avoid-capture-default-when-capturing-this
   Warns when lambda specify a capture default and capture ``this``.
 
 - New :doc:`llvmlibc-inline-function-decl
   <clang-tidy/checks/llvmlibc/inline-function-decl>` check.
 
   Checks that all implicit and explicit inline functions in header files are
   tagged with the ``LIBC_INLINE`` macro.
 
+- New :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines
+  <clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines>` check.
+
+  Adds check for cpp core guideline: "CP.51: Do not use capturing lambdas that
+  are coroutines."
+
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-msc24-c
   <clang-tidy/checks/cert/msc24-c>` to :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` was added.
 
 - New alias :doc:`cert-msc33-c

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst

+.. title:: clang-tidy - cppcoreguidelines-avoid-capturing-lambda-coroutines
+
+cppcoreguidelines-avoid-capturing-lambda-coroutines
+===================================================
+
+Warns if a capturing lambda is a coroutine. For example:
+
+.. code-block:: c++
+
+   int c;
+
+   [c] () -> task { co_return; };
+   [&] () -> task { int y = c; co_return; };
+   [=] () -> task { int y = c; co_return; };
+
+   struct s {
+       void m() {
+           [this] () -> task { co_return; };
+       }
+   };
+
+All of the cases above will trigger the warning. However, implicit captures
+do not trigger the warning unless the body of the lambda uses the capture.
+For example, the following do not trigger the warning.
+
+.. code-block:: c++
+
+   int c;
+
+   [&] () -> task { co_return; };
+   [=] () -> task { co_return; };

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

.. csv-table::
    `clang-analyzer-osx.cocoa.MissingSuperCall <clang-analyzer/osx.cocoa.MissingSuperCall.html>`_,
    `clang-analyzer-osx.cocoa.NonNilReturnValue <clang-analyzer/osx.cocoa.NonNilReturnValue.html>`_,
    `clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak <clang-analyzer/osx.cocoa.RunLoopAutoreleaseLeak.html>`_,
    `clang-analyzer-valist.CopyToSelf <clang-analyzer/valist.CopyToSelf.html>`_,
    `clang-analyzer-valist.Uninitialized <clang-analyzer/valist.Uninitialized.html>`_,
    `clang-analyzer-valist.Unterminated <clang-analyzer/valist.Unterminated.html>`_,
    `concurrency-mt-unsafe <concurrency/mt-unsafe.html>`_,
    `concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous.html>`_,
    `cppcoreguidelines-avoid-capture-default-when-capturing-this <cppcoreguidelines/avoid-capture-default-when-capturing-this.html>`_,
+   `cppcoreguidelines-avoid-capturing-lambda-coroutines <cppcoreguidelines/avoid-capturing-lambda-coroutines.html>`_, "Yes"
    `cppcoreguidelines-avoid-const-or-ref-data-members <cppcoreguidelines/avoid-const-or-ref-data-members.html>`_,
    `cppcoreguidelines-avoid-do-while <cppcoreguidelines/avoid-do-while.html>`_,
    `cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto.html>`_,
    `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines/avoid-non-const-global-variables.html>`_,
    `cppcoreguidelines-avoid-reference-coroutine-parameters <cppcoreguidelines/avoid-reference-coroutine-parameters.html>`_,
    `cppcoreguidelines-init-variables <cppcoreguidelines/init-variables.html>`_, "Yes"
    `cppcoreguidelines-interfaces-global-init <cppcoreguidelines/interfaces-global-init.html>`_,
    `cppcoreguidelines-macro-usage <cppcoreguidelines/macro-usage.html>`_,

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp

+// RUN: %check_clang_tidy -std=c++20-or-later %s cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- \
+// RUN:   -isystem %S/../readability/Inputs/identifier-naming/system
+
+#include <coroutines.h>
+
+void Caught() {
+    int v;
+
+    [&] () -> task { int y = v; co_return; };
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+    [=] () -> task { int y = v; co_return; };
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+    [v] () -> task { co_return; };
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+    [&v] () -> task { co_return; };
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+    [y=v] () -> task { co_return; };
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+    [y{v}] () -> task { co_return; };
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+}
+
+struct S {
+    void m() {
+        [this] () -> task { co_return; };
+        // CHECK-MESSAGES: [[@LINE-1]]:9: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+    }
+};
+
+void Safe() {
+    int v;
+    [] () -> task { co_return; };
+    [&] () -> task { co_return; };
+    [=] () -> task { co_return; };
+}