[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Its very interesting to see how the idea of interestingness propagation through `NoteTag`s works in practice -- I feel like many other checkers will need to adopt something like this, and its great that we have the first use case presented here. The patch from afar looks good, I only left some nits to ease the readability for the uninitiated.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp:91-111
   SVal V = C.getSVal(CE->getArg(0));
   const auto *Pos = getIteratorPosition(State, V);
+  SVal Field = Default;
+
   if (Pos) {
-    State = State->BindExpr(CE, C.getLocationContext(), get(Pos));
-  } else {
-    State = State->BindExpr(CE, C.getLocationContext(), Default);
+    Field = get(Pos);
   }
----------------
Now that we have multiple `SVal`s around, can we rename `V`? Also, I would appreciate some comments. As I understand it, `ExprInspectionChecker` now marks the arguments as interesting, so if we write this:
```lang=cpp
clang_analyzer_express(clang_analyzer_iterator_position(i2));
```
`clang_analyzer_iterator_position(i2)` will be interesting, and this function propagates this interestingness to `i2`, correct?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp:103
+      C.getNoteTag([V, Field](PathSensitiveBugReport &BR) -> std::string {
+        auto *PSBR = cast<PathSensitiveBugReport>(&BR);
+        if (PSBR->isInteresting(Field)) {
----------------
We won't need this anymore :)


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:432
 
-  reportBug(*Str, C);
+  reportBug(*Str, C, Optional<SVal>(ArgVal));
 }
----------------
Why the need for the `Optional` stuff here?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:108-109
                               SVal It2 = UndefinedVal()) const;
+  const NoteTag *getInterestingnessPropagationTag(CheckerContext &C, SVal It1,
+                                                  SVal It2) const;
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
----------------
The usage of this function is very non-obvious, plenty of comments would be appreciated here as well.


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

https://reviews.llvm.org/D75677