https://github.com/vbvictor updated
https://github.com/llvm/llvm-project/pull/132635
>From c91ad611e7a64b08a243a4a7f07a7f51e96b8ac0 Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2...@gmail.com>
Date: Sun, 23 Mar 2025 23:57:15 +0300
Subject: [PATCH 1/3] [clang-tidy] Add support of `bind` function calls that
capture `this`
---
.../CapturingThisInMemberVariableCheck.cpp | 56 +++++++++++++-----
.../CapturingThisInMemberVariableCheck.h | 1 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 +-
.../capturing-this-in-member-variable.rst | 8 +++
.../capturing-this-in-member-variable.cpp | 58 ++++++++++++++++++-
5 files changed, 110 insertions(+), 18 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
index add0576a42c33..c0bcd7698a6ae 100644
---
a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
+++
b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
@@ -64,16 +64,21 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
constexpr const char *DefaultFunctionWrapperTypes =
"::std::function;::std::move_only_function;::boost::function";
+constexpr const char *DefaultBindFunctions = "::std::bind;::boost::bind";
CapturingThisInMemberVariableCheck::CapturingThisInMemberVariableCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
FunctionWrapperTypes(utils::options::parseStringList(
- Options.get("FunctionWrapperTypes", DefaultFunctionWrapperTypes))) {}
+ Options.get("FunctionWrapperTypes", DefaultFunctionWrapperTypes))),
+ BindFunctions(utils::options::parseStringList(
+ Options.get("BindFunctions", DefaultBindFunctions))) {}
void CapturingThisInMemberVariableCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "FunctionWrapperTypes",
utils::options::serializeStringList(FunctionWrapperTypes));
+ Options.store(Opts, "BindFunctions",
+ utils::options::serializeStringList(BindFunctions));
}
void CapturingThisInMemberVariableCheck::registerMatchers(MatchFinder *Finder)
{
@@ -87,33 +92,54 @@ void
CapturingThisInMemberVariableCheck::registerMatchers(MatchFinder *Finder) {
// [self = this]
capturesVar(varDecl(hasInitializer(cxxThisExpr())))));
auto IsLambdaCapturingThis =
- lambdaExpr(hasAnyCapture(CaptureThis.bind("capture"))).bind("lambda");
- auto IsInitWithLambda =
- anyOf(IsLambdaCapturingThis,
- cxxConstructExpr(hasArgument(0, IsLambdaCapturingThis)));
+ lambdaExpr(hasAnyCapture(CaptureThis)).bind("lambda");
+
+ auto IsBindCapturingThis =
+ callExpr(
+ callee(functionDecl(matchers::matchesAnyListedName(BindFunctions))
+ .bind("callee")),
+ hasAnyArgument(cxxThisExpr()))
+ .bind("bind");
+
+ auto IsInitWithLambdaOrBind =
+ anyOf(IsLambdaCapturingThis, IsBindCapturingThis,
+ cxxConstructExpr(hasArgument(
+ 0, anyOf(IsLambdaCapturingThis, IsBindCapturingThis))));
+
Finder->addMatcher(
cxxRecordDecl(
anyOf(has(cxxConstructorDecl(
unless(isCopyConstructor()), unless(isMoveConstructor()),
hasAnyConstructorInitializer(cxxCtorInitializer(
isMemberInitializer(), forField(IsStdFunctionField),
- withInitializer(IsInitWithLambda))))),
+ withInitializer(IsInitWithLambdaOrBind))))),
has(fieldDecl(IsStdFunctionField,
- hasInClassInitializer(IsInitWithLambda)))),
+ hasInClassInitializer(IsInitWithLambdaOrBind)))),
unless(correctHandleCaptureThisLambda())),
this);
}
-
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 EmitDiag = [this](const SourceLocation &Location,
+ const FunctionDecl *Bind) {
+ const std::string BindName = Bind ? Bind->getQualifiedNameAsString() : "";
+ diag(Location, "'this' captured by a %select{lambda|'%1' call}0 and "
+ "stored in a class member variable; disable implicit class "
+ "copying/moving to prevent potential use-after-free")
+ << (Bind ? 1 : 0) << BindName;
+ };
+
+ if (const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda")) {
+ EmitDiag(Lambda->getBeginLoc(), nullptr);
+ } else if (const auto *Bind = Result.Nodes.getNodeAs<CallExpr>("bind")) {
+ const auto *Callee = Result.Nodes.getNodeAs<FunctionDecl>("callee");
+ assert(Callee);
+ EmitDiag(Bind->getBeginLoc(), Callee);
+ }
+
const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field");
- diag(Lambda->getBeginLoc(),
- "'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();
+ assert(Field);
+
diag(Field->getLocation(),
"class member of type '%0' that stores captured 'this'",
DiagnosticIDs::Note)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h
b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h
index fe0b0aa10f108..934f99cd35797 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h
@@ -37,6 +37,7 @@ class CapturingThisInMemberVariableCheck : public
ClangTidyCheck {
private:
///< store the function wrapper types
const std::vector<StringRef> FunctionWrapperTypes;
+ const std::vector<StringRef> BindFunctions;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 72aa05eb4dcd1..396e071098a18 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -106,8 +106,9 @@ New checks
- 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.
+ Finds lambda captures and ``bind`` function calls 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/capturing-this-in-member-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst
index bb75e9239d9b5..de05bfe3a34bc 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
@@ -32,8 +32,16 @@ Possible fixes:
object types.
- passing ``this`` pointer as parameter
+Options
+-------
+
.. 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`.
+
+.. option:: BindFunctions
+
+ A semicolon-separated list of fully qualified names of functions that can
+ capture ``this`` pointer. Default is `::std::bind;::boost::bind`.
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 f5ebebfe4b058..4c90a8aa8944a 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
@@ -1,4 +1,4 @@
-// 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'}}" --
+// 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',
bugprone-capturing-this-in-member-variable.BindFunctions:
'::std::bind;::Bind'}}" --
namespace std {
@@ -12,12 +12,22 @@ class function<R(Args...)> {
template<class F> function(F &&);
};
+template <typename F, typename... Args>
+function<F(Args...)> bind(F&&, Args&&...) {
+ return {};
+}
+
} // namespace std
struct Fn {
template<class F> Fn(F &&);
};
+template <typename F, typename... Args>
+std::function<F(Args...)> Bind(F&&, Args&&...) {
+ return {};
+}
+
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;
@@ -208,3 +218,49 @@ struct CustomFunctionWrapper {
Fn Captured;
// CHECK-MESSAGES: :[[@LINE-1]]:6: note: class member of type 'Fn' that
stores captured 'this'
};
+
+struct BindConstructor {
+ BindConstructor() : Captured(std::bind(&BindConstructor::method, this)) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'this' captured by a
'std::bind' call and stored in a class member variable;
+ void method() {}
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type
'std::function<void (void)>' that stores captured 'this'
+};
+
+struct BindField1 {
+ void method() {}
+ std::function<void()> Captured = std::bind(&BindField1::method, this);
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 'this' captured by a
'std::bind' call and stored in a class member variable;
+ // CHECK-MESSAGES: :[[@LINE-2]]:25: note: class member of type
'std::function<void (void)>' that stores captured 'this'
+};
+
+struct BindField2 {
+ void method() {}
+ std::function<void()> Captured{std::bind(&BindField2::method, this)};
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'this' captured by a
'std::bind' call and stored in a class member variable;
+ // CHECK-MESSAGES: :[[@LINE-2]]:25: note: class member of type
'std::function<void (void)>' that stores captured 'this'
+};
+
+struct BindCustom {
+ BindCustom() : Captured(Bind(&BindCustom::method, this)) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'this' captured by a 'Bind'
call and stored in a class member variable;
+ void method() {}
+ std::function<void()> Captured;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: note: class member of type
'std::function<void (void)>' that stores captured 'this'
+};
+
+struct BindNotCapturingThis {
+ void method(int) {}
+ BindNotCapturingThis(int V) :
Captured(std::bind(&BindNotCapturingThis::method, V)) {}
+ std::function<void()> Captured;
+};
+
+struct DeletedCopyMoveWithBind {
+ DeletedCopyMoveWithBind() :
Captured(std::bind(&DeletedCopyMoveWithBind::method, this)) {}
+ DeletedCopyMoveWithBind(DeletedCopyMoveWithBind const&) = delete;
+ DeletedCopyMoveWithBind(DeletedCopyMoveWithBind &&) = delete;
+ DeletedCopyMoveWithBind& operator=(DeletedCopyMoveWithBind const&) = delete;
+ DeletedCopyMoveWithBind& operator=(DeletedCopyMoveWithBind &&) = delete;
+ void method() {}
+ std::function<void()> Captured;
+};
>From 1e43ba372b4f63f0e295947945aceb134fbc6a74 Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2...@gmail.com>
Date: Tue, 25 Mar 2025 13:04:17 +0300
Subject: [PATCH 2/3] fix: add more `bind` functions
---
.../bugprone/CapturingThisInMemberVariableCheck.cpp | 4 +++-
.../checks/bugprone/capturing-this-in-member-variable.rst | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
index c0bcd7698a6ae..7fe186b700c8d 100644
---
a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
+++
b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
@@ -64,7 +64,9 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
constexpr const char *DefaultFunctionWrapperTypes =
"::std::function;::std::move_only_function;::boost::function";
-constexpr const char *DefaultBindFunctions = "::std::bind;::boost::bind";
+constexpr const char *DefaultBindFunctions =
+ "::std::bind;::boost::bind;::std::bind_front;::std::bind_back;"
+ "::boost::compat::bind_front;::boost::compat::bind_back";
CapturingThisInMemberVariableCheck::CapturingThisInMemberVariableCheck(
StringRef Name, ClangTidyContext *Context)
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 de05bfe3a34bc..dfc2ca1bbc7dd 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
@@ -44,4 +44,6 @@ Options
.. option:: BindFunctions
A semicolon-separated list of fully qualified names of functions that can
- capture ``this`` pointer. Default is `::std::bind;::boost::bind`.
+ capture ``this`` pointer.
+ Default is `::std::bind;::boost::bind;::std::bind_front;::std::bind_back;
+ ::boost::compat::bind_front;::boost::compat::bind_back`.
>From 82efaf640b3fc9e0b4567d4c4e44fa313809ec83 Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2...@gmail.com>
Date: Wed, 26 Mar 2025 13:29:45 +0300
Subject: [PATCH 3/3] fix: deleted `EmitDiag` and created 2 separate `diag`
calls
---
.../CapturingThisInMemberVariableCheck.cpp | 20 +++++++++----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
index 7fe186b700c8d..1bfe384258ddb 100644
---
a/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
+++
b/clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp
@@ -122,21 +122,19 @@ void
CapturingThisInMemberVariableCheck::registerMatchers(MatchFinder *Finder) {
}
void CapturingThisInMemberVariableCheck::check(
const MatchFinder::MatchResult &Result) {
- const auto EmitDiag = [this](const SourceLocation &Location,
- const FunctionDecl *Bind) {
- const std::string BindName = Bind ? Bind->getQualifiedNameAsString() : "";
- diag(Location, "'this' captured by a %select{lambda|'%1' call}0 and "
- "stored in a class member variable; disable implicit class "
- "copying/moving to prevent potential use-after-free")
- << (Bind ? 1 : 0) << BindName;
- };
-
if (const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda")) {
- EmitDiag(Lambda->getBeginLoc(), nullptr);
+ diag(Lambda->getBeginLoc(),
+ "'this' captured by a lambda and stored in a class member variable; "
+ "disable implicit class copying/moving to prevent potential "
+ "use-after-free");
} else if (const auto *Bind = Result.Nodes.getNodeAs<CallExpr>("bind")) {
const auto *Callee = Result.Nodes.getNodeAs<FunctionDecl>("callee");
assert(Callee);
- EmitDiag(Bind->getBeginLoc(), Callee);
+ diag(Bind->getBeginLoc(),
+ "'this' captured by a '%0' call and stored in a class member "
+ "variable; disable implicit class copying/moving to prevent potential
"
+ "use-after-free")
+ << Callee->getQualifiedNameAsString();
}
const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
