Szelethus added inline comments.

================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:644
   Dependencies<[ContainerModeling]>,
-  Documentation<NotDocumented>,
-  Hidden;
-
-def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">,
-  HelpText<"Check for use of invalidated iterators">,
-  Dependencies<[IteratorModeling]>,
-  Documentation<HasAlphaDocumentation>;
-
-def IteratorRangeChecker : Checker<"IteratorRange">,
-  HelpText<"Check for iterators used outside their valid ranges">,
-  Dependencies<[IteratorModeling]>,
-  Documentation<HasAlphaDocumentation>;
-
-def MismatchedIteratorChecker : Checker<"MismatchedIterator">,
-  HelpText<"Check for use of iterators of different containers where iterators 
"
-           "of the same container are expected">,
-  Dependencies<[IteratorModeling]>,
-  Documentation<HasAlphaDocumentation>;
-
-} // end: "alpha.cplusplus"
+  Documentation<NotDocumented>;
 
----------------
This checker should be `Hidden`.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:37-38
+
+  typedef bool (STLAlgorithmModeling::*FnCheck)(CheckerContext &,
+                                                const CallExpr *) const;
+
----------------
Prefer `using`. D67706#inline-614013


================
Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:67
+public:
+  STLAlgorithmModeling() {}
+
----------------
` = default;`


================
Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:126
+  // assume that in case of successful search the position of the found element
+  // is ahed of it.
+  const auto *Pos = getIteratorPosition(State, SecondParam);
----------------
NoQ wrote:
> Typo: *ahead.
Hold on, isn't it the other way around?

```
[_|_|x|_|_|_|_|y|_|_|_|z|_]
```
Suppose in the range `[x, z)` I'm looking for `y`. The range-end argument would 
be `z`, isn't `y` behind it? The following code and the test cases seem to be 
correct, so I guess its the comment?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:129-130
+  if (Pos) {
+    StateFound = createIteratorPosition(StateFound, RetVal, 
Pos->getContainer(),
+                                        CE, LCtx, C.blockCount());
+    const auto *NewPos = getIteratorPosition(StateFound, RetVal);
----------------
What if the range-end iterator is a reverse iterator? Wouldn't it mess with the 
relative position of the found element?

```
[_|_|x|_|_|_|_|y|_|_|_|z|_]

std::find(z, x, y);
```

Suppose I'm searching in the `(x, z]` range for `y`, where `x`,  `z` are 
reverse iterators. Here, you'll create a forward iterator for `y`, if I'm not 
mistaken, and you'd say that its behind `x`? Are these things correctly modeled 
in IteratorChecker?


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

https://reviews.llvm.org/D70818



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

Reply via email to