[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 12 09:30:08 PDT 2025
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/130297
>From 8ef214f6c78d710dbd9c74b06c7c637baf93e527 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 8 Mar 2025 00:03:39 +0800
Subject: [PATCH 1/8] [clang-tidy] Add new check bugprone-capture-this-by-field
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
.../bugprone/CaptureThisByFieldCheck.cpp | 99 +++++++++++++
.../bugprone/CaptureThisByFieldCheck.h | 39 +++++
clang-tools-extra/docs/ReleaseNotes.rst | 6 +
.../checks/bugprone/capture-this-by-field.rst | 28 ++++
.../docs/clang-tidy/checks/list.rst | 1 +
.../bugprone/capture-this-by-field.cpp | 133 ++++++++++++++++++
8 files changed, 310 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/capture-this-by-field.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 0a3376949b6e5..ee9fa5ef06c7a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -16,6 +16,7 @@
#include "BitwisePointerCastCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "BranchCloneCheck.h"
+#include "CaptureThisByFieldCheck.h"
#include "CastingThroughVoidCheck.h"
#include "ChainedComparisonCheck.h"
#include "ComparePointerToMemberVirtualFunctionCheck.h"
@@ -118,6 +119,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"bugprone-bool-pointer-implicit-conversion");
CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");
+ CheckFactories.registerCheck<CaptureThisByFieldCheck>(
+ "bugprone-capture-this-by-field");
CheckFactories.registerCheck<CastingThroughVoidCheck>(
"bugprone-casting-through-void");
CheckFactories.registerCheck<ChainedComparisonCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index dab139b77c770..4d1d50c4ea2a0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -12,6 +12,7 @@ add_clang_library(clangTidyBugproneModule STATIC
BoolPointerImplicitConversionCheck.cpp
BranchCloneCheck.cpp
BugproneTidyModule.cpp
+ CaptureThisByFieldCheck.cpp
CastingThroughVoidCheck.cpp
ChainedComparisonCheck.cpp
ComparePointerToMemberVirtualFunctionCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
new file mode 100644
index 0000000000000..1f0f68acf335f
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
@@ -0,0 +1,99 @@
+//===--- CaptureThisByFieldCheck.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 "CaptureThisByFieldCheck.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
+ // unresolved
+ if (Node.needsOverloadResolutionForCopyConstructor() &&
+ Node.needsImplicitCopyConstructor())
+ return false;
+ if (Node.needsOverloadResolutionForMoveConstructor() &&
+ Node.needsImplicitMoveConstructor())
+ return false;
+ if (Node.needsOverloadResolutionForCopyAssignment() &&
+ Node.needsImplicitCopyAssignment())
+ return false;
+ if (Node.needsOverloadResolutionForMoveAssignment() &&
+ Node.needsImplicitMoveAssignment())
+ return false;
+ // default but not deleted
+ if (Node.hasSimpleCopyConstructor())
+ return false;
+ if (Node.hasSimpleMoveConstructor())
+ return false;
+ if (Node.hasSimpleCopyAssignment())
+ return false;
+ if (Node.hasSimpleMoveAssignment())
+ return false;
+
+ for (CXXConstructorDecl const *C : Node.ctors()) {
+ if (C->isCopyOrMoveConstructor() && C->isDefaulted() && !C->isDeleted())
+ return false;
+ }
+ for (CXXMethodDecl const *M : Node.methods()) {
+ if (M->isCopyAssignmentOperator() && M->isDefaulted() && !M->isDeleted())
+ return false;
+ if (M->isMoveAssignmentOperator() && M->isDefaulted() && !M->isDeleted())
+ return false;
+ }
+ // FIXME: find ways to identifier correct handle capture this lambda
+ return true;
+}
+
+} // namespace
+
+void CaptureThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
+ auto IsStdFunctionField =
+ fieldDecl(hasType(cxxRecordDecl(hasName("::std::function"))))
+ .bind("field");
+ auto CaptureThis = lambdaCapture(anyOf(
+ // [this]
+ capturesThis(),
+ // [self = this]
+ capturesVar(varDecl(hasInitializer(cxxThisExpr())))));
+ auto IsInitWithLambda = cxxConstructExpr(hasArgument(
+ 0,
+ lambdaExpr(hasAnyCapture(CaptureThis.bind("capture"))).bind("lambda")));
+ Finder->addMatcher(
+ cxxRecordDecl(
+ has(cxxConstructorDecl(
+ unless(isCopyConstructor()), unless(isMoveConstructor()),
+ hasAnyConstructorInitializer(cxxCtorInitializer(
+ isMemberInitializer(), forField(IsStdFunctionField),
+ withInitializer(IsInitWithLambda))))),
+ unless(correctHandleCaptureThisLambda())),
+ this);
+}
+
+void CaptureThisByFieldCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Capture = Result.Nodes.getNodeAs<LambdaCapture>("capture");
+ const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+ const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field");
+ diag(Lambda->getBeginLoc(),
+ "using lambda expressions to capture this and storing it in class "
+ "member will cause potential variable lifetime issue when the class "
+ "instance is moved or copied")
+ << Capture->getLocation();
+ diag(Field->getLocation(),
+ "'std::function' that stores captured this and becomes invalid during "
+ "copying or moving",
+ DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
new file mode 100644
index 0000000000000..72c0a540a7743
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
@@ -0,0 +1,39 @@
+//===--- CaptureThisByFieldCheck.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_BUGPRONE_CAPTURETHISBYFIELDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURETHISBYFIELDCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include <optional>
+
+namespace clang::tidy::bugprone {
+
+/// Finds lambda captures that capture the ``this`` pointer and store it as class
+/// members without handle the copy and move constructors and the assignments.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capture-this-by-field.html
+class CaptureThisByFieldCheck : public ClangTidyCheck {
+public:
+ CaptureThisByFieldCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TraversalKind::TK_IgnoreUnlessSpelledInSource;
+ }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURETHISBYFIELDCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 71edb704b49d6..002f60dfb6fa8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`bugprone-capture-this-by-field
+ <clang-tidy/checks/bugprone/capture-this-by-field>` check.
+
+ Finds lambda captures that capture the ``this`` pointer and store it as class
+ members without handle the copy and move constructors and the assignments.
+
- New :doc:`bugprone-unintended-char-ostream-output
<clang-tidy/checks/bugprone/unintended-char-ostream-output>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capture-this-by-field.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capture-this-by-field.rst
new file mode 100644
index 0000000000000..c837ce5cb6d68
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capture-this-by-field.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - bugprone-capture-this-by-field
+
+bugprone-capture-this-by-field
+==============================
+
+Finds lambda captures that capture the ``this`` pointer and store it as class
+members without handle the copy and move constructors and the assignments.
+
+Capture this in a lambda and store it as a class member is dangerous because the
+lambda can outlive the object it captures. Especially when the object is copied
+or moved, the captured ``this`` pointer will be implicitly propagated to the
+new object. Most of the time, people will believe that the captured ``this``
+pointer points to the new object, which will lead to bugs.
+
+
+.. code-block:: c++
+
+ struct C {
+ C() : Captured([this]() -> C const * { return this; }) {}
+ std::function<C const *()> Captured;
+ };
+
+ void foo() {
+ C v1{};
+ C v2 = v1; // v2.Captured capture v1's this pointer
+ assert(v2.Captured() == v1.Captured()); // v2.Captured capture v1's this pointer
+ assert(v2.Captured() == &v2); // assertion failed.
+ }
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5f03ef72cc603..6bef286f28100 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -84,6 +84,7 @@ Clang-Tidy Checks
:doc:`bugprone-bitwise-pointer-cast <bugprone/bitwise-pointer-cast>`,
:doc:`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion>`, "Yes"
:doc:`bugprone-branch-clone <bugprone/branch-clone>`,
+ :doc:`bugprone-capture-this-by-field <bugprone/capture-this-by-field>`,
:doc:`bugprone-casting-through-void <bugprone/casting-through-void>`,
:doc:`bugprone-chained-comparison <bugprone/chained-comparison>`,
:doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
new file mode 100644
index 0000000000000..4ffd6c7ade51e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capture-this-by-field %t
+
+namespace std {
+
+template<class Fn>
+class function;
+
+template<class R, class ...Args>
+class function<R(Args...)> {
+public:
+ function() noexcept;
+ template<class F> function(F &&);
+};
+
+} // namespace std
+
+struct Basic {
+ Basic() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to capture this and storing it in class member
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
+
+struct AssignCapture {
+ AssignCapture() : Captured([Self = this]() { static_cast<void>(Self); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to capture this and storing it in class member
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
+
+struct DeleteMoveAndCopy {
+ DeleteMoveAndCopy() : Captured([this]() { static_cast<void>(this); }) {}
+ DeleteMoveAndCopy(DeleteMoveAndCopy const&) = delete;
+ DeleteMoveAndCopy(DeleteMoveAndCopy &&) = delete;
+ DeleteMoveAndCopy& operator=(DeleteMoveAndCopy const&) = delete;
+ DeleteMoveAndCopy& operator=(DeleteMoveAndCopy &&) = delete;
+ std::function<void()> Captured;
+};
+
+struct DeleteCopyImplicitDisabledMove {
+ DeleteCopyImplicitDisabledMove() : Captured([this]() { static_cast<void>(this); }) {}
+ DeleteCopyImplicitDisabledMove(DeleteCopyImplicitDisabledMove const&) = delete;
+ DeleteCopyImplicitDisabledMove& operator=(DeleteCopyImplicitDisabledMove const&) = delete;
+ std::function<void()> Captured;
+};
+
+struct DeleteCopyDefaultMove {
+ DeleteCopyDefaultMove() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture this and storing it in class member
+ DeleteCopyDefaultMove(DeleteCopyDefaultMove const&) = delete;
+ DeleteCopyDefaultMove(DeleteCopyDefaultMove &&) = default;
+ DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove const&) = delete;
+ DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove &&) = default;
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
+
+struct DeleteMoveDefaultCopy {
+ DeleteMoveDefaultCopy() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture this and storing it in class member
+ DeleteMoveDefaultCopy(DeleteMoveDefaultCopy const&) = default;
+ DeleteMoveDefaultCopy(DeleteMoveDefaultCopy &&) = delete;
+ DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy const&) = default;
+ DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy &&) = delete;
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
+
+struct DeleteCopyMoveBase {
+ DeleteCopyMoveBase() = default;
+ DeleteCopyMoveBase(DeleteCopyMoveBase const&) = delete;
+ DeleteCopyMoveBase(DeleteCopyMoveBase &&) = delete;
+ DeleteCopyMoveBase& operator=(DeleteCopyMoveBase const&) = delete;
+ DeleteCopyMoveBase& operator=(DeleteCopyMoveBase &&) = delete;
+};
+
+struct Inherit : DeleteCopyMoveBase {
+ Inherit() : DeleteCopyMoveBase{}, Captured([this]() { static_cast<void>(this); }) {}
+ std::function<void()> Captured;
+};
+
+struct UserDefinedCopyMove {
+ UserDefinedCopyMove() : Captured([this]() { static_cast<void>(this); }) {}
+ UserDefinedCopyMove(UserDefinedCopyMove const&);
+ UserDefinedCopyMove(UserDefinedCopyMove &&);
+ UserDefinedCopyMove& operator=(UserDefinedCopyMove const&);
+ UserDefinedCopyMove& operator=(UserDefinedCopyMove &&);
+ std::function<void()> Captured;
+};
+
+struct UserDefinedCopyMoveWithDefault1 {
+ UserDefinedCopyMoveWithDefault1() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 const&) = default;
+ UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 &&);
+ UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 const&);
+ UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 &&);
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
+
+struct UserDefinedCopyMoveWithDefault2 {
+ UserDefinedCopyMoveWithDefault2() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 const&);
+ UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 &&) = default;
+ UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 const&);
+ UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 &&);
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
+
+struct UserDefinedCopyMoveWithDefault3 {
+ UserDefinedCopyMoveWithDefault3() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 const&);
+ UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 &&);
+ UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 const&) = default;
+ UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 &&);
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
+
+struct UserDefinedCopyMoveWithDefault4 {
+ UserDefinedCopyMoveWithDefault4() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 const&);
+ UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 &&);
+ UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 const&);
+ UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 &&) = default;
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+};
>From 3643f35d9f6816b340879290b322e81051ea6c29 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 8 Mar 2025 09:46:29 +0800
Subject: [PATCH 2/8] Apply suggestions from code review
Co-authored-by: Baranov Victor <70346889+vbvictor at users.noreply.github.com>
---
.../bugprone/CaptureThisByFieldCheck.cpp | 4 ++--
.../checkers/bugprone/capture-this-by-field.cpp | 16 ++++++++--------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
index 1f0f68acf335f..b1dd6570da462 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
@@ -86,12 +86,12 @@ void CaptureThisByFieldCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field");
diag(Lambda->getBeginLoc(),
- "using lambda expressions to capture this and storing it in class "
+ "using lambda expressions to capture 'this' and storing it in class "
"member will cause potential variable lifetime issue when the class "
"instance is moved or copied")
<< Capture->getLocation();
diag(Field->getLocation(),
- "'std::function' that stores captured this and becomes invalid during "
+ "'std::function' that stores captured 'this' and becomes invalid during "
"copying or moving",
DiagnosticIDs::Note);
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
index 4ffd6c7ade51e..22d4af58532e7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
@@ -18,14 +18,14 @@ struct Basic {
Basic() : Captured([this]() { static_cast<void>(this); }) {}
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to capture this and storing it in class member
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
struct AssignCapture {
AssignCapture() : Captured([Self = this]() { static_cast<void>(Self); }) {}
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to capture this and storing it in class member
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
struct DeleteMoveAndCopy {
@@ -52,7 +52,7 @@ struct DeleteCopyDefaultMove {
DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove const&) = delete;
DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove &&) = default;
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
struct DeleteMoveDefaultCopy {
@@ -63,7 +63,7 @@ struct DeleteMoveDefaultCopy {
DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy const&) = default;
DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy &&) = delete;
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
struct DeleteCopyMoveBase {
@@ -96,7 +96,7 @@ struct UserDefinedCopyMoveWithDefault1 {
UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 const&);
UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 &&);
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
struct UserDefinedCopyMoveWithDefault2 {
@@ -107,7 +107,7 @@ struct UserDefinedCopyMoveWithDefault2 {
UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 const&);
UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 &&);
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
struct UserDefinedCopyMoveWithDefault3 {
@@ -118,7 +118,7 @@ struct UserDefinedCopyMoveWithDefault3 {
UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 const&) = default;
UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 &&);
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
struct UserDefinedCopyMoveWithDefault4 {
@@ -129,5 +129,5 @@ struct UserDefinedCopyMoveWithDefault4 {
UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 const&);
UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 &&) = default;
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured this
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
};
>From 6a599b35a9cd149195a3f444572761d54316cdd0 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 8 Mar 2025 09:47:46 +0800
Subject: [PATCH 3/8] format
---
.../clang-tidy/bugprone/CaptureThisByFieldCheck.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
index 72c0a540a7743..0329614ad2f06 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
@@ -15,8 +15,9 @@
namespace clang::tidy::bugprone {
-/// Finds lambda captures that capture the ``this`` pointer and store it as class
-/// members without handle the copy and move constructors and the assignments.
+/// Finds lambda captures that capture the ``this`` pointer and store it as
+/// class members without handle the copy and move constructors and the
+/// assignments.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capture-this-by-field.html
>From be2272846914761fcf0978d085278f7470b8bff1 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 8 Mar 2025 09:50:29 +0800
Subject: [PATCH 4/8] rename
---
.../clang-tidy/bugprone/BugproneTidyModule.cpp | 6 +++---
.../clang-tidy/bugprone/CMakeLists.txt | 2 +-
...ieldCheck.cpp => CapturingThisByFieldCheck.cpp} | 8 ++++----
...sByFieldCheck.h => CapturingThisByFieldCheck.h} | 14 +++++++-------
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
...is-by-field.rst => capturing-this-by-field.rst} | 6 +++---
clang-tools-extra/docs/clang-tidy/checks/list.rst | 2 +-
...is-by-field.cpp => capturing-this-by-field.cpp} | 2 +-
8 files changed, 22 insertions(+), 22 deletions(-)
rename clang-tools-extra/clang-tidy/bugprone/{CaptureThisByFieldCheck.cpp => CapturingThisByFieldCheck.cpp} (92%)
rename clang-tools-extra/clang-tidy/bugprone/{CaptureThisByFieldCheck.h => CapturingThisByFieldCheck.h} (73%)
rename clang-tools-extra/docs/clang-tidy/checks/bugprone/{capture-this-by-field.rst => capturing-this-by-field.rst} (88%)
rename clang-tools-extra/test/clang-tidy/checkers/bugprone/{capture-this-by-field.cpp => capturing-this-by-field.cpp} (99%)
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index ee9fa5ef06c7a..b4628c65dcd50 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -16,7 +16,7 @@
#include "BitwisePointerCastCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "BranchCloneCheck.h"
-#include "CaptureThisByFieldCheck.h"
+#include "CapturingThisByFieldCheck.h"
#include "CastingThroughVoidCheck.h"
#include "ChainedComparisonCheck.h"
#include "ComparePointerToMemberVirtualFunctionCheck.h"
@@ -119,8 +119,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"bugprone-bool-pointer-implicit-conversion");
CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");
- CheckFactories.registerCheck<CaptureThisByFieldCheck>(
- "bugprone-capture-this-by-field");
+ CheckFactories.registerCheck<CapturingThisByFieldCheck>(
+ "bugprone-capturing-this-by-field");
CheckFactories.registerCheck<CastingThroughVoidCheck>(
"bugprone-casting-through-void");
CheckFactories.registerCheck<ChainedComparisonCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 4d1d50c4ea2a0..e175c331d1021 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -12,7 +12,7 @@ add_clang_library(clangTidyBugproneModule STATIC
BoolPointerImplicitConversionCheck.cpp
BranchCloneCheck.cpp
BugproneTidyModule.cpp
- CaptureThisByFieldCheck.cpp
+ CapturingThisByFieldCheck.cpp
CastingThroughVoidCheck.cpp
ChainedComparisonCheck.cpp
ComparePointerToMemberVirtualFunctionCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
similarity index 92%
rename from clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
rename to clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
index b1dd6570da462..ad5d22e352854 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
@@ -1,4 +1,4 @@
-//===--- CaptureThisByFieldCheck.cpp - clang-tidy -------------------------===//
+//===--- CapturingThisByFieldCheck.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.
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
-#include "CaptureThisByFieldCheck.h"
+#include "CapturingThisByFieldCheck.h"
#include "clang/AST/DeclCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
@@ -58,7 +58,7 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
} // namespace
-void CaptureThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
+void CapturingThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
auto IsStdFunctionField =
fieldDecl(hasType(cxxRecordDecl(hasName("::std::function"))))
.bind("field");
@@ -81,7 +81,7 @@ void CaptureThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
this);
}
-void CaptureThisByFieldCheck::check(const MatchFinder::MatchResult &Result) {
+void CapturingThisByFieldCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Capture = Result.Nodes.getNodeAs<LambdaCapture>("capture");
const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h
similarity index 73%
rename from clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
rename to clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h
index 0329614ad2f06..068191411d323 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h
@@ -1,4 +1,4 @@
-//===--- CaptureThisByFieldCheck.h - clang-tidy -----------------*- C++ -*-===//
+//===--- CapturingThisByFieldCheck.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.
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURETHISBYFIELDCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURETHISBYFIELDCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H
#include "../ClangTidyCheck.h"
#include "clang/AST/ASTTypeTraits.h"
@@ -20,10 +20,10 @@ namespace clang::tidy::bugprone {
/// assignments.
///
/// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capture-this-by-field.html
-class CaptureThisByFieldCheck : public ClangTidyCheck {
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capturing-this-by-field.html
+class CapturingThisByFieldCheck : public ClangTidyCheck {
public:
- CaptureThisByFieldCheck(StringRef Name, ClangTidyContext *Context)
+ CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@@ -37,4 +37,4 @@ class CaptureThisByFieldCheck : public ClangTidyCheck {
} // namespace clang::tidy::bugprone
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURETHISBYFIELDCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 40c5ee9f60db0..b5068fa0ddf5b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,8 +91,8 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
-- New :doc:`bugprone-capture-this-by-field
- <clang-tidy/checks/bugprone/capture-this-by-field>` check.
+- New :doc:`bugprone-capturing-this-by-field
+ <clang-tidy/checks/bugprone/capturing-this-by-field>` check.
Finds lambda captures that capture the ``this`` pointer and store it as class
members without handle the copy and move constructors and the assignments.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capture-this-by-field.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
similarity index 88%
rename from clang-tools-extra/docs/clang-tidy/checks/bugprone/capture-this-by-field.rst
rename to clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
index c837ce5cb6d68..60324b7363373 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capture-this-by-field.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
@@ -1,7 +1,7 @@
-.. title:: clang-tidy - bugprone-capture-this-by-field
+.. title:: clang-tidy - bugprone-capturing-this-by-field
-bugprone-capture-this-by-field
-==============================
+bugprone-capturing-this-by-field
+================================
Finds lambda captures that capture the ``this`` pointer and store it as class
members without handle the copy and move constructors and the assignments.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6bef286f28100..8d7bf08ea99cc 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -84,7 +84,7 @@ Clang-Tidy Checks
:doc:`bugprone-bitwise-pointer-cast <bugprone/bitwise-pointer-cast>`,
:doc:`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion>`, "Yes"
:doc:`bugprone-branch-clone <bugprone/branch-clone>`,
- :doc:`bugprone-capture-this-by-field <bugprone/capture-this-by-field>`,
+ :doc:`bugprone-capturing-this-by-field <bugprone/capturing-this-by-field>`,
:doc:`bugprone-casting-through-void <bugprone/casting-through-void>`,
:doc:`bugprone-chained-comparison <bugprone/chained-comparison>`,
:doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
similarity index 99%
rename from clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
rename to clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
index 22d4af58532e7..2191d50b53457 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capture-this-by-field.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capture-this-by-field %t
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capturing-this-by-field %t
namespace std {
>From c5702f9f74380858838f4a39b1d8a5d66b57837d Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 8 Mar 2025 10:18:28 +0800
Subject: [PATCH 5/8] add FunctionWrapperTypesOption
---
.../bugprone/CapturingThisByFieldCheck.cpp | 21 +++++++--
.../bugprone/CapturingThisByFieldCheck.h | 8 +++-
.../bugprone/capturing-this-by-field.rst | 9 ++++
.../bugprone/capturing-this-by-field.cpp | 45 ++++++++++++-------
4 files changed, 61 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
index ad5d22e352854..d16187a206715 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "CapturingThisByFieldCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
#include "clang/AST/DeclCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
@@ -58,9 +60,21 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
} // namespace
+CapturingThisByFieldCheck::CapturingThisByFieldCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ FunctionWrapperTypes(utils::options::parseStringList(Options.get(
+ "FunctionWrapperTypes", "::std::function;::boost::function"))) {}
+void CapturingThisByFieldCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "FunctionWrapperTypes",
+ utils::options::serializeStringList(FunctionWrapperTypes));
+}
+
void CapturingThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
auto IsStdFunctionField =
- fieldDecl(hasType(cxxRecordDecl(hasName("::std::function"))))
+ fieldDecl(hasType(cxxRecordDecl(
+ matchers::matchesAnyListedName(FunctionWrapperTypes))))
.bind("field");
auto CaptureThis = lambdaCapture(anyOf(
// [this]
@@ -91,9 +105,10 @@ void CapturingThisByFieldCheck::check(const MatchFinder::MatchResult &Result) {
"instance is moved or copied")
<< Capture->getLocation();
diag(Field->getLocation(),
- "'std::function' that stores captured 'this' and becomes invalid during "
+ "'%0' that stores captured 'this' and becomes invalid during "
"copying or moving",
- DiagnosticIDs::Note);
+ DiagnosticIDs::Note)
+ << Field->getType().getAsString();
}
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h
index 068191411d323..ea26b690bce2f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h
@@ -23,8 +23,8 @@ namespace clang::tidy::bugprone {
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capturing-this-by-field.html
class CapturingThisByFieldCheck : public ClangTidyCheck {
public:
- CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
@@ -33,6 +33,10 @@ class CapturingThisByFieldCheck : public ClangTidyCheck {
std::optional<TraversalKind> getCheckTraversalKind() const override {
return TraversalKind::TK_IgnoreUnlessSpelledInSource;
}
+
+private:
+ ///< store the function wrapper types
+ const std::vector<StringRef> FunctionWrapperTypes;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
index 60324b7363373..98926f06690ec 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
@@ -26,3 +26,12 @@ pointer points to the new object, which will lead to bugs.
assert(v2.Captured() == v1.Captured()); // v2.Captured capture v1's this pointer
assert(v2.Captured() == &v2); // assertion failed.
}
+
+Possible fixes include refactoring the function object into a class member
+method or passing the this pointer as a parameter.
+
+.. option:: FunctionWrapperTypes
+
+ A semicolon-separated list of names of types. Used to specify function
+ wrapper that can hold lambda expressions.
+ Default is ``::std::function;::boost::function``.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
index 2191d50b53457..10a7b6bc7791d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capturing-this-by-field %t
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capturing-this-by-field %t -- -config="{CheckOptions: {bugprone-capturing-this-by-field.FunctionWrapperTypes: '::std::function;::Fn'}}" --
namespace std {
@@ -14,18 +14,22 @@ class function<R(Args...)> {
} // namespace std
+struct Fn {
+ template<class F> Fn(F &&);
+};
+
struct Basic {
Basic() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to capture 'this' and storing it in class member
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
struct AssignCapture {
AssignCapture() : Captured([Self = this]() { static_cast<void>(Self); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to capture 'this' and storing it in class member
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
struct DeleteMoveAndCopy {
@@ -46,24 +50,24 @@ struct DeleteCopyImplicitDisabledMove {
struct DeleteCopyDefaultMove {
DeleteCopyDefaultMove() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
DeleteCopyDefaultMove(DeleteCopyDefaultMove const&) = delete;
DeleteCopyDefaultMove(DeleteCopyDefaultMove &&) = default;
DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove const&) = delete;
DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove &&) = default;
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
struct DeleteMoveDefaultCopy {
DeleteMoveDefaultCopy() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
DeleteMoveDefaultCopy(DeleteMoveDefaultCopy const&) = default;
DeleteMoveDefaultCopy(DeleteMoveDefaultCopy &&) = delete;
DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy const&) = default;
DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy &&) = delete;
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
struct DeleteCopyMoveBase {
@@ -90,44 +94,51 @@ struct UserDefinedCopyMove {
struct UserDefinedCopyMoveWithDefault1 {
UserDefinedCopyMoveWithDefault1() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 const&) = default;
UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 &&);
UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 const&);
UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 &&);
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
struct UserDefinedCopyMoveWithDefault2 {
UserDefinedCopyMoveWithDefault2() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 const&);
UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 &&) = default;
UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 const&);
UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 &&);
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
struct UserDefinedCopyMoveWithDefault3 {
UserDefinedCopyMoveWithDefault3() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 const&);
UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 &&);
UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 const&) = default;
UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 &&);
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
struct UserDefinedCopyMoveWithDefault4 {
UserDefinedCopyMoveWithDefault4() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 const&);
UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 &&);
UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 const&);
UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 &&) = default;
std::function<void()> Captured;
- // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
+};
+
+struct CustomFunctionWrapper {
+ CustomFunctionWrapper() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
+ Fn Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: note: 'Fn' that stores captured 'this'
};
>From 57cdd2fe44248d1153c1d32ddf78faeb2c17d116 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 12 Mar 2025 09:52:28 +0800
Subject: [PATCH 6/8] fix review; add more test
---
.../bugprone/CapturingThisByFieldCheck.cpp | 13 ++--
.../bugprone/capturing-this-by-field.rst | 3 +-
.../bugprone/capturing-this-by-field.cpp | 61 +++++++++++++------
3 files changed, 53 insertions(+), 24 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
index d16187a206715..9e3e21ee3f5a3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
@@ -60,11 +60,14 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
} // namespace
+constexpr const char *DefaultFunctionWrapperTypes =
+ "::std::function;::std::move_only_function;::boost::function";
+
CapturingThisByFieldCheck::CapturingThisByFieldCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- FunctionWrapperTypes(utils::options::parseStringList(Options.get(
- "FunctionWrapperTypes", "::std::function;::boost::function"))) {}
+ FunctionWrapperTypes(utils::options::parseStringList(
+ Options.get("FunctionWrapperTypes", DefaultFunctionWrapperTypes))) {}
void CapturingThisByFieldCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "FunctionWrapperTypes",
@@ -100,9 +103,9 @@ void CapturingThisByFieldCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field");
diag(Lambda->getBeginLoc(),
- "using lambda expressions to capture 'this' and storing it in class "
- "member will cause potential variable lifetime issue when the class "
- "instance is moved or copied")
+ "'this' captured by a lambda and stored in a class member variable; "
+ "disable implicit class copying/moving to prevent potential "
+ "use-after-free")
<< Capture->getLocation();
diag(Field->getLocation(),
"'%0' that stores captured 'this' and becomes invalid during "
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
index 98926f06690ec..f4aa02ab869f9 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
@@ -12,7 +12,6 @@ or moved, the captured ``this`` pointer will be implicitly propagated to the
new object. Most of the time, people will believe that the captured ``this``
pointer points to the new object, which will lead to bugs.
-
.. code-block:: c++
struct C {
@@ -34,4 +33,4 @@ method or passing the this pointer as a parameter.
A semicolon-separated list of names of types. Used to specify function
wrapper that can hold lambda expressions.
- Default is ``::std::function;::boost::function``.
+ Default is ``::std::function;::std::move_only_function;::boost::function``.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
index 10a7b6bc7791d..699bcaa3eb674 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
@@ -20,14 +20,19 @@ struct Fn {
struct Basic {
Basic() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'this' captured by a lambda and stored in a class member variable;
std::function<void()> Captured;
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
+struct NotCaptureThis {
+ NotCaptureThis(int V) : Captured([V]() { static_cast<void>(V); }) {}
+ std::function<void()> Captured;
+};
+
struct AssignCapture {
AssignCapture() : Captured([Self = this]() { static_cast<void>(Self); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 'this' captured by a lambda and stored in a class member variable;
std::function<void()> Captured;
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
@@ -50,7 +55,7 @@ struct DeleteCopyImplicitDisabledMove {
struct DeleteCopyDefaultMove {
DeleteCopyDefaultMove() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: 'this' captured by a lambda and stored in a class member variable;
DeleteCopyDefaultMove(DeleteCopyDefaultMove const&) = delete;
DeleteCopyDefaultMove(DeleteCopyDefaultMove &&) = default;
DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove const&) = delete;
@@ -61,7 +66,7 @@ struct DeleteCopyDefaultMove {
struct DeleteMoveDefaultCopy {
DeleteMoveDefaultCopy() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: 'this' captured by a lambda and stored in a class member variable;
DeleteMoveDefaultCopy(DeleteMoveDefaultCopy const&) = default;
DeleteMoveDefaultCopy(DeleteMoveDefaultCopy &&) = delete;
DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy const&) = default;
@@ -70,16 +75,38 @@ struct DeleteMoveDefaultCopy {
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
-struct DeleteCopyMoveBase {
- DeleteCopyMoveBase() = default;
- DeleteCopyMoveBase(DeleteCopyMoveBase const&) = delete;
- DeleteCopyMoveBase(DeleteCopyMoveBase &&) = delete;
- DeleteCopyMoveBase& operator=(DeleteCopyMoveBase const&) = delete;
- DeleteCopyMoveBase& operator=(DeleteCopyMoveBase &&) = delete;
+struct DeleteCopyBase {
+ DeleteCopyBase() = default;
+ DeleteCopyBase(DeleteCopyBase const&) = delete;
+ DeleteCopyBase(DeleteCopyBase &&) = default;
+ DeleteCopyBase& operator=(DeleteCopyBase const&) = delete;
+ DeleteCopyBase& operator=(DeleteCopyBase &&) = default;
};
-struct Inherit : DeleteCopyMoveBase {
- Inherit() : DeleteCopyMoveBase{}, Captured([this]() { static_cast<void>(this); }) {}
+struct DeleteMoveBase {
+ DeleteMoveBase() = default;
+ DeleteMoveBase(DeleteMoveBase const&) = default;
+ DeleteMoveBase(DeleteMoveBase &&) = delete;
+ DeleteMoveBase& operator=(DeleteMoveBase const&) = default;
+ DeleteMoveBase& operator=(DeleteMoveBase &&) = delete;
+};
+
+struct DeleteCopyMoveBase : DeleteCopyBase, DeleteMoveBase {};
+
+struct InheritDeleteCopy : DeleteCopyBase {
+ InheritDeleteCopy() : DeleteCopyBase{}, Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:52: warning: 'this' captured by a lambda and stored in a class member variable;
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
+};
+struct InheritDeleteMove : DeleteMoveBase {
+ InheritDeleteMove() : DeleteMoveBase{}, Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:52: warning: 'this' captured by a lambda and stored in a class member variable;
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
+};
+struct InheritDeleteCopyMove : DeleteCopyMoveBase {
+ InheritDeleteCopyMove() : DeleteCopyMoveBase{}, Captured([this]() { static_cast<void>(this); }) {}
std::function<void()> Captured;
};
@@ -94,7 +121,7 @@ struct UserDefinedCopyMove {
struct UserDefinedCopyMoveWithDefault1 {
UserDefinedCopyMoveWithDefault1() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: 'this' captured by a lambda and stored in a class member variable;
UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 const&) = default;
UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 &&);
UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 const&);
@@ -105,7 +132,7 @@ struct UserDefinedCopyMoveWithDefault1 {
struct UserDefinedCopyMoveWithDefault2 {
UserDefinedCopyMoveWithDefault2() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: 'this' captured by a lambda and stored in a class member variable;
UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 const&);
UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 &&) = default;
UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 const&);
@@ -116,7 +143,7 @@ struct UserDefinedCopyMoveWithDefault2 {
struct UserDefinedCopyMoveWithDefault3 {
UserDefinedCopyMoveWithDefault3() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: 'this' captured by a lambda and stored in a class member variable;
UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 const&);
UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 &&);
UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 const&) = default;
@@ -127,7 +154,7 @@ struct UserDefinedCopyMoveWithDefault3 {
struct UserDefinedCopyMoveWithDefault4 {
UserDefinedCopyMoveWithDefault4() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: 'this' captured by a lambda and stored in a class member variable;
UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 const&);
UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 &&);
UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 const&);
@@ -138,7 +165,7 @@ struct UserDefinedCopyMoveWithDefault4 {
struct CustomFunctionWrapper {
CustomFunctionWrapper() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
+ // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: 'this' captured by a lambda and stored in a class member variable;
Fn Captured;
// CHECK-MESSAGES: :[[@LINE-1]]:6: note: 'Fn' that stores captured 'this'
};
>From 16cad643d5eafd644362ee651edeae81a40b76af Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 12 Mar 2025 16:12:50 +0000
Subject: [PATCH 7/8] add field default init
---
.../bugprone/CapturingThisByFieldCheck.cpp | 20 +++++++++++--------
.../bugprone/capturing-this-by-field.cpp | 17 +++++++++++++---
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
index 9e3e21ee3f5a3..ca41bf5930a5f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
@@ -84,16 +84,20 @@ void CapturingThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
capturesThis(),
// [self = this]
capturesVar(varDecl(hasInitializer(cxxThisExpr())))));
- auto IsInitWithLambda = cxxConstructExpr(hasArgument(
- 0,
- lambdaExpr(hasAnyCapture(CaptureThis.bind("capture"))).bind("lambda")));
+ auto IsLambdaCapturingThis =
+ lambdaExpr(hasAnyCapture(CaptureThis.bind("capture"))).bind("lambda");
+ auto IsInitWithLambda =
+ anyOf(IsLambdaCapturingThis,
+ cxxConstructExpr(hasArgument(0, IsLambdaCapturingThis)));
Finder->addMatcher(
cxxRecordDecl(
- has(cxxConstructorDecl(
- unless(isCopyConstructor()), unless(isMoveConstructor()),
- hasAnyConstructorInitializer(cxxCtorInitializer(
- isMemberInitializer(), forField(IsStdFunctionField),
- withInitializer(IsInitWithLambda))))),
+ anyOf(has(cxxConstructorDecl(
+ unless(isCopyConstructor()), unless(isMoveConstructor()),
+ hasAnyConstructorInitializer(cxxCtorInitializer(
+ isMemberInitializer(), forField(IsStdFunctionField),
+ withInitializer(IsInitWithLambda))))),
+ has(fieldDecl(IsStdFunctionField,
+ hasInClassInitializer(IsInitWithLambda)))),
unless(correctHandleCaptureThisLambda())),
this);
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
index 699bcaa3eb674..7bfa99320fa6f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
@@ -18,13 +18,24 @@ struct Fn {
template<class F> Fn(F &&);
};
-struct Basic {
- Basic() : Captured([this]() { static_cast<void>(this); }) {}
- // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'this' captured by a lambda and stored in a class member variable;
+struct BasicConstructor {
+ BasicConstructor() : Captured([this]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'this' captured by a lambda and stored in a class member variable;
std::function<void()> Captured;
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
+struct BasicField1 {
+ std::function<void()> Captured = [this]() { static_cast<void>(this); };
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 'this' captured by a lambda and stored in a class member variable;
+ // CHECK-MESSAGES: :[[@LINE-2]]:25: note: 'std::function<void (void)>' that stores captured 'this'
+};
+struct BasicField2 {
+ std::function<void()> Captured{[this]() { static_cast<void>(this); }};
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'this' captured by a lambda and stored in a class member variable;
+ // CHECK-MESSAGES: :[[@LINE-2]]:25: note: 'std::function<void (void)>' that stores captured 'this'
+};
+
struct NotCaptureThis {
NotCaptureThis(int V) : Captured([V]() { static_cast<void>(V); }) {}
std::function<void()> Captured;
>From b963ec9a441e4199f18431673eeb5461c76e0de5 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 12 Mar 2025 16:29:50 +0000
Subject: [PATCH 8/8] add test for = and &
---
.../checkers/bugprone/capturing-this-by-field.cpp | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
index 7bfa99320fa6f..9676822edbea8 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
@@ -25,6 +25,20 @@ struct BasicConstructor {
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
};
+struct BasicConstructorWithCaptureAllByValue {
+ BasicConstructorWithCaptureAllByValue() : Captured([=]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'this' captured by a lambda and stored in a class member variable;
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
+};
+
+struct BasicConstructorWithCaptureAllByRef {
+ BasicConstructorWithCaptureAllByRef() : Captured([&]() { static_cast<void>(this); }) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:52: warning: 'this' captured by a lambda and stored in a class member variable;
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
+};
+
struct BasicField1 {
std::function<void()> Captured = [this]() { static_cast<void>(this); };
// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 'this' captured by a lambda and stored in a class member variable;
More information about the cfe-commits
mailing list