[clang-tidy] Add new check bugprone-capture-this-by-field #130297
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-this-by-a-field-of-default-copyable-class
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Congcong Cai (HerrCai0907) ChangesFinds lambda captures that capture the Capture this in a lambda and store it as a class member is dangerous because the Full diff: https://github.com/llvm/llvm-project/pull/130297.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 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 ce1418a2a7d58..40c5ee9f60db0 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
+};
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
denzor200
suggested changes
Mar 7, 2025
clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Mar 7, 2025
clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Mar 7, 2025
clang-tools-extra/clang-tidy/bugprone/CaptureThisByFieldCheck.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Baranov Victor <[email protected]>
HerrCai0907
force-pushed
the
120863-clang-tidy-check-request-dont-capture-this-by-a-field-of-default-copyable-class
branch
from
a503ee1 to
c5702f9
Compare
PiotrZSL
reviewed
Mar 10, 2025
clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst
Outdated
Show resolved
Hide resolved
|
|
||
| void foo() { | ||
| C v1{}; | ||
| C v2 = v1; // v2.Captured capture v1's this pointer |
There was a problem hiding this comment.
Then shouldn't this check be somehow limited only to classes that got move/copy construction enabled ?
And in such case name maybe could be different, like something with dangling this.
There was a problem hiding this comment.
I don't understand it well. Do you mean we need more document about how to limit it? And mention dangling this in docs also?
clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
Outdated
Show resolved
Hide resolved
denzor200
suggested changes
Mar 10, 2025
clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
Outdated
Show resolved
Hide resolved
denzor200
suggested changes
Mar 10, 2025
clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
Outdated
Show resolved
Hide resolved
denzor200
suggested changes
Mar 10, 2025
clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
Outdated
Show resolved
Hide resolved
denzor200
suggested changes
Mar 10, 2025
clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Mar 14, 2025
clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Mar 14, 2025
clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Mar 14, 2025
clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Mar 14, 2025
clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Baranov Victor <[email protected]>
vbvictor
reviewed
Mar 14, 2025
clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
Outdated
Show resolved
Hide resolved
vbvictor
approved these changes
Mar 14, 2025
denzor200
approved these changes
Mar 15, 2025
| assert(v2.Captured() == &v2); // assertion failed. | ||
| } | ||
|
|
||
| Possible fixes include refactoring the function object into a class member |
There was a problem hiding this comment.
The first possible fix is to make the class completely uncopyable. In 90% of cases, the real reason for this warning is that someone forgot to disallow copying.
There was a problem hiding this comment.
And the rest of the users who really want the class to be copyable will refactor the code in the way you suggested.
|
LGTM, please provide the change for one comment I left |
…field-of-default-copyable-class' of github.com:HerrCai0907/llvm-project into 120863-clang-tidy-check-request-dont-capture-this-by-a-field-of-default-copyable-class
HerrCai0907
deleted the
120863-clang-tidy-check-request-dont-capture-this-by-a-field-of-default-copyable-class
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finds lambda captures that capture the
thispointer and store it as classmembers 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
thispointer will be implicitly propagated to thenew object. Most of the time, people will believe that the captured
thispointer points to the new object, which will lead to bugs.
Fixes: #120863