[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin accepted this revision.
a.sidorin added a comment.

Thank you! Just some of nits inline.



================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:394
+
+bool ExprEngine::areTemporaryMaterializationsClear(
+    ProgramStateRef State, const LocationContext *FromLC,
----------------
```
for (const auto &I : State->get<TemporaryMaterializations>()) {
  auto *LCtx = I.first.second;
  if (LCtx == FromLC || (LCtx->isParentOf(From) && (!To || To->isParentOf(LCtx)))
     return false;
}
return true;
```
is a bit shorter but less evident so I won't insist.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:396
+    ProgramStateRef State, const LocationContext *FromLC,
+    const LocationContext *ToLC) {
+  const LocationContext *LC = FromLC;
----------------
EndLC? (similar to iterators)


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:400
+    assert(LC && "ToLC must be a parent of FromLC!");
+    for (auto I : State->get<TemporaryMaterializations>())
+      if (I.first.second == LC)
----------------
const auto &?


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:503
+    Key.first->printPretty(Out, nullptr, PP);
+    if (Value)
+      Out << " : " << Value;
----------------
As I see from line 350, `Value` is always non-null. And there is same check on line 659. Am I missing something?


https://reviews.llvm.org/D43497