[PATCH] D82385: [Analyzer] Fix errors in iterator modeling

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

Hey! See my inline comments, but after those and a quick clang-format, it looks good.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:543
 
-  auto NewState =
+  auto TmpState =
     advancePosition(State, LHS, Op, *value);
----------------
TmpState feels too generic, maybe AdvancedPosition is more descriptive? Just a suggestion.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:614
   auto RegionMap = State->get<IteratorRegionMap>();
+  unsigned Count = 0;
 
----------------
I would welcome a one-line comment here, stating that this variable is here for formatting reasons (so that the messages are **joined** by a newline, and no prefix or postfix newline is present in the output). Or rename to LinesOutput, LinesPrinted or something similar.


================
Comment at: clang/test/Analysis/iterator-modeling.cpp:36
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
-  clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.begin()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i)); // expected-warning-re {{$v.begin(){{$}}}}
 
----------------
It seems important to me that this test is fixed, as we were not testing whether the message really **ends** with `$v.begin()`. 👍 


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

https://reviews.llvm.org/D82385