[PATCH] D80009: [Analyzer][StreamChecker] Changed representation of stream error state - NFC.

Mon May 18 05:52:39 PDT 2020

Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

LGTM! Amazing job! Sorry for this dragging out for so long, but I'm very happy with how this patch and the overall direction turned out. I've left an annoying nit, and as always, feel free address it and commit without another round of review.

> The error state can not always be determined from the last failed function and it was not the best design.

It was a great idea, still!

================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:40-45
+  /// There is an execution path with none of the error flags set.
+  bool NoError = true;
+  /// There is an execution path with EOF indicator set.
+  bool FEof = false;
+  /// There is an execution path with error indicator set.
+  bool FError = false;
----------------
I see what you mean, but in the context of the analyzer, I'd prefer to not use "there is an execution path with .*". Would this imply an a new branch in the exploded graph? Is this execution path modeled or does it only exist conceptually? I think we should avoid this ambiguity by saying something else, like "The steam may or may not have .* flag set".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80009