[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.

[George Karpenkov via Phabricator via cfe-commits]

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