baloghadamsoftware added a comment. Nice work, but please implement this feature also for non-pointer iterators for the matter of consistence.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:279 + // or on the RHS (eg.: 1 + it). Both cases are modeled. + bool IsItOnLHS = BO->getLHS()->getType()->isPointerType(); + Expr *&ItExpr = IsItOnLHS ? LHS : RHS; ---------------- gamesh411 wrote: > What do you think about this detection strategy? This assumes that the > Iterator being detected is a pointer (and not a used-defined type like STL > iterators etc.). Would you say that this assumption holds every time because > the pointer-iterators are only handled in this checkPostStmt callback and the > traditional iterators in another callback? Instead of `It` please use `Iter`, beacause `It` can be understood as the English //it// pronoun: "Is //it// on the left-hand side?" ================ Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:282 + SVal &OffsetVal = IsItOnLHS ? RVal : LVal; + handlePtrIncrOrDecr(C, ItExpr, BinaryOperator::getOverloadedOperator(OK), + OffsetVal); ---------------- gamesh411 wrote: > During the development of this patch, I saw something related. At the > beginning of handlePtrIncrOrDecr, there is a branch on whether the Expr (2nd > argument) is a pointer. I think that branch could just be an assertion. What > do you think? (or maybe I should create a patch to show what I mean?) I wonder whether this should be implemented here in `checkPostStmt()` ot in `handlePtrIncrOrDecr()`. Your current implementation is in `checkPostStmt()`, in this case we can assert in `handlePtrIncrOrDecl()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83190/new/ https://reviews.llvm.org/D83190 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
