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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I really like all this detective work and it would be sad to have it forgotten. I would love to see some of your comments in the documentation of symbol reaper.
More specifically:

> When a memory region is live, all its sub-regions and super-regions are automatically live (and vice versa), so it is only necessary to track liveness of base regions.

I think this is non-obvious. If we had all the information it would make perfect sense to have a dead field in an alive struct. But since the liveness analysis is intraprocedural and we cannot know what happens to a struct when it escapes we have no choice but keep the field alive. A more sophisticated analysis (which also likely to be more expensive) could have dead fields in an alive struct in case the struct never escapes.

> The answer is yes, it does work correctly, because scanReachableSymbols always scans the whole super-region chain of every region. Which means that every time the Environment or the Store marks a region as live, all of its super-regions are added to RegionRoots. However, i would still like to add conversion to the base region into markLive(), because this behavior is something that should be guaranteed by SymbolReaper rather implied by callers manually, even if the callers have to do that anyway.

I did not really follow, but probably my understanding if how memory regions work is not correct. If we work with base regions, why do we still need to scan the whole super-region chain?


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

https://reviews.llvm.org/D56632