[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

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

Szelethus added a comment.

LGTM, I just have one question in the inlines.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:111-114
+    assert((!ES.isFEof() || !IsFilePositionIndeterminate) &&
+           "FilePositionIndeterminate should be false in FEof case.");
+    assert((State == Opened || ErrorState.isNoError()) &&
+           "ErrorState should be None in non-opened stream state.");
----------------
Cool! Though it makes me wish we had a constructor that just constructs a stream in EOF that doesn't require a file position argument, because this will ensure that the state is correct, but its still the user of this interface who is responsible for making it so.

With that said, feel free to leave it as-is, if there ever emerges a desire for such an interface, we can make that happen anytime.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:670
 
-  State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+  // FilePositionIndeterminate is not cleared.
+  State = State->set<StreamMap>(
----------------
But what if I inspect errno and conclude that the file position is intact? Is there such a thing? After a call to stream operation that might leave the file position indicator indeterminate and getting ferror, am I supposed to fseek or reopen the file no matter what?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80018