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

Mon Jul 6 14:47:16 PDT 2020

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

================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:282
+    SVal &OffsetVal = IsItOnLHS ? RVal : LVal;
+    handlePtrIncrOrDecr(C, ItExpr, BinaryOperator::getOverloadedOperator(OK),
+                        OffsetVal);
----------------
baloghadamsoftware wrote:
> 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()`.
I checked and decided it not really worth it to move the logic inside the `handlePtrIncrOrDecr()` function, as that would require us to pass both the left and right-hand side as `Expr`-s. This would be fine when we handle a binary operator, but if we handle a unary operator, we manually pass the SVal of RHS (namely a manually created ConcreteInt with value 1). All this could be abstracted with a function wrapping the original `handlePtrIncrOrDecr()`, but for now, I don't think it is worth it.

================
Comment at: clang/test/Analysis/iterator-modeling.cpp:169
+
+  auto i2 = 2 + i1;
+
----------------
Note that this line requires the parent patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83190