Skip to content

Commit 0b344b4

Browse files
Extend bugprone-use-after-move check to handle std::optional::reset() and std::any::reset() (#114255)
These need to be handled similarly to the standard smart pointers; they all have a `reset` method.
1 parent ff7fca7 commit 0b344b4

File tree

4 files changed

+56
-12
lines changed

4 files changed

+56
-12
lines changed

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,10 @@ void UseAfterMoveFinder::getReinits(
315315
"::std::unordered_map", "::std::unordered_multiset",
316316
"::std::unordered_multimap"))))));
317317

318-
auto StandardSmartPointerTypeMatcher = hasType(hasUnqualifiedDesugaredType(
319-
recordType(hasDeclaration(cxxRecordDecl(hasAnyName(
320-
"::std::unique_ptr", "::std::shared_ptr", "::std::weak_ptr"))))));
318+
auto StandardResettableOwnerTypeMatcher = hasType(
319+
hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
320+
hasAnyName("::std::unique_ptr", "::std::shared_ptr",
321+
"::std::weak_ptr", "::std::optional", "::std::any"))))));
321322

322323
// Matches different types of reinitialization.
323324
auto ReinitMatcher =
@@ -340,7 +341,7 @@ void UseAfterMoveFinder::getReinits(
340341
callee(cxxMethodDecl(hasAnyName("clear", "assign")))),
341342
// reset() on standard smart pointers.
342343
cxxMemberCallExpr(
343-
on(expr(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
344+
on(expr(DeclRefMatcher, StandardResettableOwnerTypeMatcher)),
344345
callee(cxxMethodDecl(hasName("reset")))),
345346
// Methods that have the [[clang::reinitializes]] attribute.
346347
cxxMemberCallExpr(

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,11 @@ Changes in existing checks
194194
<clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
195195
additional functions to match.
196196

197+
- Improved :doc:`bugprone-use-after-move
198+
<clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on
199+
``reset()`` calls on moved-from ``std::optional`` and ``std::any`` objects,
200+
similarly to smart pointers.
201+
197202
- Improved :doc:`cert-flp30-c <clang-tidy/checks/cert/flp30-c>` check to
198203
fix false positive that floating point variable is only used in increment
199204
expression.

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,13 @@ Any occurrence of the moved variable that is not a reinitialization (see below)
196196
is considered to be a use.
197197

198198
An exception to this are objects of type ``std::unique_ptr``,
199-
``std::shared_ptr`` and ``std::weak_ptr``, which have defined move behavior
200-
(objects of these classes are guaranteed to be empty after they have been moved
201-
from). Therefore, an object of these classes will only be considered to be used
202-
if it is dereferenced, i.e. if ``operator*``, ``operator->`` or ``operator[]``
203-
(in the case of ``std::unique_ptr<T []>``) is called on it.
199+
``std::shared_ptr``, ``std::weak_ptr``, ``std::optional``, and ``std::any``.
200+
An exception to this are objects of type ``std::unique_ptr``,
201+
``std::shared_ptr``, ``std::weak_ptr``, ``std::optional``, and ``std::any``, which
202+
can be reinitialized via ``reset``. For smart pointers specifically, the
203+
moved-from objects have a well-defined state of being ``nullptr``s, and only
204+
``operator*``, ``operator->`` and ``operator[]`` are considered bad accesses as
205+
they would be dereferencing a ``nullptr``.
204206

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

@@ -222,7 +224,8 @@ The check considers a variable to be reinitialized in the following cases:
222224
``unordered_multimap``.
223225

224226
- ``reset()`` is called on the variable and the variable is of type
225-
``std::unique_ptr``, ``std::shared_ptr`` or ``std::weak_ptr``.
227+
``std::unique_ptr``, ``std::shared_ptr``, ``std::weak_ptr``,
228+
``std::optional``, or ``std::any``.
226229

227230
- A member function marked with the ``[[clang::reinitializes]]`` attribute is
228231
called on the variable.

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,19 @@ struct weak_ptr {
3333
bool expired() const;
3434
};
3535

36+
template <typename T>
37+
struct optional {
38+
optional();
39+
T& operator*();
40+
const T& operator*() const;
41+
void reset();
42+
};
43+
44+
struct any {
45+
any();
46+
void reset();
47+
};
48+
3649
template <typename T1, typename T2>
3750
struct pair {};
3851

@@ -257,6 +270,14 @@ void standardSmartPtr() {
257270
// CHECK-NOTES: [[@LINE-1]]:5: warning: 'ptr' used after it was moved
258271
// CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
259272
}
273+
{
274+
std::optional<A> opt;
275+
std::move(opt);
276+
A val = *opt;
277+
(void)val;
278+
// CHECK-NOTES: [[@LINE-2]]:14: warning: 'opt' used after it was moved
279+
// CHECK-NOTES: [[@LINE-4]]:5: note: move occurred here
280+
}
260281
{
261282
// std::weak_ptr<> cannot be dereferenced directly, so we only check that
262283
// member functions may be called on it after a move.
@@ -994,10 +1015,10 @@ void standardContainerAssignIsReinit() {
9941015
}
9951016
}
9961017

997-
// Resetting the standard smart pointer types using reset() is treated as a
1018+
// Resetting the standard smart owning types using reset() is treated as a
9981019
// re-initialization. (We don't test std::weak_ptr<> because it can't be
9991020
// dereferenced directly.)
1000-
void standardSmartPointerResetIsReinit() {
1021+
void resetIsReinit() {
10011022
{
10021023
std::unique_ptr<A> ptr;
10031024
std::move(ptr);
@@ -1010,6 +1031,20 @@ void standardSmartPointerResetIsReinit() {
10101031
ptr.reset(new A);
10111032
*ptr;
10121033
}
1034+
{
1035+
std::optional<A> opt;
1036+
std::move(opt);
1037+
opt.reset();
1038+
std::optional<A> opt2 = opt;
1039+
(void)opt2;
1040+
}
1041+
{
1042+
std::any a;
1043+
std::move(a);
1044+
a.reset();
1045+
std::any a2 = a;
1046+
(void)a2;
1047+
}
10131048
}
10141049

10151050
void reinitAnnotation() {

0 commit comments

Comments
 (0)