[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...
Name: D41151.130516.patch
Type: text/x-patch
Size: 29632 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180119/0e7b2e1b/attachment-0001.bin>

More information about the cfe-commits mailing list