NoQ added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/Environment.cpp:180
- for (; SI != SE; ++SI)
- SymReaper.maybeDead(*SI);
}
----------------
zaks.anna wrote:
> We are removing this because the maybeDead is no longer used, correct?
Yup.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:390
-
- } else {
- // Call checkers with the non-cleaned state so that they could query the
----------------
zaks.anna wrote:
> This removes an optimization to address the following issue:
> "removeDeadBindings is not run right after the last reference to the object
> is lost, which leads to imprecise error reports and failure to report the
> leak in some cases. It's because of how hasDeadSymbols() is implemented. That
> problem is solved if we disable the optimization, but I do not know how that
> effects performance. Maybe we can come up with something more clever.
> "
> I suspect the removal of this optimization causes the performance regression.
> In the patch I sent to the list, this was just a hack to demonstrate what
> causes the issue. I am not sure we should not just remove the optimization...
> The best proposal I have is to trigger remove dead bindings at the end of
> every basic block. This would degrade diagnostics (you will see leaks only at
> the end of the basic block), but should give us performance back or even
> improve performance.
>
> Artem and Devin, WDYT?
>
> Artem, can you experiment with this and investigate if the diagnostics become
> much worse? Maybe send a couple of examples? I suggest we implement this mode
> behind a flag as a separate patch.
This cannot be kept around because `.hasDeadSymbols()` cannot be implemented
correctly.
================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:2452
- SymReaper.maybeDead(*SI);
- }
}
----------------
zaks.anna wrote:
> Looks like we are saying that we should no longer add to maybeDead because
> it's not used.
Yup.
https://reviews.llvm.org/D18860
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
