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


================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1929-1930
+
+  auto &SymMgr = State->getSymbolManager();
+  auto Sym = SymMgr.conjureSymbol(E, LCtx, T, BlockCount, "end");
+  State = assumeNoOverflow(State, Sym, 4);
----------------
NoQ wrote:
> This is a bit more `auto` than allowed by [[ 
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
>  | our coding standards ]] (it pretty much disallows `auto` unless it's some 
> sort of `dyn_cast` (`getAs`) or an iterator.
I can add the type, of course. However, until now I understood and it is also 
in the coding standard that "other places where the type is already obvious 
from the context". For me it is obvious that `conjureSymbol()` returns a 
`SymbolRef`. Even more obvious is the `getSymbolManager()` returns a 
`SymbolManager`.


================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:2247
 
+ProgramStateRef ensureNonNegativeDiff(ProgramStateRef State, SymbolRef Sym1,
+                                      SymbolRef Sym2) {
----------------
NoQ wrote:
> This looks like a new feature. Is it testable?
I am not sure I can test it alone. Maybe I should leave it for now and add it 
in another patch together with modelling of `empty()` or `size()`. Then I 
should also rename this patch which remains pure refactoring.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61136



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to