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 <congcongcai0...@163.com> Date: Sat, 8 Mar 2025 00:03:39 +0800 Subject: [PATCH 01/12] [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 <congcongcai0...@163.com> Date: Sat, 8 Mar 2025 09:46:29 +0800 Subject: [PATCH 02/12] Apply suggestions from code review Co-authored-by: Baranov Victor <70346889+vbvic...@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 <congcongcai0...@163.com> Date: Sat, 8 Mar 2025 09:47:46 +0800 Subject: [PATCH 03/12] 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 <congcongcai0...@163.com> Date: Sat, 8 Mar 2025 09:50:29 +0800 Subject: [PATCH 04/12] 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 <congcongcai0...@163.com> Date: Sat, 8 Mar 2025 10:18:28 +0800 Subject: [PATCH 05/12] 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 <congcongcai0...@163.com> Date: Wed, 12 Mar 2025 09:52:28 +0800 Subject: [PATCH 06/12] 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 <congcongcai0...@163.com> Date: Wed, 12 Mar 2025 16:12:50 +0000 Subject: [PATCH 07/12] 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 <congcongcai0...@163.com> Date: Wed, 12 Mar 2025 16:29:50 +0000 Subject: [PATCH 08/12] 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; >From 441a45dc9169f8d80c40128f566b1b039e51c584 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Fri, 14 Mar 2025 13:13:13 +0000 Subject: [PATCH 09/12] rename --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 6 +++--- .../clang-tidy/bugprone/CMakeLists.txt | 2 +- ...p => CapturingThisInMemberVariableCheck.cpp} | 17 ++++++++++------- ...k.h => CapturingThisInMemberVariableCheck.h} | 14 +++++++------- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- ...st => capturing-this-in-member-variable.rst} | 6 +++--- .../docs/clang-tidy/checks/list.rst | 2 +- ...pp => capturing-this-in-member-variable.cpp} | 16 +++++++++++++++- 8 files changed, 42 insertions(+), 25 deletions(-) rename clang-tools-extra/clang-tidy/bugprone/{CapturingThisByFieldCheck.cpp => CapturingThisInMemberVariableCheck.cpp} (88%) rename clang-tools-extra/clang-tidy/bugprone/{CapturingThisByFieldCheck.h => CapturingThisInMemberVariableCheck.h} (82%) rename clang-tools-extra/docs/clang-tidy/checks/bugprone/{capturing-this-by-field.rst => capturing-this-in-member-variable.rst} (89%) rename clang-tools-extra/test/clang-tidy/checkers/bugprone/{capturing-this-by-field.cpp => capturing-this-in-member-variable.cpp} (92%) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index b4628c65dcd50..b780a85bdf3fe 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 "CapturingThisByFieldCheck.h" +#include "CapturingThisInMemberVariableCheck.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<CapturingThisByFieldCheck>( - "bugprone-capturing-this-by-field"); + CheckFactories.registerCheck<CapturingThisInMemberVariableCheck>( + "bugprone-capturing-this-in-member-variable"); 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 e175c331d1021..e310ea9c94543 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 - CapturingThisByFieldCheck.cpp + CapturingThisInMemberVariableCheck.cpp CastingThroughVoidCheck.cpp ChainedComparisonCheck.cpp ComparePointerToMemberVirtualFunctionCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp similarity index 88% rename from clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp rename to clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp index ca41bf5930a5f..3a0f9881929b4 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp @@ -1,4 +1,4 @@ -//===--- CapturingThisByFieldCheck.cpp - clang-tidy -----------------------===// +//===--- CapturingThisInMemberVariableCheck.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 "CapturingThisByFieldCheck.h" +#include "CapturingThisInMemberVariableCheck.h" #include "../utils/Matchers.h" #include "../utils/OptionsUtils.h" #include "clang/AST/DeclCXX.h" @@ -49,6 +49,8 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) { return false; } for (CXXMethodDecl const *M : Node.methods()) { + if (M->isCopyAssignmentOperator()) + llvm::errs() << M->isDeleted() << "\n"; if (M->isCopyAssignmentOperator() && M->isDefaulted() && !M->isDeleted()) return false; if (M->isMoveAssignmentOperator() && M->isDefaulted() && !M->isDeleted()) @@ -63,18 +65,18 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) { constexpr const char *DefaultFunctionWrapperTypes = "::std::function;::std::move_only_function;::boost::function"; -CapturingThisByFieldCheck::CapturingThisByFieldCheck(StringRef Name, - ClangTidyContext *Context) +CapturingThisInMemberVariableCheck::CapturingThisInMemberVariableCheck( + StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), FunctionWrapperTypes(utils::options::parseStringList( Options.get("FunctionWrapperTypes", DefaultFunctionWrapperTypes))) {} -void CapturingThisByFieldCheck::storeOptions( +void CapturingThisInMemberVariableCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "FunctionWrapperTypes", utils::options::serializeStringList(FunctionWrapperTypes)); } -void CapturingThisByFieldCheck::registerMatchers(MatchFinder *Finder) { +void CapturingThisInMemberVariableCheck::registerMatchers(MatchFinder *Finder) { auto IsStdFunctionField = fieldDecl(hasType(cxxRecordDecl( matchers::matchesAnyListedName(FunctionWrapperTypes)))) @@ -102,7 +104,8 @@ void CapturingThisByFieldCheck::registerMatchers(MatchFinder *Finder) { this); } -void CapturingThisByFieldCheck::check(const MatchFinder::MatchResult &Result) { +void CapturingThisInMemberVariableCheck::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/CapturingThisByFieldCheck.h b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h similarity index 82% rename from clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h rename to clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h index ea26b690bce2f..fe0b0aa10f108 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h @@ -1,4 +1,4 @@ -//===--- CapturingThisByFieldCheck.h - clang-tidy ---------------*- C++ -*-===// +//===--- CapturingThisInMemberVariableCheck.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_CAPTURINGTHISBYFIELDCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISINMEMBERVARIABLECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISINMEMBERVARIABLECHECK_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/capturing-this-by-field.html -class CapturingThisByFieldCheck : public ClangTidyCheck { +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capturing-this-in-member-variable.html +class CapturingThisInMemberVariableCheck : public ClangTidyCheck { public: - CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context); + CapturingThisInMemberVariableCheck(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; @@ -41,4 +41,4 @@ class CapturingThisByFieldCheck : public ClangTidyCheck { } // namespace clang::tidy::bugprone -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISINMEMBERVARIABLECHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b5068fa0ddf5b..eae152d2be6b1 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-capturing-this-by-field - <clang-tidy/checks/bugprone/capturing-this-by-field>` check. +- New :doc:`bugprone-capturing-this-in-member-variable + <clang-tidy/checks/bugprone/capturing-this-in-member-variable>` 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/capturing-this-by-field.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst similarity index 89% rename from clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst rename to clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst index f4aa02ab869f9..e2cc32744f97f 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-in-member-variable.rst @@ -1,7 +1,7 @@ -.. title:: clang-tidy - bugprone-capturing-this-by-field +.. title:: clang-tidy - bugprone-capturing-this-in-member-variable -bugprone-capturing-this-by-field -================================ +bugprone-capturing-this-in-member-variable +========================================== 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 8d7bf08ea99cc..748e2541f17f1 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-capturing-this-by-field <bugprone/capturing-this-by-field>`, + :doc:`bugprone-capturing-this-in-member-variable <bugprone/capturing-this-in-member-variable>`, :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/capturing-this-by-field.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp similarity index 92% rename from clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp rename to clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp index 9676822edbea8..fcaacae258cf5 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-in-member-variable.cpp @@ -1,4 +1,4 @@ -// 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'}}" -- +// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capturing-this-in-member-variable %t -- -config="{CheckOptions: {bugprone-capturing-this-in-member-variable.FunctionWrapperTypes: '::std::function;::Fn'}}" -- namespace std { @@ -135,6 +135,20 @@ struct InheritDeleteCopyMove : DeleteCopyMoveBase { std::function<void()> Captured; }; +struct PrivateCopyMoveBase { +// It is how to disable copy and move in C++03 + PrivateCopyMoveBase() = default; +private: + PrivateCopyMoveBase(PrivateCopyMoveBase const&) = default; + PrivateCopyMoveBase(PrivateCopyMoveBase &&) = default; + PrivateCopyMoveBase& operator=(PrivateCopyMoveBase const&) = default; + PrivateCopyMoveBase& operator=(PrivateCopyMoveBase &&) = default; +}; +struct InheritPrivateCopyMove : PrivateCopyMoveBase { + InheritPrivateCopyMove() : PrivateCopyMoveBase{}, Captured([this]() { static_cast<void>(this); }) {} + std::function<void()> Captured; +}; + struct UserDefinedCopyMove { UserDefinedCopyMove() : Captured([this]() { static_cast<void>(this); }) {} UserDefinedCopyMove(UserDefinedCopyMove const&); >From 3d75722e9717ba583d4ff859a9ac1efedf4d1441 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Fri, 14 Mar 2025 21:37:12 +0800 Subject: [PATCH 10/12] Apply suggestions from code review Co-authored-by: Baranov Victor <bar.victor.2...@gmail.com> --- .../checks/bugprone/capturing-this-in-member-variable.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst index e2cc32744f97f..907850d795532 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst @@ -21,16 +21,16 @@ pointer points to the new object, which will lead to bugs. 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 + 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. } Possible fixes include refactoring the function object into a class member -method or passing the this pointer as a parameter. +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;::std::move_only_function;::boost::function``. + Default is `::std::function;::std::move_only_function;::boost::function`. >From d9a2d83a5d644245845c04dd3d4eda12341fc201 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Mon, 17 Mar 2025 06:45:11 +0000 Subject: [PATCH 11/12] fix to review --- .../CapturingThisInMemberVariableCheck.cpp | 3 +- .../capturing-this-in-member-variable.cpp | 30 +++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp index 3a0f9881929b4..add0576a42c33 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp @@ -115,8 +115,7 @@ void CapturingThisInMemberVariableCheck::check( "use-after-free") << Capture->getLocation(); diag(Field->getLocation(), - "'%0' that stores captured 'this' and becomes invalid during " - "copying or moving", + "class member of type '%0' that stores captured 'this'", DiagnosticIDs::Note) << Field->getType().getAsString(); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp index fcaacae258cf5..f5ebebfe4b058 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp @@ -22,32 +22,32 @@ 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' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type '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' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type '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' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type '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' + // CHECK-MESSAGES: :[[@LINE-2]]:25: note: class member of type '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' + // CHECK-MESSAGES: :[[@LINE-2]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct NotCaptureThis { @@ -59,7 +59,7 @@ struct AssignCapture { AssignCapture() : Captured([Self = this]() { static_cast<void>(Self); }) {} // 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' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct DeleteMoveAndCopy { @@ -86,7 +86,7 @@ struct DeleteCopyDefaultMove { DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove const&) = delete; DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove &&) = default; std::function<void()> Captured; - // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct DeleteMoveDefaultCopy { @@ -97,7 +97,7 @@ struct DeleteMoveDefaultCopy { DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy const&) = default; DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy &&) = delete; std::function<void()> Captured; - // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct DeleteCopyBase { @@ -122,13 +122,13 @@ 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' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type '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' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct InheritDeleteCopyMove : DeleteCopyMoveBase { InheritDeleteCopyMove() : DeleteCopyMoveBase{}, Captured([this]() { static_cast<void>(this); }) {} @@ -166,7 +166,7 @@ struct UserDefinedCopyMoveWithDefault1 { UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 const&); UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 &&); std::function<void()> Captured; - // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct UserDefinedCopyMoveWithDefault2 { @@ -177,7 +177,7 @@ struct UserDefinedCopyMoveWithDefault2 { UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 const&); UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 &&); std::function<void()> Captured; - // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct UserDefinedCopyMoveWithDefault3 { @@ -188,7 +188,7 @@ struct UserDefinedCopyMoveWithDefault3 { UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 const&) = default; UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 &&); std::function<void()> Captured; - // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct UserDefinedCopyMoveWithDefault4 { @@ -199,12 +199,12 @@ struct UserDefinedCopyMoveWithDefault4 { UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 const&); UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 &&) = default; std::function<void()> Captured; - // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this' + // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type 'std::function<void (void)>' that stores captured 'this' }; struct CustomFunctionWrapper { CustomFunctionWrapper() : Captured([this]() { static_cast<void>(this); }) {} // 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' + // CHECK-MESSAGES: :[[@LINE-1]]:6: note: class member of type 'Fn' that stores captured 'this' }; >From 55efe1fb94b9944a71e905f17dde8fe5ba145e32 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Mon, 17 Mar 2025 07:03:56 +0000 Subject: [PATCH 12/12] fix doc --- .../checks/bugprone/capturing-this-in-member-variable.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst index 907850d795532..bb75e9239d9b5 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst @@ -26,8 +26,11 @@ pointer points to the new object, which will lead to bugs. 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. +Possible fixes: + - marking copy and move constructors and assignment operators deleted. + - using class member method instead of class member variable with function + object types. + - passing ``this`` pointer as parameter .. option:: FunctionWrapperTypes _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits