NoQ updated this revision to Diff 174477.
NoQ added a comment.
Write down full messages in tests. When the message was updated from `'x' is
moved'` to `Object 'x' is moved`, the tests were not updated because they kept
passing because the former is still a sub-string of the latter.
https://reviews.llvm.org/D54560
Files:
lib/StaticAnalyzer/Checkers/MoveChecker.cpp
test/Analysis/use-after-move.cpp
Index: test/Analysis/use-after-move.cpp
===================================================================
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -88,7 +88,7 @@
A(const A &other) : i(other.i), d(other.d), b(other.b) {}
A(A &&other) : i(other.i), d(other.d), b(std::move(other.b)) {
#ifdef AGGRESSIVE
- // expected-note@-2{{'b' became 'moved-from' here}}
+ // expected-note@-2{{Object 'b' is moved}}
#endif
}
A(A &&other, char *k) {
@@ -137,49 +137,49 @@
void simpleMoveCtorTest() {
{
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 b = std::move(a); // expected-note {{Object 'a' is moved}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on 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 b = std::move(a); // expected-note {{Object 'a' is moved}}
+ b = a; // expected-warning {{Moved-from object is copied 'a'}} expected-note {{Moved-from object is copied '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'}}
+ A b = std::move(a); // expected-note {{Object 'a' is moved}}
+ b = std::move(a); // expected-warning {{Moved-from object is moved 'a'}} expected-note {{Moved-from object is moved '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'}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on 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'}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
+ A c(a); // expected-warning {{Moved-from object is copied 'a'}} expected-note {{Moved-from object is copied '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'}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
+ A c(std::move(a)); // expected-warning {{Moved-from object is moved 'a'}} expected-note {{Moved-from object is moved 'a'}}
}
}
void moveInInitListTest() {
struct S {
A a;
};
A a;
- S s{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'}}
+ S s{std::move(a)}; // expected-note {{Object 'a' is moved}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
}
// Don't report a bug if the variable was assigned to in the meantime.
@@ -227,19 +227,19 @@
A b;
b = std::move(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'}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
}
// If a path exist where we not reinitialize the variable we report a bug.
{
A a;
A b;
- b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
if (i < 10) { // expected-note {{Assuming 'i' is >= 10}} expected-note {{Taking false branch}}
a = A();
}
if (i > 5) { // expected-note {{Taking true branch}}
- a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
}
}
}
@@ -325,8 +325,8 @@
{
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 {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}}
- // expected-note@-1 {{'a' became 'moved-from' here}}
+ constCopyOrMoveCall(std::move(a)); // expected-warning {{Moved-from object is moved 'a'}} expected-note {{Moved-from object is moved 'a'}}
+ // expected-note@-1 {{Object 'a' is moved}}
}
}
@@ -348,10 +348,10 @@
void uniqueTest(bool cond) {
A a(42, 42.0);
A b;
- b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
if (cond) { // expected-note {{Assuming 'cond' is not equal to 0}} expected-note {{Taking true branch}}
- a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
}
if (cond) {
a.bar(); // no-warning
@@ -362,8 +362,8 @@
void uniqueTest2() {
A a;
- A a1 = 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 a1 = std::move(a); // expected-note {{Object 'a' is moved}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
A a2 = std::move(a); // no-warning
a.foo(); // no-warning
@@ -373,12 +373,12 @@
//even on moved-from objects.
void moveSafeFunctionsTest() {
A a;
- A b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ A b = std::move(a); // expected-note {{Object 'a' is moved}}
a.empty(); // no-warning
a.isEmpty(); // no-warning
(void)a; // no-warning
(bool)a; // expected-warning {{expression result unused}}
- a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
}
void moveStateResetFunctionsTest() {
@@ -445,17 +445,17 @@
b = std::move(a);
a.foo();
#ifdef AGGRESSIVE
- // expected-note@-3{{'a' became 'moved-from' here}}
- // expected-warning@-3 {{Method call on a 'moved-from' object 'a'}}
- // expected-note@-4{{Method call on a 'moved-from' object 'a'}}
+ // expected-note@-3{{Object 'a' is moved}}
+ // expected-warning@-3 {{Method called on moved-from object 'a'}}
+ // expected-note@-4{{Method called on moved-from object 'a'}}
#endif
b = std::move(static_a);
static_a.foo();
#ifdef AGGRESSIVE
- // expected-note@-3{{'static_a' became 'moved-from' here}}
- // expected-warning@-3{{Method call on a 'moved-from' object 'static_a'}}
- // expected-note@-4{{Method call on a 'moved-from' object 'static_a'}}
+ // expected-note@-3{{Object 'static_a' is moved}}
+ // expected-warning@-3{{Method called on moved-from object 'static_a'}}
+ // expected-note@-4{{Method called on moved-from object 'static_a'}}
#endif
}
};
@@ -466,26 +466,26 @@
Arr[2] = std::move(*Ptr);
(*Ptr).foo();
#ifdef AGGRESSIVE
- // expected-note@-3{{Became 'moved-from' here}}
- // expected-warning@-3{{Method call on a 'moved-from' object}}
- // expected-note@-4{{Method call on a 'moved-from' object}}
+ // expected-note@-3{{Object is moved}}
+ // expected-warning@-3{{Method called on moved-from object}}
+ // expected-note@-4{{Method called on moved-from object}}
#endif
Ptr = &Arr[1];
Arr[3] = std::move(Arr[1]);
Ptr->foo();
#ifdef AGGRESSIVE
- // expected-note@-3{{Became 'moved-from' here}}
- // expected-warning@-3{{Method call on a 'moved-from' object}}
- // expected-note@-4{{Method call on a 'moved-from' object}}
+ // expected-note@-3{{Object is moved}}
+ // expected-warning@-3{{Method called on moved-from object}}
+ // expected-note@-4{{Method called on moved-from object}}
#endif
Arr[3] = std::move(Arr[2]);
Arr[2].foo();
#ifdef AGGRESSIVE
- // expected-note@-3{{Became 'moved-from' here}}
- // expected-warning@-3{{Method call on a 'moved-from' object}}
- // expected-note@-4{{Method call on a 'moved-from' object}}
+ // expected-note@-3{{Object is moved}}
+ // expected-warning@-3{{Method called on moved-from object}}
+ // expected-note@-4{{Method called on moved-from object}}
#endif
Arr[2] = std::move(Arr[3]); // reinitialization
@@ -545,9 +545,9 @@
A a, b;
switch (i) { // expected-note {{Control jumps to 'case 1:'}}
case 1:
- b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
case 2:
- a.foo(); // expected-warning {{Method call on a 'moved-from' object}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ a.foo(); // expected-warning {{Method called on moved-from object}} expected-note {{Method called on moved-from object 'a'}}
break;
}
}
@@ -562,23 +562,23 @@
}
void interFunTest1(A &a) {
- a.bar(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
+ a.bar(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
}
void interFunTest2() {
A a;
A b;
- b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
+ b = std::move(a); // expected-note {{Object 'a' is moved}}
interFunTest1(a); // expected-note {{Calling 'interFunTest1'}}
}
void foobar(A a, int i);
void foobar(int i, A a);
void paramEvaluateOrderTest() {
A a;
- foobar(std::move(a), a.getI()); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
- // expected-note@-1 {{'a' became 'moved-from' here}}
+ foobar(std::move(a), a.getI()); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
+ // expected-note@-1 {{Object 'a' is moved}}
//FALSE NEGATIVE since parameters evaluate order is undefined
foobar(a.getI(), std::move(a)); //no-warning
@@ -613,8 +613,8 @@
}
{
A a;
- A a1 = std::move(a), a2 = a; // expected-warning {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}}
- // expected-note@-1 {{'a' became 'moved-from' here}}
+ A a1 = std::move(a), a2 = a; // expected-warning {{Moved-from object is copied 'a'}} expected-note {{Moved-from object is copied 'a'}}
+ // expected-note@-1 {{Object 'a' is moved}}
}
}
@@ -640,8 +640,8 @@
}
{
A a;
- if (A(std::move(a)).foo() > 0 && a.foo() > 0) { // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
- // expected-note@-1 {{'a' became 'moved-from' here}} expected-note@-1 {{Assuming the condition is true}} expected-note@-1 {{Assuming the condition is false}}
+ if (A(std::move(a)).foo() > 0 && a.foo() > 0) { // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
+ // expected-note@-1 {{Object 'a' is moved}} expected-note@-1 {{Assuming the condition is true}} expected-note@-1 {{Assuming the condition is false}}
// expected-note@-2 {{Left side of '&&' is false}} expected-note@-2 {{Left side of '&&' is true}}
// expected-note@-3 {{Taking false branch}}
A().bar();
@@ -657,8 +657,8 @@
}
{
A a;
- if (A(std::move(a)).foo() > 0 || a.foo() > 0) { // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}}
- // expected-note@-1 {{'a' became 'moved-from' here}} expected-note@-1 {{Assuming the condition is false}} expected-note@-1 {{Left side of '||' is false}}
+ if (A(std::move(a)).foo() > 0 || a.foo() > 0) { // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
+ // expected-note@-1 {{Object 'a' is moved}} expected-note@-1 {{Assuming the condition is false}} expected-note@-1 {{Left side of '||' is false}}
A().bar();
}
}
@@ -695,9 +695,9 @@
B b = std::move(a.b);
a.b.foo();
#ifdef AGGRESSIVE
- // expected-note@-3{{'b' became 'moved-from' here}}
- // expected-warning@-3{{Method call on a 'moved-from' object 'b'}}
- // expected-note@-4 {{Method call on a 'moved-from' object 'b'}}
+ // expected-note@-3{{Object 'b' is moved}}
+ // expected-warning@-3{{Method called on moved-from object 'b'}}
+ // expected-note@-4 {{Method called on moved-from object 'b'}}
#endif
}
{
@@ -707,21 +707,21 @@
#ifdef AGGRESSIVE
// expected-note@-3{{Calling move constructor for 'A'}}
// expected-note@-4{{Returning from move constructor for 'A'}}
- // expected-warning@-4{{Method call on a 'moved-from' object 'b'}}
- // expected-note@-5{{Method call on a 'moved-from' object 'b'}}
+ // expected-warning@-4{{Method called on moved-from object 'b'}}
+ // expected-note@-5{{Method called on moved-from object 'b'}}
#endif
}
// Don't report a misuse if any SuperRegion is already reported.
{
A a;
- A a1 = 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 a1 = std::move(a); // expected-note {{Object 'a' is moved}}
+ a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
a.b.foo(); // no-warning
}
{
C c;
- C c1 = std::move(c); // expected-note {{'c' became 'moved-from' here}}
- c.foo(); // expected-warning {{Method call on a 'moved-from' object 'c'}} expected-note {{Method call on a 'moved-from' object 'c'}}
+ C c1 = std::move(c); // expected-note {{Object 'c' is moved}}
+ c.foo(); // expected-warning {{Method called on moved-from object 'c'}} expected-note {{Method called on moved-from object 'c'}}
c.b.foo(); // no-warning
}
}
@@ -742,8 +742,8 @@
void reportSuperClass() {
C c;
- C c1 = std::move(c); // expected-note {{'c' became 'moved-from' here}}
- c.foo(); // expected-warning {{Method call on a 'moved-from' object 'c'}} expected-note {{Method call on a 'moved-from' object 'c'}}
+ C c1 = std::move(c); // expected-note {{Object 'c' is moved}}
+ c.foo(); // expected-warning {{Method called on moved-from object 'c'}} expected-note {{Method called on moved-from object 'c'}}
C c2 = c; // no-warning
}
@@ -786,14 +786,14 @@
void foo() {
// Warn even in non-aggressive mode when it comes to STL, because
// in STL the object is left in "valid but unspecified state" after move.
- std::vector<int> W = std::move(V); // expected-note{{'V' became 'moved-from' here}}
- V.push_back(123); // expected-warning{{Method call on a 'moved-from' object 'V'}}
- // expected-note@-1{{Method call on a 'moved-from' object 'V'}}
+ std::vector<int> W = std::move(V); // expected-note{{Object 'V' of type 'std::vector' is left in valid but unspecified state after move}}
+ V.push_back(123); // expected-warning{{Method called on moved-from object 'V'}}
+ // expected-note@-1{{Method called on moved-from object 'V'}}
}
};
void localRValueMove(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}}
- // expected-note@-1{{Method call on a 'moved-from' object}}
+ A b = std::move(a); // expected-note{{Object 'a' is moved}}
+ a.foo(); // expected-warning{{Method called on moved-from object 'a'}}
+ // expected-note@-1{{Method called on moved-from object 'a'}}
}
Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -68,15 +68,22 @@
static ObjectKind classifyObject(const MemRegion *MR,
const CXXRecordDecl *RD);
+ static ObjectKind explainObject(llvm::raw_ostream &OS,
+ const MemRegion *MR, const CXXRecordDecl *RD);
class MovedBugVisitor : public BugReporterVisitor {
public:
- MovedBugVisitor(const MemRegion *R) : Region(R), Found(false) {}
+ MovedBugVisitor(const MemRegion *R, const CXXRecordDecl *RD)
+ : Region(R), RD(RD), Found(false) {}
void Profile(llvm::FoldingSetNodeID &ID) const override {
static int X = 0;
ID.AddPointer(&X);
ID.AddPointer(Region);
+ // Don't add RD because it's, in theory, uniquely determined by
+ // the region. In practice though, it's not always possible to obtain
+ // the declaration directly from the region, that's why we store it
+ // in the first place.
}
std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
@@ -86,6 +93,8 @@
private:
// The tracked region.
const MemRegion *Region;
+ // The class of the tracked object.
+ const CXXRecordDecl *RD;
bool Found;
};
@@ -163,23 +172,21 @@
return nullptr;
Found = true;
- std::string ObjectName;
- if (const auto DecReg =
- unwrapRValueReferenceIndirection(Region)->getAs<DeclRegion>()) {
- const auto *RegionDecl = dyn_cast<NamedDecl>(DecReg->getDecl());
- ObjectName = RegionDecl->getNameAsString();
- }
- std::string InfoText;
- if (ObjectName != "")
- InfoText = "'" + ObjectName + "' became 'moved-from' here";
+ SmallString<128> Str;
+ llvm::raw_svector_ostream OS(Str);
+
+ OS << "Object";
+ ObjectKind OK = explainObject(OS, Region, RD);
+ if (OK.STL)
+ OS << " is left in valid but unspecified state after move";
else
- InfoText = "Became 'moved-from' here";
+ OS << " is moved";
// Generate the extra diagnostic.
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
N->getLocationContext());
- return std::make_shared<PathDiagnosticEventPiece>(Pos, InfoText, true);
-}
+ return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true);
+ }
const ExplodedNode *MoveChecker::getMoveLocation(const ExplodedNode *N,
const MemRegion *Region,
@@ -204,7 +211,7 @@
MisuseKind MK) const {
if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
if (!BT)
- BT.reset(new BugType(this, "Usage of a 'moved-from' object",
+ BT.reset(new BugType(this, "Use-after-move",
"C++ move semantics"));
// Uniqueing report to the same object.
@@ -216,27 +223,25 @@
MoveStmt, C.getSourceManager(), MoveNode->getLocationContext());
// Creating the error message.
- std::string ErrorMessage;
+ llvm::SmallString<128> Str;
+ llvm::raw_svector_ostream OS(Str);
switch(MK) {
case MK_FunCall:
- ErrorMessage = "Method call on a 'moved-from' object";
+ OS << "Method called on moved-from object";
break;
case MK_Copy:
- ErrorMessage = "Copying a 'moved-from' object";
+ OS << "Moved-from object is copied";
break;
case MK_Move:
- ErrorMessage = "Moving a 'moved-from' object";
+ OS << "Moved-from object is moved";
break;
}
- if (const auto DecReg = Region->getAs<DeclRegion>()) {
- const auto *RegionDecl = dyn_cast<NamedDecl>(DecReg->getDecl());
- ErrorMessage += " '" + RegionDecl->getNameAsString() + "'";
- }
+ explainObject(OS, Region, RD);
auto R =
- llvm::make_unique<BugReport>(*BT, ErrorMessage, N, LocUsedForUniqueing,
+ llvm::make_unique<BugReport>(*BT, OS.str(), N, LocUsedForUniqueing,
MoveNode->getLocationContext()->getDecl());
- R->addVisitor(llvm::make_unique<MovedBugVisitor>(Region));
+ R->addVisitor(llvm::make_unique<MovedBugVisitor>(Region, RD));
C.emitReport(std::move(R));
return N;
}
@@ -363,6 +368,25 @@
};
}
+MoveChecker::ObjectKind MoveChecker::explainObject(llvm::raw_ostream &OS,
+ const MemRegion *MR,
+ const CXXRecordDecl *RD) {
+ // We may need a leading space every time we actually explain anything,
+ // and we never know if we are to explain anything until we try.
+ if (const auto DR =
+ dyn_cast_or_null<DeclRegion>(unwrapRValueReferenceIndirection(MR))) {
+ const auto *RegionDecl = cast<NamedDecl>(DR->getDecl());
+ OS << " '" << RegionDecl->getNameAsString() << "'";
+ }
+ ObjectKind OK = classifyObject(MR, RD);
+ if (OK.STL) {
+ OS << " of type '" << RD->getQualifiedNameAsString() << "'";
+ }
+ // Provide the caller with the classification of the object
+ // we've obtained here accidentally, for later use.
+ return OK;
+}
+
void MoveChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
ProgramStateRef State = C.getState();
const LocationContext *LC = C.getLocationContext();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
