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

Fri Jul 17 11:09:41 PDT 2020

gamesh411 marked 6 inline comments as done.
gamesh411 added a comment.

Thanks for reviewing this patch this quickly!
I have updated the diff according to your suggestions, but I will not land it till I run a llvm+clang analysis with it.
Do you think non-ctu mode is enough to test the stability?

================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:400
+                 "Either first or second argument should have structure "
+                 "or class type!");
           handleRandomIncrOrDecr(C, OrigExpr, Op, Call.getReturnValue(),
----------------
baloghadamsoftware wrote:
> This is generally true in //C++// that overloaded operators must either be class member or must have at least one class argument. Do we have to assert it in this particular checker?
I think you are right, at second glance this assert seems confusing for someone reading the code for the first time and does not specifically belong to the logic of 'checking an overloaded increment or decrement operation'. Moreover, the special instance that is modelled as an iterator is not strictly tied to the type system (I mean there could be other things that we could model as iterators, not just structs or classes even beside pointer which are handled in another method of this modelling class).
All in all, I removed the assert.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83190