Skip to content

Commit c5702f9

Browse files
committed
add FunctionWrapperTypesOption
1 parent be22728 commit c5702f9

File tree

4 files changed

+61
-22
lines changed

4 files changed

+61
-22
lines changed

clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.cpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "CapturingThisByFieldCheck.h"
10+
#include "../utils/Matchers.h"
11+
#include "../utils/OptionsUtils.h"
1012
#include "clang/AST/DeclCXX.h"
1113
#include "clang/ASTMatchers/ASTMatchFinder.h"
1214
#include "clang/ASTMatchers/ASTMatchers.h"
@@ -58,9 +60,21 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
5860

5961
} // namespace
6062

63+
CapturingThisByFieldCheck::CapturingThisByFieldCheck(StringRef Name,
64+
ClangTidyContext *Context)
65+
: ClangTidyCheck(Name, Context),
66+
FunctionWrapperTypes(utils::options::parseStringList(Options.get(
67+
"FunctionWrapperTypes", "::std::function;::boost::function"))) {}
68+
void CapturingThisByFieldCheck::storeOptions(
69+
ClangTidyOptions::OptionMap &Opts) {
70+
Options.store(Opts, "FunctionWrapperTypes",
71+
utils::options::serializeStringList(FunctionWrapperTypes));
72+
}
73+
6174
void CapturingThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
6275
auto IsStdFunctionField =
63-
fieldDecl(hasType(cxxRecordDecl(hasName("::std::function"))))
76+
fieldDecl(hasType(cxxRecordDecl(
77+
matchers::matchesAnyListedName(FunctionWrapperTypes))))
6478
.bind("field");
6579
auto CaptureThis = lambdaCapture(anyOf(
6680
// [this]
@@ -91,9 +105,10 @@ void CapturingThisByFieldCheck::check(const MatchFinder::MatchResult &Result) {
91105
"instance is moved or copied")
92106
<< Capture->getLocation();
93107
diag(Field->getLocation(),
94-
"'std::function' that stores captured 'this' and becomes invalid during "
108+
"'%0' that stores captured 'this' and becomes invalid during "
95109
"copying or moving",
96-
DiagnosticIDs::Note);
110+
DiagnosticIDs::Note)
111+
<< Field->getType().getAsString();
97112
}
98113

99114
} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/CapturingThisByFieldCheck.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ namespace clang::tidy::bugprone {
2323
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capturing-this-by-field.html
2424
class CapturingThisByFieldCheck : public ClangTidyCheck {
2525
public:
26-
CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context)
27-
: ClangTidyCheck(Name, Context) {}
26+
CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context);
27+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
2828
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
2929
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
3030
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
@@ -33,6 +33,10 @@ class CapturingThisByFieldCheck : public ClangTidyCheck {
3333
std::optional<TraversalKind> getCheckTraversalKind() const override {
3434
return TraversalKind::TK_IgnoreUnlessSpelledInSource;
3535
}
36+
37+
private:
38+
///< store the function wrapper types
39+
const std::vector<StringRef> FunctionWrapperTypes;
3640
};
3741

3842
} // namespace clang::tidy::bugprone

clang-tools-extra/docs/clang-tidy/checks/bugprone/capturing-this-by-field.rst

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,12 @@ pointer points to the new object, which will lead to bugs.
2626
assert(v2.Captured() == v1.Captured()); // v2.Captured capture v1's this pointer
2727
assert(v2.Captured() == &v2); // assertion failed.
2828
}
29+
30+
Possible fixes include refactoring the function object into a class member
31+
method or passing the this pointer as a parameter.
32+
33+
.. option:: FunctionWrapperTypes
34+
35+
A semicolon-separated list of names of types. Used to specify function
36+
wrapper that can hold lambda expressions.
37+
Default is ``::std::function;::boost::function``.

clang-tools-extra/test/clang-tidy/checkers/bugprone/capturing-this-by-field.cpp

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-capturing-this-by-field %t
1+
// 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'}}" --
22

33
namespace std {
44

@@ -14,18 +14,22 @@ class function<R(Args...)> {
1414

1515
} // namespace std
1616

17+
struct Fn {
18+
template<class F> Fn(F &&);
19+
};
20+
1721
struct Basic {
1822
Basic() : Captured([this]() { static_cast<void>(this); }) {}
19-
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to capture this and storing it in class member
23+
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to capture 'this' and storing it in class member
2024
std::function<void()> Captured;
21-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
25+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
2226
};
2327

2428
struct AssignCapture {
2529
AssignCapture() : Captured([Self = this]() { static_cast<void>(Self); }) {}
26-
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to capture this and storing it in class member
30+
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to capture 'this' and storing it in class member
2731
std::function<void()> Captured;
28-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
32+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
2933
};
3034

3135
struct DeleteMoveAndCopy {
@@ -46,24 +50,24 @@ struct DeleteCopyImplicitDisabledMove {
4650

4751
struct DeleteCopyDefaultMove {
4852
DeleteCopyDefaultMove() : Captured([this]() { static_cast<void>(this); }) {}
49-
// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture this and storing it in class member
53+
// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
5054
DeleteCopyDefaultMove(DeleteCopyDefaultMove const&) = delete;
5155
DeleteCopyDefaultMove(DeleteCopyDefaultMove &&) = default;
5256
DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove const&) = delete;
5357
DeleteCopyDefaultMove& operator=(DeleteCopyDefaultMove &&) = default;
5458
std::function<void()> Captured;
55-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
59+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
5660
};
5761

5862
struct DeleteMoveDefaultCopy {
5963
DeleteMoveDefaultCopy() : Captured([this]() { static_cast<void>(this); }) {}
60-
// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture this and storing it in class member
64+
// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
6165
DeleteMoveDefaultCopy(DeleteMoveDefaultCopy const&) = default;
6266
DeleteMoveDefaultCopy(DeleteMoveDefaultCopy &&) = delete;
6367
DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy const&) = default;
6468
DeleteMoveDefaultCopy& operator=(DeleteMoveDefaultCopy &&) = delete;
6569
std::function<void()> Captured;
66-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
70+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
6771
};
6872

6973
struct DeleteCopyMoveBase {
@@ -90,44 +94,51 @@ struct UserDefinedCopyMove {
9094

9195
struct UserDefinedCopyMoveWithDefault1 {
9296
UserDefinedCopyMoveWithDefault1() : Captured([this]() { static_cast<void>(this); }) {}
93-
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
97+
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
9498
UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 const&) = default;
9599
UserDefinedCopyMoveWithDefault1(UserDefinedCopyMoveWithDefault1 &&);
96100
UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 const&);
97101
UserDefinedCopyMoveWithDefault1& operator=(UserDefinedCopyMoveWithDefault1 &&);
98102
std::function<void()> Captured;
99-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
103+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
100104
};
101105

102106
struct UserDefinedCopyMoveWithDefault2 {
103107
UserDefinedCopyMoveWithDefault2() : Captured([this]() { static_cast<void>(this); }) {}
104-
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
108+
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
105109
UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 const&);
106110
UserDefinedCopyMoveWithDefault2(UserDefinedCopyMoveWithDefault2 &&) = default;
107111
UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 const&);
108112
UserDefinedCopyMoveWithDefault2& operator=(UserDefinedCopyMoveWithDefault2 &&);
109113
std::function<void()> Captured;
110-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
114+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
111115
};
112116

113117
struct UserDefinedCopyMoveWithDefault3 {
114118
UserDefinedCopyMoveWithDefault3() : Captured([this]() { static_cast<void>(this); }) {}
115-
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
119+
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
116120
UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 const&);
117121
UserDefinedCopyMoveWithDefault3(UserDefinedCopyMoveWithDefault3 &&);
118122
UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 const&) = default;
119123
UserDefinedCopyMoveWithDefault3& operator=(UserDefinedCopyMoveWithDefault3 &&);
120124
std::function<void()> Captured;
121-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
125+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
122126
};
123127

124128
struct UserDefinedCopyMoveWithDefault4 {
125129
UserDefinedCopyMoveWithDefault4() : Captured([this]() { static_cast<void>(this); }) {}
126-
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture this and storing it in class member
130+
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: using lambda expressions to capture 'this' and storing it in class member
127131
UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 const&);
128132
UserDefinedCopyMoveWithDefault4(UserDefinedCopyMoveWithDefault4 &&);
129133
UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 const&);
130134
UserDefinedCopyMoveWithDefault4& operator=(UserDefinedCopyMoveWithDefault4 &&) = default;
131135
std::function<void()> Captured;
132-
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that stores captured 'this;
136+
// CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function<void (void)>' that stores captured 'this'
137+
};
138+
139+
struct CustomFunctionWrapper {
140+
CustomFunctionWrapper() : Captured([this]() { static_cast<void>(this); }) {}
141+
// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: using lambda expressions to capture 'this' and storing it in class member
142+
Fn Captured;
143+
// CHECK-MESSAGES: :[[@LINE-1]]:6: note: 'Fn' that stores captured 'this'
133144
};

0 commit comments

Comments
 (0)