[clang-tools-extra] r285842 - [clang-tidy] Extend misc-use-after-move to support unique_ptr and shared_ptr.
Martin Bohme via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 2 10:34:47 PDT 2016
Author: mboehme
Date: Wed Nov 2 12:34:47 2016
New Revision: 285842
URL: http://llvm.org/viewvc/llvm-project?rev=285842&view=rev
Log:
[clang-tidy] Extend misc-use-after-move to support unique_ptr and shared_ptr.
Summary:
As a unique_ptr or shared_ptr that has been moved from is guaranteed to be null,
we only warn if the pointer is dereferenced.
Reviewers: hokein, alexfh, aaron.ballman
Subscribers: Prazek, cfe-commits
Differential Revision: https://reviews.llvm.org/D26041
Modified:
clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst
clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp?rev=285842&r1=285841&r2=285842&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp Wed Nov 2 12:34:47 2016
@@ -463,6 +463,26 @@ void UseAfterMoveFinder::getUsesAndReini
});
}
+bool isStandardSmartPointer(const ValueDecl *VD) {
+ const Type *TheType = VD->getType().getTypePtrOrNull();
+ if (!TheType)
+ return false;
+
+ const CXXRecordDecl *RecordDecl = TheType->getAsCXXRecordDecl();
+ if (!RecordDecl)
+ return false;
+
+ const IdentifierInfo *ID = RecordDecl->getIdentifier();
+ if (!ID)
+ return false;
+
+ StringRef Name = ID->getName();
+ if (Name != "unique_ptr" && Name != "shared_ptr" && Name != "weak_ptr")
+ return false;
+
+ return RecordDecl->getDeclContext()->isStdNamespace();
+}
+
void UseAfterMoveFinder::getDeclRefs(
const CFGBlock *Block, const Decl *MovedVariable,
llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs) {
@@ -472,17 +492,33 @@ void UseAfterMoveFinder::getDeclRefs(
if (!S)
continue;
- SmallVector<BoundNodes, 1> Matches =
- match(findAll(declRefExpr(hasDeclaration(equalsNode(MovedVariable)),
- unless(inDecltypeOrTemplateArg()))
- .bind("declref")),
- *S->getStmt(), *Context);
+ auto addDeclRefs = [this, Block,
+ DeclRefs](const ArrayRef<BoundNodes> Matches) {
+ for (const auto &Match : Matches) {
+ const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref");
+ const auto *Operator = Match.getNodeAs<CXXOperatorCallExpr>("operator");
+ if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) {
+ // Ignore uses of a standard smart pointer that don't dereference the
+ // pointer.
+ if (Operator || !isStandardSmartPointer(DeclRef->getDecl())) {
+ DeclRefs->insert(DeclRef);
+ }
+ }
+ }
+ };
- for (const auto &Match : Matches) {
- const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref");
- if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block)
- DeclRefs->insert(DeclRef);
- }
+ auto DeclRefMatcher = declRefExpr(hasDeclaration(equalsNode(MovedVariable)),
+ unless(inDecltypeOrTemplateArg()))
+ .bind("declref");
+
+ addDeclRefs(match(findAll(DeclRefMatcher), *S->getStmt(), *Context));
+ addDeclRefs(match(
+ findAll(cxxOperatorCallExpr(anyOf(hasOverloadedOperatorName("*"),
+ hasOverloadedOperatorName("->"),
+ hasOverloadedOperatorName("[]")),
+ hasArgument(0, DeclRefMatcher))
+ .bind("operator")),
+ *S->getStmt(), *Context));
}
}
@@ -500,6 +536,9 @@ void UseAfterMoveFinder::getReinits(
"::std::unordered_set", "::std::unordered_map",
"::std::unordered_multiset", "::std::unordered_multimap")));
+ auto StandardSmartPointerTypeMatcher = hasType(cxxRecordDecl(
+ hasAnyName("::std::unique_ptr", "::std::shared_ptr", "::std::weak_ptr")));
+
// Matches different types of reinitialization.
auto ReinitMatcher =
stmt(anyOf(
@@ -521,6 +560,10 @@ void UseAfterMoveFinder::getReinits(
// is called on any of the other containers, this will be
// flagged by a compile error anyway.
callee(cxxMethodDecl(hasAnyName("clear", "assign")))),
+ // reset() on standard smart pointers.
+ cxxMemberCallExpr(
+ on(allOf(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
+ callee(cxxMethodDecl(hasName("reset")))),
// Passing variable to a function as a non-const pointer.
callExpr(forEachArgumentWithParam(
unaryOperator(hasOperatorName("&"),
@@ -587,15 +630,10 @@ void UseAfterMoveCheck::registerMatchers
if (!getLangOpts().CPlusPlus11)
return;
- auto StandardSmartPointerTypeMatcher = hasType(
- cxxRecordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr")));
-
auto CallMoveMatcher =
callExpr(
callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
- hasArgument(
- 0,
- declRefExpr(unless(StandardSmartPointerTypeMatcher)).bind("arg")),
+ hasArgument(0, declRefExpr().bind("arg")),
anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()))
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst?rev=285842&r1=285841&r2=285842&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst Wed Nov 2 12:34:47 2016
@@ -75,10 +75,6 @@ move:
std::cout << str;
}
-No warnings are emitted for objects of type ``std::unique_ptr`` and
-``std::shared_ptr``, as they have defined move behavior. (Objects of these
-classes are guaranteed to be empty after they have been moved from.)
-
Subsections below explain more precisely what exactly the check considers to be
a move, use, and reinitialization.
@@ -153,6 +149,13 @@ Use
Any occurrence of the moved variable that is not a reinitialization (see below)
is considered to be a use.
+An exception to this are objects of type ``std::unique_ptr``,
+``std::shared_ptr`` and ``std::weak_ptr``, which have defined move behavior
+(objects of these classes are guaranteed to be empty after they have been moved
+from). Therefore, an object of these classes `` will only be considered to be
+used if it is dereferenced, i.e. if ``operator*``, ``operator->`` or
+``operator[]`` (in the case of ``std::unique_ptr<T []>``) is called on it.
+
If multiple uses occur after a move, only the first of these is flagged.
Reinitialization
@@ -172,6 +175,9 @@ The check considers a variable to be rei
``unordered_set``, ``unordered_map``, ``unordered_multiset``,
``unordered_multimap``.
+ - ``reset()`` is called on the variable and the variable is of type
+ ``std::unique_ptr``, ``std::shared_ptr`` or ``std::weak_ptr``.
+
If the variable in question is a struct and an individual member variable of
that struct is written to, the check does not consider this to be a
reinitialization -- even if, eventually, all member variables of the struct are
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp?rev=285842&r1=285841&r2=285842&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp Wed Nov 2 12:34:47 2016
@@ -9,12 +9,27 @@ template <typename T>
struct unique_ptr {
unique_ptr();
T *get() const;
+ explicit operator bool() const;
+ void reset(T *ptr);
+ T &operator*() const;
+ T *operator->() const;
+ T& operator[](size_t i) const;
};
template <typename T>
struct shared_ptr {
shared_ptr();
T *get() const;
+ explicit operator bool() const;
+ void reset(T *ptr);
+ T &operator*() const;
+ T *operator->() const;
+};
+
+template <typename T>
+struct weak_ptr {
+ weak_ptr();
+ bool expired() const;
};
#define DECLARE_STANDARD_CONTAINER(name) \
@@ -146,20 +161,58 @@ void parameters(A a) {
// CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
}
-void uniquePtrAndSharedPtr() {
- // Use-after-moves on std::unique_ptr<> or std::shared_ptr<> aren't flagged.
+void standardSmartPtr() {
+ // std::unique_ptr<>, std::shared_ptr<> and std::weak_ptr<> are guaranteed to
+ // be null after a std::move. So the check only flags accesses that would
+ // dereference the pointer.
{
std::unique_ptr<A> ptr;
std::move(ptr);
ptr.get();
+ static_cast<bool>(ptr);
+ *ptr;
+ // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'ptr' used after it was moved
+ // CHECK-MESSAGES: [[@LINE-5]]:5: note: move occurred here
+ }
+ {
+ std::unique_ptr<A> ptr;
+ std::move(ptr);
+ ptr->foo();
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'ptr' used after it was moved
+ // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+ }
+ {
+ std::unique_ptr<A> ptr;
+ std::move(ptr);
+ ptr[0];
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'ptr' used after it was moved
+ // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
}
{
std::shared_ptr<A> ptr;
std::move(ptr);
ptr.get();
+ static_cast<bool>(ptr);
+ *ptr;
+ // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'ptr' used after it was moved
+ // CHECK-MESSAGES: [[@LINE-5]]:5: note: move occurred here
+ }
+ {
+ std::shared_ptr<A> ptr;
+ std::move(ptr);
+ ptr->foo();
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'ptr' used after it was moved
+ // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
}
- // This is also true if the std::unique_ptr<> or std::shared_ptr<> is wrapped
- // in a typedef.
+ {
+ // std::weak_ptr<> cannot be dereferenced directly, so we only check that
+ // member functions may be called on it after a move.
+ std::weak_ptr<A> ptr;
+ std::move(ptr);
+ ptr.expired();
+ }
+ // Make sure we recognize std::unique_ptr<> or std::shared_ptr<> if they're
+ // wrapped in a typedef.
{
typedef std::unique_ptr<A> PtrToA;
PtrToA ptr;
@@ -172,7 +225,8 @@ void uniquePtrAndSharedPtr() {
std::move(ptr);
ptr.get();
}
- // And it's also true if the template argument is a little more involved.
+ // And we don't get confused if the template argument is a little more
+ // involved.
{
struct B {
typedef A AnotherNameForA;
@@ -181,6 +235,17 @@ void uniquePtrAndSharedPtr() {
std::move(ptr);
ptr.get();
}
+ // We don't give any special treatment to types that are called "unique_ptr"
+ // or "shared_ptr" but are not in the "::std" namespace.
+ {
+ struct unique_ptr {
+ void get();
+ } ptr;
+ std::move(ptr);
+ ptr.get();
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'ptr' used after it was moved
+ // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+ }
}
// The check also works in member functions.
@@ -795,6 +860,24 @@ void standardContainerAssignIsReinit() {
}
}
+// Resetting the standard smart pointer types using reset() is treated as a
+// re-initialization. (We don't test std::weak_ptr<> because it can't be
+// dereferenced directly.)
+void standardSmartPointerResetIsReinit() {
+ {
+ std::unique_ptr<A> ptr;
+ std::move(ptr);
+ ptr.reset(new A);
+ *ptr;
+ }
+ {
+ std::shared_ptr<A> ptr;
+ std::move(ptr);
+ ptr.reset(new A);
+ *ptr;
+ }
+}
+
////////////////////////////////////////////////////////////////////////////////
// Tests related to order of evaluation within expressions
More information about the cfe-commits
mailing list