[PATCH] D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

This looks great to me Artem! I'll be very curious to see how you extend this to handle initializer lists.



================
Comment at: lib/Analysis/CFG.cpp:4737
+        }
+        case ConstructionContext::SimpleVariableKind: {
+          const auto *DSCC = cast<SimpleVariableConstructionContext>(CC);
----------------
Eventually (not now) I think it would be great to include the construction context kind in the printed CFG. This would make it easier to understand at a glance the context.


================
Comment at: lib/Analysis/ConstructionContext.cpp:49
+  // patterns.
+  if (const Stmt *S = TopLayer->getTriggerStmt()) {
+    if (const auto *DS = dyn_cast<DeclStmt>(S)) {
----------------
I like how this puts all the messy pattern matching in one place. The follows the general LLVM guidelines of "if it has to be messy, put it all in one place and hide the messiness from everything else".


================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:643
 
-    if (ParentExpr && isa<CXXNewExpr>(ParentExpr) &&
+    if (CC && isa<NewAllocatedObjectConstructionContext>(CC) &&
         !Opts.mayInlineCXXAllocator())
----------------
This is much more semantically clear. Thank you!!


https://reviews.llvm.org/D43533