Whenever the analyzer budget runs out just at the point where std::advance(), std::prev() or std::next() is invoked the function are not inlined. This results in strange behavior such as std::prev(v.end()) equals v.end(). To prevent this model these functions if they were not inlined. It may also happend that although std::advance() is inlined but a function it calls inside (e.g. __advance() in some implementations) is not. This case is also handled in this patch.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Whenever the analyzer budget runs out just at the point where std::advance(), std::prev() or std::next() is invoked the function are not inlined. This results in strange behavior such as std::prev(v.end()) equals v.end(). To prevent this model these functions if they were not inlined. It may also happend that although std::advance() is inlined but a function it calls inside (e.g. __advance() in some implementations) is not. This case is also handled in this patch.
I suppose we could get this strange behavior with std::distance too? Would it be worth to handle that as well here?
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
193 | Perhaps putting this hunk into a separate function or lambda could decrease the nesting level, b/c you could have an early return if there is no Handler. | |
194 | Maybe a comment could be helpful here about the common signature of the prev/next functions. Like: advanceFun ( It it, Distance n = 1 ) By the way, can we call std::advance with one argument? | |
210 | Should this be __advance? | |
584 | Some comments would be helpful here. Also, should we use this function only with advance() or it could be useful perhaps in other context? | |
596 | I have a rough presumption that this hunk is a general pattern to get the previous node for the previous iterator position. So perhaps it would be useful as a standalone free/static member function too? |
Modeling of std::distance() is planned after we model the size of the container properly.
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp | ||
---|---|---|
193 | Early return is not possible because the execution must continue if the is no handler. However, I refactored checkPostCall now, the handling of both overloaded operators and advance-like functions are moved to separate functions. | |
194 | I added them to the map. | |
210 | No. __advance() is non-standard, I cannot rely on it. If the function was std::advance(), it was inlined, but still did not change the iterator's position means that it did not do its job. Probably the function it calls inside (which could be __advance() in most implementations) was not inlined. In this case we model the behavior of std::advance() explicitely. | |
584 | I renamed the function and added a short comment. Is it OK? | |
596 | I moved this to a standalone function and named it accordingly. |
Perhaps putting this hunk into a separate function or lambda could decrease the nesting level, b/c you could have an early return if there is no Handler.