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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
