[PATCH] D104925: [Analyzer] Improve report of file read at end-of-file condition.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Let's simplify this even further!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:419-421
+  const CheckerNameRef CheckerName;
+  SymbolRef StreamSym;
+  const ExplodedNode *NotePosition;
----------------
I guess it's a matter of preference but I really don't understand the need to define a structure when a lambda is sufficient. Lambda is the intended syntax sugar for exactly what you have written.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:425-426
+    if (!BR.isInteresting(StreamSym) ||
+        BR.getBugType().getDescription() != Desc_StreamEof ||
+        CheckerName != BR.getBugType().getCheckerName())
+      return "";
----------------
You can compare `BugType` objects directly (by address).


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:429-435
+    const ExplodedNode *N = BR.getErrorNode();
+    while (N && N != NotePosition) {
+      const StreamState *StreamS = N->getState()->get<StreamMap>(StreamSym);
+      if (!StreamS || !StreamS->ErrorState.FEof)
+        return "";
+      N = N->getFirstPred();
+    }
----------------
This code says "If there exists a state below in the path where the stream is not at EOF, drop the note". The report will not be emitted at all if the stream is not at EOF at the end, right? Are you trying to account for the situation when the stream gets reset into a non-EOF state and therefore you don't need to emit the note above that point?

In such cases the intended solution is to add another note tag to mark the stream uninteresting once it gets reset, because all the history before that point is irrelevant to our report entirely (whatever the previous position was, its get completely overwritten by the reset).

The API for marking objects uninteresting is not yet there but it's suggested in D104300. I believe you should rebase your patch on top of it and take advantage of it as it lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104925