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