Skip to content

Commit 3861d77

Browse files
Extend bugprone-use-after-move check to handle std::optional::reset() and std::any::reset() similarly to smart pointers
1 parent a78861f commit 3861d77

File tree

4 files changed

+48
-13
lines changed

4 files changed

+48
-13
lines changed

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ void UseAfterMoveFinder::getUsesAndReinits(
242242
});
243243
}
244244

245-
bool isStandardSmartPointer(const ValueDecl *VD) {
245+
bool isStandardResettableOwner(const ValueDecl *VD) {
246246
const Type *TheType = VD->getType().getNonReferenceType().getTypePtrOrNull();
247247
if (!TheType)
248248
return false;
@@ -256,7 +256,9 @@ bool isStandardSmartPointer(const ValueDecl *VD) {
256256
return false;
257257

258258
StringRef Name = ID->getName();
259-
if (Name != "unique_ptr" && Name != "shared_ptr" && Name != "weak_ptr")
259+
static const llvm::StringSet<> Names = {"any", "optional", "shared_ptr",
260+
"unique_ptr", "weak_ptr"};
261+
if (!Names.contains(Name))
260262
return false;
261263

262264
return RecordDecl->getDeclContext()->isStdNamespace();
@@ -279,7 +281,7 @@ void UseAfterMoveFinder::getDeclRefs(
279281
if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) {
280282
// Ignore uses of a standard smart pointer that don't dereference the
281283
// pointer.
282-
if (Operator || !isStandardSmartPointer(DeclRef->getDecl())) {
284+
if (Operator || !isStandardResettableOwner(DeclRef->getDecl())) {
283285
DeclRefs->insert(DeclRef);
284286
}
285287
}
@@ -315,9 +317,10 @@ void UseAfterMoveFinder::getReinits(
315317
"::std::unordered_map", "::std::unordered_multiset",
316318
"::std::unordered_multimap"))))));
317319

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

322325
// Matches different types of reinitialization.
323326
auto ReinitMatcher =
@@ -340,7 +343,7 @@ void UseAfterMoveFinder::getReinits(
340343
callee(cxxMethodDecl(hasAnyName("clear", "assign")))),
341344
// reset() on standard smart pointers.
342345
cxxMemberCallExpr(
343-
on(expr(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
346+
on(expr(DeclRefMatcher, StandardResettableOwnerTypeMatcher)),
344347
callee(cxxMethodDecl(hasName("reset")))),
345348
// Methods that have the [[clang::reinitializes]] attribute.
346349
cxxMemberCallExpr(

clang-tools-extra/docs/ReleaseNotes.rst

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

180+
- Improved :doc:`bugprone-use-after-move
181+
<clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on
182+
``reset()`` calls on moved-from ``std::optional`` and ``std::any`` objects,
183+
similarly to smart pointers.
184+
180185
- Improved :doc:`cert-flp30-c <clang-tidy/checks/cert/flp30-c>` check to
181186
fix false positive that floating point variable is only used in increment
182187
expression.

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,10 @@ 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
199+
``std::shared_ptr``, ``std::weak_ptr``, ``std::optional``, and ``std::any``,
200+
which have defined move behavior (objects of these classes are guaranteed to be
201+
empty after they have been moved from).
202+
Therefore, an object of these classes will only be considered to be used
202203
if it is dereferenced, i.e. if ``operator*``, ``operator->`` or ``operator[]``
203204
(in the case of ``std::unique_ptr<T []>``) is called on it.
204205

@@ -222,7 +223,8 @@ The check considers a variable to be reinitialized in the following cases:
222223
``unordered_multimap``.
223224

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

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

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

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

36+
template <typename T>
37+
struct optional {
38+
optional();
39+
void reset();
40+
};
41+
42+
struct any {
43+
any();
44+
void reset();
45+
};
46+
3647
template <typename T1, typename T2>
3748
struct pair {};
3849

@@ -994,10 +1005,10 @@ void standardContainerAssignIsReinit() {
9941005
}
9951006
}
9961007

997-
// Resetting the standard smart pointer types using reset() is treated as a
1008+
// Resetting the standard smart owning types using reset() is treated as a
9981009
// re-initialization. (We don't test std::weak_ptr<> because it can't be
9991010
// dereferenced directly.)
1000-
void standardSmartPointerResetIsReinit() {
1011+
void resetIsReinit() {
10011012
{
10021013
std::unique_ptr<A> ptr;
10031014
std::move(ptr);
@@ -1010,6 +1021,20 @@ void standardSmartPointerResetIsReinit() {
10101021
ptr.reset(new A);
10111022
*ptr;
10121023
}
1024+
{
1025+
std::optional<A> opt;
1026+
std::move(opt);
1027+
opt.reset();
1028+
std::optional<A> opt2 = opt;
1029+
(void)opt2;
1030+
}
1031+
{
1032+
std::any a;
1033+
std::move(a);
1034+
a.reset();
1035+
std::any a2 = a;
1036+
(void)a2;
1037+
}
10131038
}
10141039

10151040
void reinitAnnotation() {

0 commit comments

Comments
 (0)