Szelethus added a comment.
The bug reports speak for themselves, they are awesome. Nice work!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:28-30
+//===----------------------------------------------------------------------===//
+// Definition of state data structures.
+//===----------------------------------------------------------------------===//
----------------
Thank you! Big fan of these.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:34-35
+const char *Desc_StreamEof = "Stream already in EOF";
+const char *Desc_ResourceLeak = "Resource leak";
+
----------------
You don't seem to reuse these in this or the followup patch? Its not a big
deal, I don't mind you creating them, just feels a bit odd to me.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:409
+ SymbolRef StreamSym) const {
+ return C.getNoteTag([StreamSym](PathSensitiveBugReport &BR) {
+ if (!BR.isInteresting(StreamSym) ||
----------------
Can you not capture `this` as well? We could eliminate
`StreamChecker::Instance` that way I suppose.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:715-716
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState);
- C.addTransition(StateFailed);
+ if (IsFread && SS->ErrorState != ErrorFEof)
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ else
----------------
Oh, this took me quite a bit. Can we change `SS` to something that reflects
that its the current error state, not the new one? Like `CurrSS`?
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
