[analyzer]StreamChecker refactoring (NFC).

[Szelethus]

LGTM! The code looks a lot cleaner.

[balazske]

Rebase and not using "ProgramStateRef &".

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

CallDescriptionMap<FnCheck> Callbacks = {
       {{"fileno", 1},
        std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)},
   };
 
   void evalFopen(const CallEvent &Call, CheckerContext &C) const;
   void evalFreopen(const CallEvent &Call, CheckerContext &C) const;
   void evalFclose(const CallEvent &Call, CheckerContext &C) const;
   void evalFseek(const CallEvent &Call, CheckerContext &C) const;
-
   void checkArgNullStream(const CallEvent &Call, CheckerContext &C,
                           unsigned ArgI) const;
-  bool checkNullStream(SVal SV, CheckerContext &C,
-                       ProgramStateRef &State) const;
-  void checkFseekWhence(SVal SV, CheckerContext &C,
-                        ProgramStateRef &State) const;
-  bool checkDoubleClose(const CallEvent &Call, CheckerContext &C,
-                        ProgramStateRef &State) const;
+
+  ProgramStateRef checkNullStream(SVal SV, CheckerContext &C,
+                                  ProgramStateRef State) const;
+  ProgramStateRef checkFseekWhence(SVal SV, CheckerContext &C,
+                                   ProgramStateRef State) const;
+  ProgramStateRef checkDoubleClose(const CallEvent &Call, CheckerContext &C,
+                                   ProgramStateRef State) const;
 };
 
 } // end anonymous namespace
 
 REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
 
 
 bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
void StreamChecker::evalFreopen(const CallEvent &Call,
   if (!CE)
     return;
 
   Optional<DefinedSVal> StreamVal = Call.getArgSVal(2).getAs<DefinedSVal>();
   if (!StreamVal)
     return;
   // Do not allow NULL as passed stream pointer.
   // This is not specified in the man page but may crash on some system.
-  checkNullStream(*StreamVal, C, State);
-  // Check if error was generated.
-  if (C.isDifferent())
+  State = checkNullStream(*StreamVal, C, State);
+  if (!State)
     return;
 
   SymbolRef StreamSym = StreamVal->getAsSymbol();
   // Do not care about special values for stream ("(FILE *)0x12345"?).
   if (!StreamSym)
     return;
 
   // Generate state for non-failed case.
StateRetNull =
       StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed());
 
   C.addTransition(StateRetNotNull);
   C.addTransition(StateRetNull);
 }
 
 void StreamChecker::evalFclose(const CallEvent &Call, CheckerContext &C) const {
   ProgramStateRef State = C.getState();
-  if (checkDoubleClose(Call, C, State))
+  State = checkDoubleClose(Call, C, State);
+  if (State)
     C.addTransition(State);
 }
 
 void StreamChecker::evalFseek(const CallEvent &Call, CheckerContext &C) const {
   const Expr *AE2 = Call.getArgExpr(2);
   if (!AE2)
     return;
 
   ProgramStateRef State = C.getState();
 
-  bool StateChanged = checkNullStream(Call.getArgSVal(0), C, State);
-  // Check if error was generated.
-  if (C.isDifferent())
+  State = checkNullStream(Call.getArgSVal(0), C, State);
+  if (!State)
     return;
 
-  // Check the legality of the 'whence' argument of 'fseek'.
+  State =
       checkFseekWhence(State->getSVal(AE2, C.getLocationContext()), C, State);
+  if (!State)
+    return;
 
-  if (!C.isDifferent() && StateChanged)
-    C.addTransition(State);
 }
 
 void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext &C,
                                        unsigned ArgI) const {
   ProgramStateRef State = C.getState();
-  if (checkNullStream(Call.getArgSVal(ArgI), C, State))
+  State = checkNullStream(Call.getArgSVal(ArgI), C, State);
+  if (State)
     C.addTransition(State);
 }
 
-bool StreamChecker::checkNullStream(SVal SV, CheckerContext &C,
-                                    ProgramStateRef &State) const {
+ProgramStateRef StreamChecker::checkNullStream(SVal SV, CheckerContext &C,
+                                               ProgramStateRef State) const {
   Optional<DefinedSVal> DV = SV.getAs<DefinedSVal>();
   if (!DV)
-    return false;
+    return State;
 
   ConstraintManager &CM = C.getConstraintManager();
   ProgramStateRef StateNotNull, StateNull;
   std::tie(StateNotNull, StateNull) = CM.assumeDual(C.getState(), *DV);
 
   if (!StateNotNull && StateNull) {
     if (ExplodedNode *N = C.generateErrorNode(StateNull)) {
       if (!BT_nullfp)
         BT_nullfp.reset(new BuiltinBug(this, "NULL stream pointer",
                                        "Stream pointer might be NULL."));
       C.emitReport(std::make_unique<PathSensitiveBugReport>(
           *BT_nullfp, BT_nullfp->getDescription(), N));
     }
-    return false;
+    return nullptr;
   }
 
-  if (StateNotNull) {
-    State = StateNotNull;
-    return true;
-  }
-
-  return false;
+  return StateNotNull;
 }
 
-void StreamChecker::checkFseekWhence(SVal SV, CheckerContext &C,
-                                     ProgramStateRef &State) const {
+// Check the legality of the 'whence' argument of 'fseek'.
+ProgramStateRef StreamChecker::checkFseekWhence(SVal SV, CheckerContext &C,
+                                                ProgramStateRef State) const {
   Optional<nonloc::ConcreteInt> CI = SV.getAs<nonloc::ConcreteInt>();
   if (!CI)
-    return;
+    return State;
 
   int64_t X = CI->getValue().getSExtValue();
   if (X >= 0 && X <= 2)
-    return;
+    return State;
 
   if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
     if (!BT_illegalwhence)
       BT_illegalwhence.reset(
           new BuiltinBug(this, "Illegal whence argument",
                          "The whence argument to fseek() should be "
                          "SEEK_SET, SEEK_END, or SEEK_CUR."));
     C.emitReport(std::make_unique<PathSensitiveBugReport>(
         *BT_illegalwhence, BT_illegalwhence->getDescription(), N));
+    return nullptr;
   }
+
+  return State;
 }
 
-bool StreamChecker::checkDoubleClose(const CallEvent &Call, CheckerContext &C,
-                                     ProgramStateRef &State) const {
+ProgramStateRef StreamChecker::checkDoubleClose(const CallEvent &Call,
+                                                CheckerContext &C,
+                                                ProgramStateRef State) const {
   SymbolRef Sym = Call.getArgSVal(0).getAsSymbol();
   if (!Sym)
-    return false;
+    return State;
 
   const StreamState *SS = State->get<StreamMap>(Sym);
 
   // If the file stream is not tracked, return.
   if (!SS)
-    return false;
+    return State;
 
   // Check: Double close a File Descriptor could cause undefined behaviour.
   // Conforming to man-pages
   if (SS->isClosed()) {
     ExplodedNode *N = C.generateErrorNode();
     if (N) {
       if (!BT_doubleclose)
         BT_doubleclose.reset(new BuiltinBug(
             this, "Double fclose", "Try to close a file Descriptor already"
                                    " closed. Cause undefined behaviour."));
       C.emitReport(std::make_unique<PathSensitiveBugReport>(
           *BT_doubleclose, BT_doubleclose->getDescription(), N));
+      return nullptr;
     }
-    return false;
+
+    return State;
   }
 
   // Close the File Descriptor.
   State = State->set<StreamMap>(Sym, StreamState::getClosed());
 
-  return true;
+  return State;
 }
 
 void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                      CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   // TODO: Clean up the state.
   const StreamMapTy &Map = State->get<StreamMap>();