george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.

requesting changes due to minor nits inline



================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:152
+    }
+  }
+  void Profile(llvm::FoldingSetNodeID &ID) const {
----------------
newline after the brace. here and elsewhere within this class.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:161
+  bool operator<(const ConstructedObjectKey &Other) const {
+    return std::make_pair(P, LC) < std::make_pair(Other.P, Other.LC);
+  }
----------------
would it be faster to store a pair in the first place, instead of constructing 
it on each comparison? Is this operator required to be in a GDM?


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:756
   SVal FieldLoc;
+  bool ObjectUnderConstructionNeedsCleanup = false;
 
----------------
gotta love state variables used 70 lines later on. Could we add a comment here? 
What is the semantics of the cleanup action? Running destructors? Maybe a 
better name would make this more readable?

Also would it be worse to change `finishObjectConstruction` function to not do 
anything when there's no object being constructed, and then we could simply 
call it on all code paths?


https://reviews.llvm.org/D47350



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

Reply via email to