[PATCH] D83190: [analyzer] Model iterator random incrementation symmetrically

Mon Jul 6 12:53:22 PDT 2020

gamesh411 marked 2 inline comments as done.
gamesh411 added inline comments.

================
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;
----------------
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?

================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:282
+    SVal &OffsetVal = IsItOnLHS ? RVal : LVal;
+    handlePtrIncrOrDecr(C, ItExpr, BinaryOperator::getOverloadedOperator(OK),
+                        OffsetVal);
----------------
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?)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83190/new/

https://reviews.llvm.org/D83190