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



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

Reply via email to