Skip to content

Commit 51e3a4b

Browse files
PiotrZSLPiotrZSL
committed
[clang-tidy] Fix smart pointers handling in bugprone-use-after-move
Removed custom handling of smart pointers and added option IgnoreNonDerefSmartPtrs to restore previous behavior if needed.
1 parent 2a5f7e5 commit 51e3a4b

File tree

8 files changed

+150
-67
lines changed

8 files changed

+150
-67
lines changed

‎clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

+17-6
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ struct UseAfterMove {
4848
/// various internal helper functions).
4949
class UseAfterMoveFinder {
5050
public:
51-
UseAfterMoveFinder(ASTContext *TheContext);
51+
UseAfterMoveFinder(ASTContext *TheContext, bool IgnoreNonDerefSmartPtrs);
5252

5353
// Within the given code block, finds the first use of 'MovedVariable' that
5454
// occurs after 'MovingCall' (the expression that performs the move). If a
@@ -71,6 +71,7 @@ class UseAfterMoveFinder {
7171
llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
7272

7373
ASTContext *Context;
74+
const bool IgnoreNonDerefSmartPtrs;
7475
std::unique_ptr<ExprSequence> Sequence;
7576
std::unique_ptr<StmtToBlockMap> BlockMap;
7677
llvm::SmallPtrSet<const CFGBlock *, 8> Visited;
@@ -92,8 +93,9 @@ static StatementMatcher inDecltypeOrTemplateArg() {
9293
hasAncestor(expr(hasUnevaluatedContext())));
9394
}
9495

95-
UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext)
96-
: Context(TheContext) {}
96+
UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext,
97+
bool IgnoreNonDerefSmartPtrs)
98+
: Context(TheContext), IgnoreNonDerefSmartPtrs(IgnoreNonDerefSmartPtrs) {}
9799

98100
std::optional<UseAfterMove>
99101
UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
@@ -275,11 +277,13 @@ void UseAfterMoveFinder::getDeclRefs(
275277
DeclRefs](const ArrayRef<BoundNodes> Matches) {
276278
for (const auto &Match : Matches) {
277279
const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref");
278-
const auto *Operator = Match.getNodeAs<CXXOperatorCallExpr>("operator");
279280
if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) {
280281
// Ignore uses of a standard smart pointer that don't dereference the
281282
// pointer.
282-
if (Operator || !isStandardSmartPointer(DeclRef->getDecl())) {
283+
const auto *Operator =
284+
Match.getNodeAs<CXXOperatorCallExpr>("operator");
285+
if (Operator || !IgnoreNonDerefSmartPtrs ||
286+
!isStandardSmartPointer(DeclRef->getDecl())) {
283287
DeclRefs->insert(DeclRef);
284288
}
285289
}
@@ -429,6 +433,13 @@ static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg,
429433
<< IsMove;
430434
}
431435
}
436+
UseAfterMoveCheck::UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context)
437+
: ClangTidyCheck(Name, Context),
438+
IgnoreNonDerefSmartPtrs(Options.get("IgnoreNonDerefSmartPtrs", false)) {}
439+
440+
void UseAfterMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
441+
Options.store(Opts, "IgnoreNonDerefSmartPtrs", IgnoreNonDerefSmartPtrs);
442+
}
432443

433444
void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
434445
// try_emplace is a common maybe-moving function that returns a
@@ -520,7 +531,7 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) {
520531
}
521532

522533
for (Stmt *CodeBlock : CodeBlocks) {
523-
UseAfterMoveFinder Finder(Result.Context);
534+
UseAfterMoveFinder Finder(Result.Context, IgnoreNonDerefSmartPtrs);
524535
if (auto Use = Finder.find(CodeBlock, MovingCall, Arg))
525536
emitDiagnostic(MovingCall, Arg, *Use, this, Result.Context,
526537
determineMoveType(MoveDecl));

‎clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@ namespace clang::tidy::bugprone {
2020
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html
2121
class UseAfterMoveCheck : public ClangTidyCheck {
2222
public:
23-
UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context)
24-
: ClangTidyCheck(Name, Context) {}
23+
UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context);
2524
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
2625
return LangOpts.CPlusPlus11;
2726
}
2827
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
2928
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
29+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
30+
31+
private:
32+
const bool IgnoreNonDerefSmartPtrs;
3033
};
3134

3235
} // namespace clang::tidy::bugprone

‎clang-tools-extra/docs/ReleaseNotes.rst

+5
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ New check aliases
104104
Changes in existing checks
105105
^^^^^^^^^^^^^^^^^^^^^^^^^^
106106

107+
- Improved :doc:`bugprone-use-after-move
108+
<clang-tidy/checks/bugprone/use-after-move>` check to handle smart pointers
109+
like any other objects allowing to detect more cases, previous behavior can
110+
be restored by setting `IgnoreNonDerefSmartPtrs` option to `true`.
111+
107112
Removed checks
108113
^^^^^^^^^^^^^^
109114

‎clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst

+13-1
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,8 @@ An exception to this are objects of type ``std::unique_ptr``,
200200
(objects of these classes are guaranteed to be empty after they have been moved
201201
from). Therefore, an object of these classes will only be considered to be used
202202
if it is dereferenced, i.e. if ``operator*``, ``operator->`` or ``operator[]``
203-
(in the case of ``std::unique_ptr<T []>``) is called on it.
203+
(in the case of ``std::unique_ptr<T []>``) is called on it. This behavior can be
204+
overridden with the option :option:`IgnoreNonDerefSmartPtrs`.
204205

205206
If multiple uses occur after a move, only the first of these is flagged.
206207

@@ -250,3 +251,14 @@ For example, if an additional member variable is added to ``S``, it is easy to
250251
forget to add the reinitialization for this additional member. Instead, it is
251252
safer to assign to the entire struct in one go, and this will also avoid the
252253
use-after-move warning.
254+
255+
Options
256+
-------
257+
258+
.. option:: IgnoreNonDerefSmartPtrs
259+
260+
If this option is set to `true`, the check will not warn about uses of
261+
``std::unique_ptr``, ``std::shared_ptr`` that are not dereferences. This
262+
can be useful if you are using these smart pointers in a way that is not
263+
idiomatic, but that you know is safe. Default is `false`.
264+

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-smart-ptr.cpp

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- \
2+
// RUN: -config="{CheckOptions: {bugprone-use-after-move.IgnoreNonDerefSmartPtrs: false}}" -- -fno-delayed-template-parsing -I %S/../modernize/Inputs/smart-ptr/
3+
4+
#include "unique_ptr.h"
5+
6+
namespace PR90174 {
7+
8+
struct A {};
9+
10+
struct SinkA {
11+
SinkA(std::unique_ptr<A>);
12+
};
13+
14+
class ClassB {
15+
ClassB(std::unique_ptr<A> aaa) : aa(std::move(aaa)) {
16+
a = std::make_unique<SinkA>(std::move(aaa));
17+
// CHECK-MESSAGES: [[@LINE-1]]:43: warning: 'aaa' used after it was moved
18+
// CHECK-MESSAGES: [[@LINE-3]]:36: note: move occurred here
19+
}
20+
std::unique_ptr<A> aa;
21+
std::unique_ptr<SinkA> a;
22+
};
23+
24+
void s(const std::unique_ptr<A> &);
25+
26+
template <typename T, typename... Args> auto my_make_unique(Args &&...args) {
27+
return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
28+
}
29+
30+
void natively(std::unique_ptr<A> x) {
31+
std::unique_ptr<A> tmp = std::move(x);
32+
std::unique_ptr<SinkA> y2{new SinkA(std::move(x))};
33+
// CHECK-MESSAGES: [[@LINE-1]]:49: warning: 'x' used after it was moved
34+
// CHECK-MESSAGES: [[@LINE-3]]:28: note: move occurred here
35+
}
36+
37+
void viaStdMakeUnique(std::unique_ptr<A> x) {
38+
std::unique_ptr<A> tmp = std::move(x);
39+
std::unique_ptr<SinkA> y2 =
40+
std::make_unique<SinkA>(std::move(x));
41+
// CHECK-MESSAGES: [[@LINE-1]]:41: warning: 'x' used after it was moved
42+
// CHECK-MESSAGES: [[@LINE-4]]:28: note: move occurred here
43+
}
44+
45+
void viaMyMakeUnique(std::unique_ptr<A> x) {
46+
std::unique_ptr<A> tmp = std::move(x);
47+
std::unique_ptr<SinkA> y2 = my_make_unique<SinkA>(std::move(x));
48+
// CHECK-MESSAGES: [[@LINE-1]]:63: warning: 'x' used after it was moved
49+
// CHECK-MESSAGES: [[@LINE-3]]:28: note: move occurred here
50+
}
51+
52+
void viaMyMakeUnique2(std::unique_ptr<A> x) {
53+
std::unique_ptr<A> tmp = std::move(x);
54+
s(x);
55+
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'x' used after it was moved
56+
// CHECK-MESSAGES: [[@LINE-3]]:28: note: move occurred here
57+
}
58+
59+
}

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

+7-58
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,16 @@
1-
// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing
2-
// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing
1+
// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- \
2+
// RUN: -config="{CheckOptions: {bugprone-use-after-move.IgnoreNonDerefSmartPtrs: true}}" -- -fno-delayed-template-parsing -I %S/../modernize/Inputs/smart-ptr/
3+
// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- \
4+
// RUN: -config="{CheckOptions: {bugprone-use-after-move.IgnoreNonDerefSmartPtrs: true}}" -- -fno-delayed-template-parsing -I %S/../modernize/Inputs/smart-ptr/
5+
6+
#include "unique_ptr.h"
7+
#include "shared_ptr.h"
38

49
typedef decltype(nullptr) nullptr_t;
510

611
namespace std {
712
typedef unsigned size_t;
813

9-
template <typename T>
10-
struct unique_ptr {
11-
unique_ptr();
12-
T *get() const;
13-
explicit operator bool() const;
14-
void reset(T *ptr);
15-
T &operator*() const;
16-
T *operator->() const;
17-
T& operator[](size_t i) const;
18-
};
19-
20-
template <typename T>
21-
struct shared_ptr {
22-
shared_ptr();
23-
T *get() const;
24-
explicit operator bool() const;
25-
void reset(T *ptr);
26-
T &operator*() const;
27-
T *operator->() const;
28-
};
29-
3014
template <typename T>
3115
struct weak_ptr {
3216
weak_ptr();
@@ -89,41 +73,6 @@ DECLARE_STANDARD_CONTAINER(unordered_multimap);
8973

9074
typedef basic_string<char> string;
9175

92-
template <typename>
93-
struct remove_reference;
94-
95-
template <typename _Tp>
96-
struct remove_reference {
97-
typedef _Tp type;
98-
};
99-
100-
template <typename _Tp>
101-
struct remove_reference<_Tp &> {
102-
typedef _Tp type;
103-
};
104-
105-
template <typename _Tp>
106-
struct remove_reference<_Tp &&> {
107-
typedef _Tp type;
108-
};
109-
110-
template <typename _Tp>
111-
constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
112-
return static_cast<typename remove_reference<_Tp>::type &&>(__t);
113-
}
114-
115-
template <class _Tp>
116-
constexpr _Tp&&
117-
forward(typename std::remove_reference<_Tp>::type& __t) noexcept {
118-
return static_cast<_Tp&&>(__t);
119-
}
120-
121-
template <class _Tp>
122-
constexpr _Tp&&
123-
forward(typename std::remove_reference<_Tp>::type&& __t) noexcept {
124-
return static_cast<_Tp&&>(__t);
125-
}
126-
12776
} // namespace std
12877

12978
class A {

‎clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/shared_ptr.h

+2
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ class __shared_ptr {
99
public:
1010
type &operator*() { return *ptr; }
1111
type *operator->() { return ptr; }
12+
type *get() const;
1213
type *release();
1314
void reset();
1415
void reset(type *pt);
16+
explicit operator bool() const;
1517

1618
private:
1719
type *ptr;

‎clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/unique_ptr.h

+42
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ class unique_ptr {
1414
type &operator*() { return *ptr; }
1515
type *operator->() { return ptr; }
1616
type *release() { return ptr; }
17+
type *get() const;
18+
type& operator[](unsigned i) const;
1719
void reset() {}
1820
void reset(type *pt) {}
1921
void reset(type pt) {}
22+
explicit operator bool() const;
2023
unique_ptr &operator=(unique_ptr &&) { return *this; }
2124
template <typename T>
2225
unique_ptr &operator=(unique_ptr<T> &&) { return *this; }
@@ -25,4 +28,43 @@ class unique_ptr {
2528
type *ptr;
2629
};
2730

31+
template <typename>
32+
struct remove_reference;
33+
34+
template <typename _Tp>
35+
struct remove_reference {
36+
typedef _Tp type;
37+
};
38+
39+
template <typename _Tp>
40+
struct remove_reference<_Tp &> {
41+
typedef _Tp type;
42+
};
43+
44+
template <typename _Tp>
45+
struct remove_reference<_Tp &&> {
46+
typedef _Tp type;
47+
};
48+
49+
template <typename _Tp>
50+
constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
51+
return static_cast<typename remove_reference<_Tp>::type &&>(__t);
52+
}
53+
54+
template <class _Tp>
55+
constexpr _Tp&&
56+
forward(typename std::remove_reference<_Tp>::type& __t) noexcept {
57+
return static_cast<_Tp&&>(__t);
58+
}
59+
60+
template <class _Tp>
61+
constexpr _Tp&&
62+
forward(typename std::remove_reference<_Tp>::type&& __t) noexcept {
63+
return static_cast<_Tp&&>(__t);
64+
}
65+
66+
template <typename T, typename... Args> std::unique_ptr<T> make_unique(Args &&...args) {
67+
return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
68+
}
69+
2870
} // namespace std

0 commit comments

Comments
 (0)