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

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

Reply via email to