[clang-tools-extra] [clang-tidy] Improve `bugprone-capturing-this-in-member-variable` check: add support of `bind` functions. (PR #132635)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 23 13:59:39 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Baranov Victor (vbvictor)
<details>
<summary>Changes</summary>
Improve `bugprone-capturing-this-in-member-variable` check:
Added support of `bind`-like functions that capture and store `this` pointer in class member.
Closes https://github.com/llvm/llvm-project/issues/131220.
---
Full diff: https://github.com/llvm/llvm-project/pull/132635.diff
5 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.cpp (+41-15)
- (modified) clang-tools-extra/clang-tidy/bugprone/CapturingThisInMemberVariableCheck.h (+1)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3-2)
- (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-in-member-variable.rst (+8)
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-in-member-variable.cpp (+57-1)
``````````diff
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;
+};
``````````
</details>
https://github.com/llvm/llvm-project/pull/132635
More information about the cfe-commits
mailing list