[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

[Peter Szecsi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316852: [analyzer] MisusedMovedObjectChecker: More precise 
warning message (authored by szepet).

Changed prior to commit:
  https://reviews.llvm.org/D38674?vs=118170=120737#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38674

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  cfe/trunk/test/Analysis/MisusedMovedObject.cpp

Index: cfe/trunk/test/Analysis/MisusedMovedObject.cpp
===
--- cfe/trunk/test/Analysis/MisusedMovedObject.cpp
+++ cfe/trunk/test/Analysis/MisusedMovedObject.cpp
@@ -38,6 +38,7 @@
   B() = default;
   B(const B &) = default;
   B(B &&) = default;
+  B& operator=(const B ) = default;
   void operator=(B &) {
 return;
   }
@@ -70,6 +71,12 @@
   A(A &, char *k) {
 moveconstruct(std::move(other));
   }
+  void operator=(const A ) {
+i = other.i;
+d = other.d;
+b = other.b;
+return;
+  }
   void operator=(A &) {
 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: cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -60,6 +60,7 @@
   const char *NL, const char *Sep) const override;
 
 private:
+  enum MisuseKind {MK_FunCall, MK_Copy, MK_Move};
   class MovedBugVisitor : public BugReporterVisitorImpl {
   public:
 MovedBugVisitor(const MemRegion *R) : Region(R), 

[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

Please, commit.


https://reviews.llvm.org/D38674



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.

LGTM


https://reviews.llvm.org/D38674



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D38674



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

[Peter Szecsi via Phabricator via cfe-commits]

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 ) = default;
   void operator=(B &) {
 return;
   }
@@ -70,6 +71,12 @@
   A(A &, char *k) {
 moveconstruct(std::move(other));
   }
+  void operator=(const A ) {
+i = other.i;
+d = other.d;
+b = other.b;
+return;
+  }
   void operator=(A &) {
 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 :