This is an archive of the discontinued LLVM Phabricator instance.

[Analyzer] Split container modeling from iterator modeling
ClosedPublic

Authored by baloghadamsoftware on Jan 28 2020, 6:05 AM.

Details

Summary

Iterator modeling depends on container modeling, but not vice versa. This enables the possibility to arrange these two modeling checkers into separate layers.

There are several advantages for doing this: the first one is that this way we can keep the respective modeling checkers moderately simple and small. Furthermore, this enables creation of checkers on container operations which only depend on the container modeling. Thus iterator modeling can be disabled together with the iterator checkers if they are not needed.

Since many container operations also affect iterators, container modeling also uses the iterator library: it creates iterator positions upon calling the begin() or end() method of a containter (but propagation of the abstract position is left to the iterator modeling), shifts or invalidates iterators according to the rules upon calling a container modifier and rebinds the iterator to a new container upon std::move().

Iterator modeling propagates the abstract iterator position, handles the relations between iterator positions and models iterator operations such as increments and decrements.

Diff Detail

Event Timeline

NoQ added a comment.Jan 28 2020, 11:47 AM

This patch is simply moving code around, right? If so, why did tests need to be removed?

In D73547#1845339, @NoQ wrote:

This patch is simply moving code around, right? If so, why did tests need to be removed?

All of the container begin- and end-tests were removed from iterator-modeling.cpp and moved to container-modelin.cpp except those which do not make sense at the moment: container ends are not touched in modeling insert() and erase() methods. This will probably change in the future but I cannot even add FIXME tests because I do not know every detail yet. If you which, I can add these tests back to container-modeling.cpp as they were in iterator-modeling.cpp.

NoQ accepted this revision.Jan 28 2020, 10:55 PM

Ouch, whoops, i didn't notice the new test file. LGTM then!

This revision is now accepted and ready to land.Jan 28 2020, 10:55 PM
This revision was automatically updated to reflect the committed changes.

I'm late to the party, but have looked at the code and I really, really-really like what you did in this patch! It solves one of my biggest complaints about this project. LGTM!

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
1389

This just works??? I admit that I didn't test very thoroughly whether multiple dependencies would work out, but I'm glad it does.