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

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

Szelethus added a comment.
Herald added a subscriber: ASDenysPetrov.

In D75682#1926889 <https://reviews.llvm.org/D75682#1926889>, @balazske wrote:

> In D75682#1926732 <https://reviews.llvm.org/D75682#1926732>, @Szelethus wrote:
>
> > Riiiight I think I finally get it. You don't want to state split on `feof()` and `ferror()`, but rather on the stream operations!
>
>
> Yes the split is at the operations. I did not think even of splitting at start of `feof()` to determine "backwards" the result of the previous operation. This could be another approach. But the way of split depends on the previous operation (to check if error is possible based on possible constraints on its return value), probably not better (but less state split?).


This is probably why it took a bit for us to understand each other! :) Generally speaking, we try to avoid state splitting as much as possible, but you totally convinced me, this isn't the place where we should be conservative. One should always expect stream operations to potentially fail, and that really does introduce two separate paths of execution.

The main issue that remains is testability, and now that we are on the same page, I see why you were concerned about it. I have a couple ideas:

- For streams where the precise state is unknown (they are not tracked), start tracking. If we explicitly check whether a state is `foef()`, we can rightfully assume both of those possibilities.
- Add debug function similar to `clang_analyzer_express`, like `clang_analyzer_set_eof(FILE *)`, etc.
- Evalulate a less complicated stream modeling function that sets such flags, though I suspect the followup patch is supposed to be the one doing this.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682