[PATCH] D56632: [analyzer] Track region liveness only through base regions.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Artem,
This looks perfect, just some stylish issues.



================
Comment at: test/Analysis/symbol-reaper.cpp:13
+  void foo() {
+    // Ugh, maybe just let clang_analyzer_eval() work within callees already?
+    // The glob variable shouldn't keep our symbol alive because
----------------
//FIXME?


================
Comment at: unittests/StaticAnalyzer/SymbolReaperTest.cpp:52
+    assert(Matches.size() == 1 && "Ambiguous name!");
+    for (BoundNodes &M : Matches)
+      return M.getNodeAs<T>("d");
----------------
It looks like `selectFirst` helper is what you need here.


================
Comment at: unittests/StaticAnalyzer/SymbolReaperTest.cpp:53
+    for (BoundNodes &M : Matches)
+      return M.getNodeAs<T>("d");
+    llvm_unreachable("Unable to find declaration!");
----------------
This loop will be executed one time only.


================
Comment at: unittests/StaticAnalyzer/SymbolReaperTest.cpp:97
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+    for (auto D: DG)
+      performTest(D);
----------------
Nit: `const auto *D : DG`


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

https://reviews.llvm.org/D56632