[PATCH] D19311: [analyzer] Self Assignment Checker

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Other than adding expected notes for the path notes to the test, this looks good to me. Thanks Ádám!


================
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1698
@@ +1697,3 @@
+PathDiagnosticPiece *
+CXXSelfAssignmentBRVisitor::VisitNode(const ExplodedNode *Succ,
+                                      const ExplodedNode *Pred,
----------------
This is great.

================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:452
@@ -444,5 +451,3 @@
   // inlining when reanalyzing an already inlined function.
-  if (Visited.count(D)) {
-    assert(isa<ObjCMethodDecl>(D) &&
-           "We are only reanalyzing ObjCMethods.");
+  if (Visited.count(D) && isa<ObjCMethodDecl>(D)) {
     const ObjCMethodDecl *ObjCM = cast<ObjCMethodDecl>(D);
----------------
baloghadamsoftware wrote:
> I made a quick measurement on our build server using clang as target code. Real/user/system times were 75/575/18 for minimal and 76/587/18 for regular. This does not seem too much. However, since the semantics of a copy assignment operator is often composed of the semantics of a destructor and the semantics of a copy constructor implementations often put these two functionalities into separate functions and call them from the destructor, copy constructor and copy assignment operator. Such implementations requires regular inlining to be checked.
Ok, makes sense to me.

================
Comment at: test/Analysis/self-assign.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection %s -verify
+
----------------
I think it would be good to add -analyzer-output=text to this test line and check the expected output of that path notes.


https://reviews.llvm.org/D19311