martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:221
+ if (Handler) {
+ if (!C.wasInlined) {
+ if (Call.getNumArgs() > 1) {
----------------
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`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:222
+ if (!C.wasInlined) {
+ if (Call.getNumArgs() > 1) {
+ (this->**Handler)(C, OrigExpr, Call.getReturnValue(),
----------------
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?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:238
+ if (IdInfo) {
+ if (IdInfo->getName() == "advance") {
+ if (noChangeInPosition(C, Call.getArgSVal(0), OrigExpr)) {
----------------
Should this be `__advance`?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:559
+bool IteratorModeling::noChangeInPosition(CheckerContext &C, SVal Iter,
+ const Expr *CE) const {
----------------
Some comments would be helpful here. Also, should we use this function only
with `advance()` or it could be useful perhaps in other context?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:571
+ const ExplodedNode *N = C.getPredecessor();
+ while (N) {
+ ProgramPoint PP = N->getLocation();
----------------
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?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76361/new/
https://reviews.llvm.org/D76361
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
