szepet created this revision.
Herald added subscribers: baloghadamsoftware, whisperity.
Added new enum in order to differentiate the warning messages on "misusing"
into 3 categories: function calls, moving an object, copying an object. (At the
moment the checker gives the same message in case of copying and moving.)
Additional test cases added as well.
Note: The dependency is only added for the reason not having conflict problems
on the test cases.
https://reviews.llvm.org/D38674
Files:
lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
test/Analysis/MisusedMovedObject.cpp
Index: test/Analysis/MisusedMovedObject.cpp
===================================================================
--- test/Analysis/MisusedMovedObject.cpp
+++ test/Analysis/MisusedMovedObject.cpp
@@ -38,6 +38,7 @@
B() = default;
B(const B &) = default;
B(B &&) = default;
+ B& operator=(const B &q) = default;
void operator=(B &&b) {
return;
}
@@ -70,6 +71,12 @@
A(A &&other, char *k) {
moveconstruct(std::move(other));
}
+ void operator=(const A &other) {
+ i = other.i;
+ d = other.d;
+ b = other.b;
+ return;
+ }
void operator=(A &&other) {
moveconstruct(std::move(other));
return;
@@ -105,17 +112,42 @@
}
void simpleMoveCtorTest() {
- A a;
- A b;
- b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
- a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ {
+ A a;
+ A b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ }
+ {
+ A a;
+ A b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ b = a; // expected-warning {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}}
+ }
+ {
+ A a;
+ A b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ b = std::move(a); // expected-warning {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}}
+ }
}
void simpleMoveAssignementTest() {
- A a;
- A b;
- b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
- a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ {
+ A a;
+ A b;
+ b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ }
+ {
+ A a;
+ A b;
+ b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ A c(a); // expected-warning {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}}
+ }
+ {
+ A a;
+ A b;
+ b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ A c(std::move(a)); // expected-warning {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}}
+ }
}
void moveInInitListTest() {
@@ -270,7 +302,7 @@
{
A a;
for (int i = 0; i < bignum(); i++) { // expected-note {{Loop condition is true. Entering loop body}} expected-note {{Loop condition is true. Entering loop body}}
- constCopyOrMoveCall(std::move(a)); // expected-warning {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}}
+ constCopyOrMoveCall(std::move(a)); // expected-warning {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}}
// expected-note@-1 {{'a' became 'moved-from' here}}
}
}
@@ -447,7 +479,7 @@
// Same thing, but with a switch statement.
{
A a, b;
- switch (i) { // expected-note {{Control jumps to 'case 1:' at line 451}}
+ switch (i) { // expected-note {{Control jumps to 'case 1:' at line 483}}
case 1:
b = std::move(a); // no-warning
break; // expected-note {{Execution jumps to the end of the function}}
@@ -459,7 +491,7 @@
// However, if there's a fallthrough, we do warn.
{
A a, b;
- switch (i) { // expected-note {{Control jumps to 'case 1:' at line 463}}
+ switch (i) { // expected-note {{Control jumps to 'case 1:' at line 495}}
case 1:
b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
case 2:
Index: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -58,6 +58,7 @@
const LocationContext *LCtx, const CallEvent *Call) const;
private:
+ enum MisuseKind {MK_FunCall, MK_Copy, MK_Move};
class MovedBugVisitor : public BugReporterVisitorImpl<MovedBugVisitor> {
public:
MovedBugVisitor(const MemRegion *R) : Region(R), Found(false) {}
@@ -81,7 +82,7 @@
mutable std::unique_ptr<BugType> BT;
ExplodedNode *reportBug(const MemRegion *Region, const CallEvent &Call,
- CheckerContext &C, bool isCopy) const;
+ CheckerContext &C, MisuseKind MK) const;
bool isInMoveSafeContext(const LocationContext *LC) const;
bool isStateResetMethod(const CXXMethodDecl *MethodDec) const;
bool isMoveSafeMethod(const CXXMethodDecl *MethodDec) const;
@@ -177,7 +178,7 @@
ExplodedNode *MisusedMovedObjectChecker::reportBug(const MemRegion *Region,
const CallEvent &Call,
CheckerContext &C,
- bool isCopy = false) const {
+ MisuseKind MK) const {
if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
if (!BT)
BT.reset(new BugType(this, "Usage of a 'moved-from' object",
@@ -193,10 +194,17 @@
// Creating the error message.
std::string ErrorMessage;
- if (isCopy)
- ErrorMessage = "Copying a 'moved-from' object";
- else
- ErrorMessage = "Method call on a 'moved-from' object";
+ switch(MK) {
+ case MK_FunCall:
+ ErrorMessage = "Method call on a 'moved-from' object";
+ break;
+ case MK_Copy:
+ ErrorMessage = "Copying a 'moved-from' object";
+ break;
+ case MK_Move:
+ ErrorMessage = "Moving a 'moved-from' object";
+ break;
+ }
if (const auto DecReg = Region->getAs<DeclRegion>()) {
const auto *RegionDecl = dyn_cast<NamedDecl>(DecReg->getDecl());
ErrorMessage += " '" + RegionDecl->getNameAsString() + "'";
@@ -363,7 +371,10 @@
const RegionState *ArgState = State->get<TrackedRegionMap>(ArgRegion);
if (ArgState && ArgState->isMoved()) {
if (!isInMoveSafeContext(LC)) {
- N = reportBug(ArgRegion, Call, C, /*isCopy=*/true);
+ if(CtorDec->isMoveConstructor())
+ N = reportBug(ArgRegion, Call, C, MK_Move);
+ else
+ N = reportBug(ArgRegion, Call, C, MK_Copy);
State = State->set<TrackedRegionMap>(ArgRegion,
RegionState::getReported());
}
@@ -400,7 +411,10 @@
State->get<TrackedRegionMap>(IC->getArgSVal(0).getAsRegion());
if (ArgState && ArgState->isMoved() && !isInMoveSafeContext(LC)) {
const MemRegion *ArgRegion = IC->getArgSVal(0).getAsRegion();
- N = reportBug(ArgRegion, Call, C, /*isCopy=*/true);
+ if(MethodDecl->isMoveAssignmentOperator())
+ N = reportBug(ArgRegion, Call, C, MK_Move);
+ else
+ N = reportBug(ArgRegion, Call, C, MK_Copy);
State =
State->set<TrackedRegionMap>(ArgRegion, RegionState::getReported());
}
@@ -438,7 +452,7 @@
if (isInMoveSafeContext(LC))
return;
- N = reportBug(ThisRegion, Call, C);
+ N = reportBug(ThisRegion, Call, C, MK_FunCall);
State = State->set<TrackedRegionMap>(ThisRegion, RegionState::getReported());
C.addTransition(State, N);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
