[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Looks good overall. I want to think a bit more about potential additional tests...



================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:232
+    // This case is disallowed by the precondition from the method docstring.
+    assert(false);
+  }
----------------
i think that llvm_unreachable is preferable for this purpose.


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:248
+      // This case is disallowed by the precondition from the method docstring.
+      assert(false);
+    }
----------------
same.


================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:678-684
+    if (const auto *NonConstructExpr = dyn_cast<CallExpr>(S)) {
+      // Note that it is important for the storage location of `S` to be set
+      // before `pushCall`, because the latter uses it to set the storage
+      // location for `return`.
+      auto &ReturnLoc = Env.createStorageLocation(*S);
+      Env.setStorageLocation(*S, ReturnLoc);
+    }
----------------
Why can't this stay in `VisitCallExpr`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131438/new/

https://reviews.llvm.org/D131438