[PATCH] D41151: [analyzer] Adding LoopContext and improve loop modeling
Peter Szecsi via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 18 16:37:06 PST 2018
szepet updated this revision to Diff 130516.
szepet marked 2 inline comments as done.
szepet added a comment.
First, sorry for this delayed update, however, I was working on this and running this on real projects. I wanted to make sure that this update will be complete enough that this patch would not cause any harm (yes, these features are hidden behind a flag but anyway) and not crashes on edge cases I haven't thought of. The core used the fact that LocationContext only contains StackFramce and BlockInvocation and I aimed to eliminate all these code snippets (more like rewrite).
> This thing is very similar to https://reviews.llvm.org/D19979. Do we really need to create a separate LoopContext or we can reuse ScopeContext instead?
> I guess LoopContext can be treated as a sub-class of ScopeContext. And i don't mind having ScopeContext be split into small distinct sub-classes. Because we're stuck in corner cases for covering all possible scopes, while we're fine with covering only simple scopes (it's better than nothing) and lacking a scope from time to time.
In this patch I left it as it was, so a separate context. I agree with Aleksei that yes, it could be implemented as a ScopeContext and checked if it contains a While/Do/For Statement.
On the other hand, I like the idea more, that we split ScopeContext into small distinct subclasses which would result in a more manageable code.
However, this would require refactoring on ScopeContext as well, in order to make it a great base class (like StmtPoint for ProgramPoint). Then, LoopContext would be its only subclass. So, I do not really see the point of doing these changes right now. I think in this state (when ScopeContext not used by the analyzer as I can see) the LoopContext could live as a separate Context and not make any harm. In case when another Context shows up which is similar LoopContext (e.g. ScopeContext) I would happily refactor it but right now, I do not see the point of this.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 29632 bytes
Desc: not available
More information about the cfe-commits