NoQ added inline comments.
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:390
- // FIXME: Remove when we migrate over to just using ValueManager.
SymbolManager &getSymbolManager() { return SymMgr; }
----------------
baloghadamsoftware wrote:
> Is this comment intentionally deleted?
Yeah, i don't think anybody remembers what was that about and there doesn't
seem to be an immediate need in something like that.
Hmm, why did i delete it as part of that revision? I guess because i was moving
these helper methods around. Let me bring them back because this place is
actually better, now that i think about it.
Also i wonder if anybody ever uses this const getter two lines below (or even
passes `ExprEngine` by const reference anywhere). Hmm, seems to compile without
it.
================
Comment at: test/Analysis/symbol-reaper.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
+
----------------
Szelethus wrote:
> Core intentionally left out?
Thx ^.^ No, just still have this habit since before it was decided to make it
mandatory >.<
================
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
----------------
a_sidorin wrote:
> //FIXME?
Yeah, i guess it's a more polite way of expressing it :)
================
Comment at: test/Analysis/symbol-reaper.cpp:31
+ clang_analyzer_eval(glob); // expected-warning{{TRUE}}
+ // expected-warning@-1{{SYMBOL DEAD}}
+}
----------------
Szelethus wrote:
> N00b question: What does `SYMBOL DEAD` mean here exactly?
It's a warning produced by `clang_analyzer_warnOnDeadSymbol(a.x)` when the
value that was in `a.x` (that was there when that function was called) dies.
This is an `ExprInspection` utility that was created in order to test
`SymbolReaper` more directly. See `symbol-reaper.c` for more such tests.
================
Comment at: unittests/StaticAnalyzer/SymbolReaperTest.cpp:52
+ assert(Matches.size() == 1 && "Ambiguous name!");
+ for (BoundNodes &M : Matches)
+ return M.getNodeAs<T>("d");
----------------
a_sidorin wrote:
> It looks like `selectFirst` helper is what you need here.
Wow, this one's handy.
================
Comment at: unittests/StaticAnalyzer/SymbolReaperTest.cpp:97
+ bool HandleTopLevelDecl(DeclGroupRef DG) override {
+ for (auto D: DG)
+ performTest(D);
----------------
a_sidorin wrote:
> Nit: `const auto *D : DG`
Fxd^^
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56632/new/
https://reviews.llvm.org/D56632
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
