[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

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

Szelethus added a comment.

Also, thank you guys for chipping in!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:394
+        StreamSym, StreamState::getOpenedWithOtherError());
+    C.addTransition(StateEof);
+    C.addTransition(StateOther);
----------------
balazske wrote:
> baloghadamsoftware wrote:
> > xazax.hun wrote:
> > > As you probably know we are splitting the execution paths here. This will potentially result in doubling the analysis tome for a function for each `feof` call. Since state splits can be really bad for performance we need good justification for each of them.
> > > So the questions are:
> > > * Is it really important to differentiate between eof and other error or is it possible to just have an any error state?
> > > * Do we find more bugs in real world code that justifies these state splits?
> > I agree here. Do not introduce such forks without specific reason.
> `feof` and `ferror` should return the same value if called multiple times (and no other file operations in between). If the exact error is not saved in the state, how can we ensure that the calls return the same value?
This is a very interesting question indeed, never crossed my mind before. I think it would be better to move `AnyError` to the next revision and discuss it there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682