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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: include/clang/Analysis/ConstructionContext.h:119
+  static const ConstructionContext *
+  finalize(BumpVectorContext &C, const ConstructionContextLayer *TopLayer);
+
----------------
a.sidorin wrote:
> Maybe just `build()`? For me, `finalize()` strongly associates with Java's broken clean-up mechanism.
Renamed into `createFromLayers()`.


================
Comment at: lib/Analysis/CFG.cpp:4737
+        }
+        case ConstructionContext::SimpleVariableKind: {
+          const auto *DSCC = cast<SimpleVariableConstructionContext>(CC);
----------------
dcoughlin wrote:
> 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.
Yeah, the only reason i don't do verbose printing is because changes in tests are massive.


================
Comment at: lib/Analysis/ConstructionContext.cpp:49
+  // patterns.
+  if (const Stmt *S = TopLayer->getTriggerStmt()) {
+    if (const auto *DS = dyn_cast<DeclStmt>(S)) {
----------------
dcoughlin wrote:
> 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".
Actually two places, the other one being `findConstructionContext()`.


https://reviews.llvm.org/D43533