baloghadamsoftware marked 6 inline comments as done.
baloghadamsoftware added inline comments.
================
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:1328-1337
+def ContainerInspectionChecker : Checker<"ContainerInspection">,
+ HelpText<"Check the analyzer's understanding of C++ containers">,
+ Dependencies<[IteratorModeling]>,
+ Documentation<NotDocumented>;
+
+def IteratorInspectionChecker : Checker<"IteratorInspection">,
+ HelpText<"Check the analyzer's understanding of C++ iterators">,
----------------
Szelethus wrote:
> NoQ wrote:
> > Dunno, i would keep it all in one checker, just to save a few `// RUN:`
> > lines :)
> I agree. Let's combine these into `DebugIteratorModeling`, because, as I
> understand it, that is what we're doing!
I did it on the short term, but on the long term iterator and container
modelling are planned to be two different things, where iterator modelling
depends on container modelling, but the latter works alone as well to support
container checkers, such as e.g. copying data into a container with
insufficient size. Furthermore I do not really like the name
`debug.DebugIteratorModeling`.
================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:243-244
+ Getter get) const;
+ void analyzerContainerBegin(const CallExpr *CE, CheckerContext &C) const;
+ void analyzerContainerEnd(const CallExpr *CE, CheckerContext &C) const;
+ template <typename Getter>
----------------
NoQ wrote:
> We usually define such getters for stuff that the programmer cannot obtain
> otherwise during normal program execution. These two functions look like
> they're probably equivalent to normal `.begin()` and `.end()` calls. I don't
> really object but do we really ever need them other than for testing the
> trivial implementations of `.begin()` and `.end()`?
Not exactly. These functions return the internal representation of their
`.begin()` and `.end()`. These symbols are conjured by the iterator checker and
are bound to the return value of `.begin()` and `.end()` in the container data
map. It is an extra check to check whether they return the same internal symbol
as `clang_analyzer_iterator_position()` for the return value of `.begin()` and
`.end()`.
================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1689
+ if (CE->getNumArgs() == 0) {
+ reportDebugMsg("Missing container argument", C);
+ return;
----------------
Szelethus wrote:
> Ah, right, so this would fire for `clang_analyzer_iterator_position()`?
Yes, and also for the other two.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67156/new/
https://reviews.llvm.org/D67156
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
