Remove unused check::RegionChanges::wantsRegionChangeUpdate callback

[k-wisniewski]

Summary

Hi!

I've noticed that check::RegionChanges::wantsRegionChangeUpdate is no longer used - as far as the discussion went it has been unused at least since checkPointerEscape has been added. This patch removes unused code as a part of cleanup/improvement in check::RegionChanges that I have undertaken. Let's discuss the necessity of such action here and decide upon it asap, because I need to know what to do with my another patch that adds LocationContext (or CheckerContext) to check::RegionChanges::checkRegionChanges.

[zaks.anna]

Looks great!

[zaks.anna]

lib/StaticAnalyzer/Core/CheckerManager.cpp

535 ↗

(On Diff #78234)

Looks like the other patch leaked in here: you have LCtx.

[k-wisniewski]

Removed a line that leaked from another diff - thanks Anna!

cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h

_checkRegionChanges(void *checker,
                       ProgramStateRef state,
                       const InvalidatedSymbols *invalidated,
                       ArrayRef<const MemRegion *> Explicits,
                       ArrayRef<const MemRegion *> Regions,
                       const CallEvent *Call) {
     return ((const CHECKER *)checker)->checkRegionChanges(state, invalidated,
                                                       Explicits, Regions, Call);
   }
-  template <typename CHECKER>
-  static bool _wantsRegionChangeUpdate(void *checker,
-                                       ProgramStateRef state) {
-    return ((const CHECKER *)checker)->wantsRegionChangeUpdate(state);
-  }
 
 public:
   template <typename CHECKER>
   static void _register(CHECKER *checker, CheckerManager &mgr) {
     mgr._registerForRegionChanges(
           CheckerManager::CheckRegionChangesFunc(checker,
-                                                 _checkRegionChanges<CHECKER>),
-          CheckerManager::WantsRegionChangeUpdateFunc(checker,
-                                            _wantsRegionChangeUpdate<CHECKER>));
+                                                 _checkRegionChanges<CHECKER>));
   }
 };
 
 class PointerEscape {
   template <typename CHECKER>
   static ProgramStateRef
   _checkPointerEscape(void *Checker,
                      ProgramStateRef State,

cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h

//===----------------------------------------------------------------------===//
   /// checkers to aggressively clean up/reduce the checker state and produce
   /// precise diagnostics.
   void runCheckersForDeadSymbols(ExplodedNodeSet &Dst,
                                  const ExplodedNodeSet &Src,
                                  SymbolReaper &SymReaper, const Stmt *S,
                                  ExprEngine &Eng,
                                  ProgramPoint::Kind K);
 
-  /// \brief True if at least one checker wants to check region changes.
-  bool wantsRegionChangeUpdate(ProgramStateRef state);
-
   /// \brief Run checkers for region changes.
   ///
   /// This corresponds to the check::RegionChanges callback.
   /// \param state The current program state.
   /// \param invalidated A set of all symbols potentially touched by the change.
   /// \param ExplicitRegions The regions explicitly requested for invalidation.
   ///   For example, in the case of a function call, these would be arguments.
   /// \param Regions The transitive closure of accessible regions,
//===----------------------------------------------------------------------===//
   
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
                                 const InvalidatedSymbols *symbols,
                                 ArrayRef<const MemRegion *> ExplicitRegions,
                                 ArrayRef<const MemRegion *> Regions,
                                 const CallEvent *Call)>
       CheckRegionChangesFunc;
   
-  typedef CheckerFn<bool (ProgramStateRef)> WantsRegionChangeUpdateFunc;
-
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
                                      const InvalidatedSymbols &Escaped,
                                      const CallEvent *Call,
                                      PointerEscapeKind Kind,
                                      RegionAndSymbolInvalidationTraits *ITraits)>
       CheckPointerEscapeFunc;
   
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
//===----------------------------------------------------------------------===//
   void _registerForEndFunction(CheckEndFunctionFunc checkfn);
 
   void _registerForBranchCondition(CheckBranchConditionFunc checkfn);
 
   void _registerForLiveSymbols(CheckLiveSymbolsFunc checkfn);
 
   void _registerForDeadSymbols(CheckDeadSymbolsFunc checkfn);
 
-  void _registerForRegionChanges(CheckRegionChangesFunc checkfn,
-                                 WantsRegionChangeUpdateFunc wantUpdateFn);
+  void _registerForRegionChanges(CheckRegionChangesFunc checkfn);
 
   void _registerForPointerEscape(CheckPointerEscapeFunc checkfn);
 
   void _registerForConstPointerEscape(CheckPointerEscapeFunc checkfn);
 
   void _registerForEvalAssume(EvalAssumeFunc checkfn);
 
   void _registerForEvalCall(EvalCallFunc checkfn);
private:
   std::vector<CheckEndFunctionFunc> EndFunctionCheckers;
 
   std::vector<CheckBranchConditionFunc> BranchConditionCheckers;
 
   std::vector<CheckLiveSymbolsFunc> LiveSymbolsCheckers;
 
   std::vector<CheckDeadSymbolsFunc> DeadSymbolsCheckers;
 
-  struct RegionChangesCheckerInfo {
-    CheckRegionChangesFunc CheckFn;
-    WantsRegionChangeUpdateFunc WantUpdateFn;
-  };
-  std::vector<RegionChangesCheckerInfo> RegionChangesCheckers;
+  std::vector<CheckRegionChangesFunc> RegionChangesCheckers;
 
   std::vector<CheckPointerEscapeFunc> PointerEscapeCheckers;
 
   std::vector<EvalAssumeFunc> EvalAssumeCheckers;
 
   std::vector<EvalCallFunc> EvalCallCheckers;
 
   std::vector<CheckEndOfTranslationUnit> EndOfTranslationUnitCheckers;

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

public:
   /// Called by CoreEngine when the analysis worklist has terminated.
   void processEndWorklist(bool hasWorkRemaining) override;
 
   /// evalAssume - Callback function invoked by the ConstraintManager when
   ///  making assumptions about state values.
   ProgramStateRef processAssume(ProgramStateRef state, SVal cond,
                                 bool assumption) override;
 
-  /// wantsRegionChangeUpdate - Called by ProgramStateManager to determine if a
-  ///  region change should trigger a processRegionChanges update.
-  bool wantsRegionChangeUpdate(ProgramStateRef state) override;
-
   /// processRegionChanges - Called by ProgramStateManager whenever a change is made
   ///  to the store. Used to update checkers that track region values.
   ProgramStateRef 
   processRegionChanges(ProgramStateRef state,
                        const InvalidatedSymbols *invalidated,
                        ArrayRef<const MemRegion *> ExplicitRegions,
                        ArrayRef<const MemRegion *> Regions,
                        const CallEvent *Call) override;

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h

public:
   // Generate the first post callsite node.
   virtual void processCallExit(ExplodedNode *Pred) = 0;
 
   /// Called by ConstraintManager. Used to call checker-specific
   /// logic for handling assumptions on symbolic values.
   virtual ProgramStateRef processAssume(ProgramStateRef state,
                                        SVal cond, bool assumption) = 0;
 
-  /// wantsRegionChangeUpdate - Called by ProgramStateManager to determine if a
-  ///  region change should trigger a processRegionChanges update.
-  virtual bool wantsRegionChangeUpdate(ProgramStateRef state) = 0;
-
   /// processRegionChanges - Called by ProgramStateManager whenever a change is
   /// made to the store. Used to update checkers that track region values.
   virtual ProgramStateRef 
   processRegionChanges(ProgramStateRef state,
                        const InvalidatedSymbols *invalidated,
                        ArrayRef<const MemRegion *> ExplicitRegions,
                        ArrayRef<const MemRegion *> Regions,
                        const CallEvent *Call) = 0;

cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

public:
   CStringChecksFilter Filter;
 
   static void *getTag() { static int tag; return &tag; }
 
   bool evalCall(const CallExpr *CE, CheckerContext &C) const;
   void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef state, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
-  bool wantsRegionChangeUpdate(ProgramStateRef state) const;
 
   ProgramStateRef
     checkRegionChanges(ProgramStateRef state,
                        const InvalidatedSymbols *,
                        ArrayRef<const MemRegion *> ExplicitRegions,
                        ArrayRef<const MemRegion *> Regions,
                        const CallEvent *Call) const;
 
DefinedOrUnknownSVal strLength =
         getCStringLength(C, state, Init, StrVal).castAs<DefinedOrUnknownSVal>();
 
     state = state->set<CStringLength>(MR, strLength);
   }
 
   C.addTransition(state);
 }
 
-bool CStringChecker::wantsRegionChangeUpdate(ProgramStateRef state) const {
-  CStringLengthTy Entries = state->get<CStringLength>();
-  return !Entries.isEmpty();
-}
-
 ProgramStateRef
 CStringChecker::checkRegionChanges(ProgramStateRef state,
                                    const InvalidatedSymbols *,
                                    ArrayRef<const MemRegion *> ExplicitRegions,
                                    ArrayRef<const MemRegion *> Regions,
                                    const CallEvent *Call) const {
   CStringLengthTy Entries = state->get<CStringLength>();
   if (Entries.isEmpty())

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

public:
 
   ProgramStateRef
   checkRegionChanges(ProgramStateRef state,
                      const InvalidatedSymbols *invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
                      const CallEvent *Call) const;
 
-  bool wantsRegionChangeUpdate(ProgramStateRef state) const {
-    return true;
-  }
-
   void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
   void checkReturnWithRetEffect(const ReturnStmt *S, CheckerContext &C,
                                 ExplodedNode *Pred, RetEffect RE, RefVal X,
                                 SymbolRef Sym, ProgramStateRef state) const;
 
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   void checkEndFunction(CheckerContext &C) const;
 

cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp

void CheckerManager::runCheckersForDeadSymbols(ExplodedNodeSet &Dst,
                                                SymbolReaper &SymReaper,
                                                const Stmt *S,
                                                ExprEngine &Eng,
                                                ProgramPoint::Kind K) {
   CheckDeadSymbolsContext C(DeadSymbolsCheckers, SymReaper, S, Eng, K);
   expandGraphWithCheckers(C, Dst, Src);
 }
 
-/// \brief True if at least one checker wants to check region changes.
-bool CheckerManager::wantsRegionChangeUpdate(ProgramStateRef state) {
-  for (unsigned i = 0, e = RegionChangesCheckers.size(); i != e; ++i)
-    if (RegionChangesCheckers[i].WantUpdateFn(state))
-      return true;
-
-  return false;
-}
-
 /// \brief Run checkers for region changes.
 ProgramStateRef
 CheckerManager::runCheckersForRegionChanges(ProgramStateRef state,
                                     const InvalidatedSymbols *invalidated,
                                     ArrayRef<const MemRegion *> ExplicitRegions,
                                     ArrayRef<const MemRegion *> Regions,
                                     const CallEvent *Call) {
   for (unsigned i = 0, e = RegionChangesCheckers.size(); i != e; ++i) {
     // If any checker declares the state infeasible (or if it starts that way),
     // bail out.
     if (!state)
       return nullptr;
-    state = RegionChangesCheckers[i].CheckFn(state, invalidated,
-                                             ExplicitRegions, Regions, Call);
+    state = RegionChangesCheckers[i](state, invalidated,
   }
   return state;
 }
 
 /// \brief Run checkers to process symbol escape event.
 ProgramStateRef
 CheckerManager::runCheckersForPointerEscape(ProgramStateRef State,
                                    const InvalidatedSymbols &Escaped,

 void CheckerManager::_registerForLiveSymbols(CheckLiveSymbolsFunc checkfn) {
   LiveSymbolsCheckers.push_back(checkfn);
 }
 
 void CheckerManager::_registerForDeadSymbols(CheckDeadSymbolsFunc checkfn) {
   DeadSymbolsCheckers.push_back(checkfn);
 }
 
-void CheckerManager::_registerForRegionChanges(CheckRegionChangesFunc checkfn,
-                                     WantsRegionChangeUpdateFunc wantUpdateFn) {
-  RegionChangesCheckerInfo info = {checkfn, wantUpdateFn};
-  RegionChangesCheckers.push_back(info);
+void CheckerManager::_registerForRegionChanges(CheckRegionChangesFunc checkfn) {
+  RegionChangesCheckers.push_back(checkfn);
 }
 
 void CheckerManager::_registerForPointerEscape(CheckPointerEscapeFunc checkfn){
   PointerEscapeCheckers.push_back(checkfn);
 }
 
 void CheckerManager::_registerForConstPointerEscape(
                                           CheckPointerEscapeFunc checkfn) {

cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

 
 /// evalAssume - Called by ConstraintManager. Used to call checker-specific
 ///  logic for handling assumptions on symbolic values.
 ProgramStateRef ExprEngine::processAssume(ProgramStateRef state,
                                               SVal cond, bool assumption) {
   return getCheckerManager().runCheckersForEvalAssume(state, cond, assumption);
 }
 
-bool ExprEngine::wantsRegionChangeUpdate(ProgramStateRef state) {
-  return getCheckerManager().wantsRegionChangeUpdate(state);
-}
-
 ProgramStateRef
 ExprEngine::processRegionChanges(ProgramStateRef state,
                                  const InvalidatedSymbols *invalidated,
                                  ArrayRef<const MemRegion *> Explicits,
                                  ArrayRef<const MemRegion *> Regions,
                                  const CallEvent *Call) {
   return getCheckerManager().runCheckersForRegionChanges(state, invalidated,
                                                       Explicits, Regions, Call);