balazske marked 2 inline comments as done.
balazske added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:50-68
+struct StreamErrorState {
+  // The error state of an opened stream.
+  // EofError: EOF condition (feof returns true)
+  // OtherError: other (non-EOF) error (ferror returns true)
+  // AnyError: EofError or OtherError
+  enum Kind { EofError, OtherError, AnyError } K;
+
----------------
Szelethus wrote:
> Shouldn't we merge this with `StreamState`?
The intention was that the error state is only stored when the stream is opened 
(in opened state). Additionally it exists in the map only if there is error, so 
no "NoError" kind is needed. This is only to save memory, if it is not relevant 
I can move the error information into `StreamState` (that will contain two 
enums then).


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:92-125
+class MakeRetVal {
+  const CallExpr *CE = nullptr;
+  std::unique_ptr<DefinedSVal> RetVal;
+  SymbolRef RetSym;
+
+public:
+  MakeRetVal(const CallEvent &Call, CheckerContext &C)
----------------
Szelethus wrote:
> Do you have other patches that really crave the need for this class? Why 
> isn't `CallEvent::getReturnValue` sufficient? This is a legitimate question, 
> I really don't know. :)
This is an "interesting" solution for the problem that there is need for a 
function with 3 return values. The constructor performs the task of the 
function: Create a conjured value (and get the various objects for it). The 
output values are RetVal and RetSym, and the success state, and the call expr 
that is computed here anyway. It could be computed independently but if the 
value was retrieved once it is better to store it for later use. (I did not 
check how costly that operation is.)

I had some experience that using only `getReturnValue` and make constraints on 
that does not work as intended, and the reason can be that we need to bind a 
value for the call expr otherwise it is an unknown (undefined?) value (and not 
the conjured symbol)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



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

Reply via email to