This is an archive of the discontinued LLVM Phabricator instance.

[StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

Authored by szepet on Jul 20 2017, 2:56 AM.

Diff Detail


Event Timeline

szepet created this revision.Jul 20 2017, 2:56 AM
whisperity removed a subscriber: whisperity.
szepet updated this revision to Diff 108312.Jul 26 2017, 9:46 AM
szepet added a subscriber: cfe-commits.

Accidentally left debug print removed.

dcoughlin added inline comments.Aug 7 2017, 9:07 PM
658 ↗(On Diff #108312)

Can you add a comment explaining what meaning of this program point is.

586 ↗(On Diff #108312)

I'm surprised both this and the checks for N's location above are needed. How does this arise?

szepet updated this revision to Diff 110981.Aug 14 2017, 8:40 AM
szepet marked an inline comment as done.

Updates based on comments.

szepet added inline comments.Aug 14 2017, 8:43 AM
658 ↗(On Diff #108312)

Can you help me with that? I am not sure what is important to say about this point to understand it better than from its name.

586 ↗(On Diff #108312)

We don't need both of the checks, I just left it by mistake.

dcoughlin accepted this revision.Aug 14 2017, 11:25 AM

Looks good to me! (Please expand the comment, though.)

658 ↗(On Diff #108312)

My recommendation would be to think about it from the perspective of someone new to the codebase trying to understand when the analyzer will reach such a program point. What additional context will they need that is not conveyed in the name alone?

At a minimum, I think it is worth mentioning that a LoopExit point can arise even when a loop is not entered and that not all loop exits will result in a LoopExit program point.

This revision is now accepted and ready to land.Aug 14 2017, 11:25 AM

Also, please mention in the commit message that tests will be added in a following commit.

This revision was automatically updated to reflect the committed changes.