[analyzer][NFC] Remove dead code and modernize surroundings

[steakhal]

Summary

Thanks @kazu for helping me clean these parts in D127799.

I'm leaving the dump methods, along with the unused visitor handlers and
the forwarding methods.

The dead parts actually helped to uncover two bugs, to which I'm going
to post separate patches.

[martong]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

929–930

Why should we keep this?

[steakhal]

remove dead code from MallocChecker

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

929–930

Accident xD

[martong]

LGTM

clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

 namespace {
 class CFNumberChecker : public Checker< check::PreStmt<CallExpr> > {
   mutable std::unique_ptr<APIMisuse> BT;
   mutable IdentifierInfo *ICreate, *IGetValue;
 public:
   CFNumberChecker() : ICreate(nullptr), IGetValue(nullptr) {}
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
-
-private:
-  void EmitError(const TypedRegion* R, const Expr *Ex,
-                uint64_t SourceSize, uint64_t TargetSize, uint64_t NumberKind);
 };
 } // end anonymous namespace
 
 enum CFNumberType {
   kCFNumberSInt8Type = 1,
   kCFNumberSInt16Type = 2,
   kCFNumberSInt32Type = 3,
   kCFNumberSInt64Type = 4,

clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

class DynamicTypePropagation:
     public Checker< check::PreCall,
                     check::PostCall,
                     check::DeadSymbols,
                     check::PostStmt<CastExpr>,
                     check::PostStmt<CXXNewExpr>,
                     check::PreObjCMessage,
                     check::PostObjCMessage > {
 
-  const ObjCObjectType *getObjectTypeForAllocAndNew(const ObjCMessageExpr *MsgE,
-                                                    CheckerContext &C) const;
-
   /// Return a better dynamic type if one can be derived from the cast.
   const ObjCObjectPointerType *getBetterObjCType(const Expr *CastE,
                                                  CheckerContext &C) const;
 
   ExplodedNode *dynamicTypePropagationOnCasts(const CastExpr *CE,
                                               ProgramStateRef &State,
                                               CheckerContext &C) const;
 

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

while (N) {
     N = N->getFirstPred();
   }
   return nullptr;
 }
 
 namespace {
 class FuchsiaHandleSymbolVisitor final : public SymbolVisitor {
 public:
-  FuchsiaHandleSymbolVisitor(ProgramStateRef State) : State(std::move(State)) {}
-  ProgramStateRef getState() const { return State; }
-
   bool VisitSymbol(SymbolRef S) override {
     if (const auto *HandleType = S->getType()->getAs<TypedefType>())
       if (HandleType->getDecl()->getName() == HandleTypeName)
         Symbols.push_back(S);
     return true;
   }
 
   SmallVector<SymbolRef, 1024> GetSymbols() { return Symbols; }
 
 private:
   SmallVector<SymbolRef, 1024> Symbols;
-  ProgramStateRef State;
 };
 } // end anonymous namespace
 
 /// Returns the symbols extracted from the argument or empty vector if it cannot
 /// be found. It is unlikely to have over 1024 symbols in one argument.
 static SmallVector<SymbolRef, 1024>
 getFuchsiaHandleSymbols(QualType QT, SVal Arg, ProgramStateRef State) {
   int PtrToHandleLevel = 0;
   while (QT->isAnyPointerType() || QT->isReferenceType()) {
     ++PtrToHandleLevel;
     QT = QT->getPointeeType();
   }
   if (QT->isStructureType()) {
     // If we see a structure, see if there is any handle referenced by the
     // structure.
-    FuchsiaHandleSymbolVisitor Visitor(State);
+    FuchsiaHandleSymbolVisitor Visitor;
     State->scanReachableSymbols(Arg, Visitor);
     return Visitor.GetSymbols();
   }
   if (const auto *HandleType = QT->getAs<TypedefType>()) {
     if (HandleType->getDecl()->getName() != HandleTypeName)
       return {};
     if (PtrToHandleLevel > 1)
       // Not supported yet.

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

bool contains(ArgIdxTy ArgIdx) const {
     if (llvm::is_contained(DiscreteArgs, ArgIdx))
       return true;
 
     return VariadicIndex && ArgIdx >= *VariadicIndex;
   }
 
   bool isEmpty() const { return DiscreteArgs.empty() && !VariadicIndex; }
 
-  ArgVecTy ArgsUpTo(ArgIdxTy LastArgIdx) const {
-    ArgVecTy Args;
-    for (ArgIdxTy I = ReturnValueIndex; I <= LastArgIdx; ++I) {
-      if (contains(I))
-        Args.push_back(I);
-    }
-    return Args;
-  }
-
 private:
   ArgVecTy DiscreteArgs;
   Optional<ArgIdxTy> VariadicIndex;
 };
 
 /// A struct used to specify taint propagation rules for a function.
 ///
 /// If any of the possible taint source arguments is tainted, all of the
void parseConfig(const std::string &Option,
                    TaintConfiguration::Propagation &&P,
                    RulesContTy &Rules) const;
 
   CheckerManager &Mgr;
 };
 
 class GenericTaintChecker : public Checker<check::PreCall, check::PostCall> {
 public:
-  static void *getTag() {
-    static int Tag;
-    return &Tag;
-  }
-
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
                   const char *Sep) const override;
 
   /// Generate a report if the expression is tainted or points to tainted data.
   bool generateReportIfTainted(const Expr *E, StringRef Msg,

clang/lib/StaticAnalyzer/Checkers/Iterator.h

public:
   IteratorPosition reAssign(const MemRegion *NewCont) const {
     return IteratorPosition(NewCont, Valid, Offset);
   }
 
   bool operator==(const IteratorPosition &X) const {
     return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
   }
 
-  bool operator!=(const IteratorPosition &X) const {
-    return Cont != X.Cont || Valid != X.Valid || Offset != X.Offset;
-  }
+  bool operator!=(const IteratorPosition &X) const { return !(*this == X); }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.AddPointer(Cont);
     ID.AddInteger(Valid);
     ID.Add(Offset);
   }
 };
 
public:
   ContainerData newBegin(SymbolRef B) const { return ContainerData(B, End); }
 
   ContainerData newEnd(SymbolRef E) const { return ContainerData(Begin, E); }
 
   bool operator==(const ContainerData &X) const {
     return Begin == X.Begin && End == X.End;
   }
 
-  bool operator!=(const ContainerData &X) const {
-    return Begin != X.Begin || End != X.End;
-  }
+  bool operator!=(const ContainerData &X) const { return !(*this == X); }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.Add(Begin);
     ID.Add(End);
   }
 };
 
 class IteratorSymbolMap {};

clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp

 
 public:
   IteratorModeling() = default;
 
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const;
   void checkPostStmt(const UnaryOperator *UO, CheckerContext &C) const;
   void checkPostStmt(const BinaryOperator *BO, CheckerContext &C) const;
-  void checkPostStmt(const CXXConstructExpr *CCE, CheckerContext &C) const;
-  void checkPostStmt(const DeclStmt *DS, CheckerContext &C) const;
   void checkPostStmt(const MaterializeTemporaryExpr *MTE,
                      CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
 };
 
 bool isSimpleComparisonOperator(OverloadedOperatorKind OK);
 bool isSimpleComparisonOperator(BinaryOperatorKind OK);

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

public:
   NoOwnershipChangeVisitor(SymbolRef Sym, const MallocChecker *Checker)
       : NoStateChangeFuncVisitor(bugreporter::TrackingKind::Thorough), Sym(Sym),
         Checker(*Checker) {}
 
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     static int Tag = 0;
     ID.AddPointer(&Tag);
     ID.AddPointer(Sym);
   }
-
-  void *getTag() const {
-    static int Tag = 0;
-    return static_cast<void *>(&Tag);
-  }
 };
 
 } // end anonymous namespace
 
 //===----------------------------------------------------------------------===//
 // Definition of MallocBugVisitor.
 //===----------------------------------------------------------------------===//
 
 namespace {

clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

 };
 } // end of anonymous namespace
 
 namespace {
 class MoveChecker
     : public Checker<check::PreCall, check::PostCall,
                      check::DeadSymbols, check::RegionChanges> {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
   void checkPreCall(const CallEvent &MC, CheckerContext &C) const;
   void checkPostCall(const CallEvent &MC, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   ProgramStateRef
   checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
                      ArrayRef<const MemRegion *> RequestedRegions,
                      ArrayRef<const MemRegion *> InvalidatedRegions,

clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp

inline SymbolRef getArraySym(const Expr *E, CheckerContext &C) const {
     SymbolRef ArraySym = ArrayRef.getAsSymbol();
     return ArraySym;
   }
 
   void addSizeInfo(const Expr *Array, const Expr *Size,
                    CheckerContext &C) const;
 
 public:
-  /// A tag to id this checker.
-  static void *getTag() { static int Tag; return &Tag; }
-
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
                                      const InvalidatedSymbols &Escaped,
                                      const CallEvent *Call,
                                      PointerEscapeKind Kind) const;
 
   void printState(raw_ostream &OS, ProgramStateRef State,

clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp

 };
 
 } // end anonymous namespace
 
 /// The state of the checker is a map from tracked stream symbols to their
 /// state. Let's store it in the ProgramState.
 REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
 
-namespace {
-class StopTrackingCallback final : public SymbolVisitor {
-  ProgramStateRef state;
-public:
-  StopTrackingCallback(ProgramStateRef st) : state(std::move(st)) {}
-  ProgramStateRef getState() const { return state; }
-
-  bool VisitSymbol(SymbolRef sym) override {
-    state = state->remove<StreamMap>(sym);
-    return true;
-  }
-};
-} // end anonymous namespace
-
 SimpleStreamChecker::SimpleStreamChecker()
     : OpenFn("fopen"), CloseFn("fclose", 1) {
   // Initialize the bug types.
   DoubleCloseBugType.reset(
       new BugType(this, "Double fclose", "Unix Stream API Error"));
 
   // Sinks are higher importance bugs as well as calls to assert() or exit(0).
   LeakBugType.reset(

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

: public Checker<eval::Call, check::DeadSymbols, check::RegionChanges,
                      check::LiveSymbols> {
 
   bool isBoolConversionMethod(const CallEvent &Call) const;
 
 public:
   // Whether the checker should model for null dereferences of smart pointers.
   bool ModelSmartPtrDereference = false;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
-  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   ProgramStateRef
   checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
                      const LocationContext *LCtx, const CallEvent *Call) const;
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,

clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp

 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 using namespace clang;
 using namespace ento;
 using namespace taint;
 
 namespace {
-class TaintTesterChecker : public Checker< check::PostStmt<Expr> > {
-
-  mutable std::unique_ptr<BugType> BT;
-  void initBugType() const;
-
-  /// Given a pointer argument, get the symbol of the value it contains
-  /// (points to).
-  SymbolRef getPointedToSymbol(CheckerContext &C,
-                               const Expr* Arg,
-                               bool IssueWarning = true) const;
+class TaintTesterChecker : public Checker<check::PostStmt<Expr>> {
+  std::unique_ptr<BugType> BT =
+      std::make_unique<BugType>(this, "Tainted data", "General");
 
 public:
   void checkPostStmt(const Expr *E, CheckerContext &C) const;
 };
 }
 
-inline void TaintTesterChecker::initBugType() const {
-  if (!BT)
-    BT.reset(new BugType(this, "Tainted data", "General"));
-}
-
 void TaintTesterChecker::checkPostStmt(const Expr *E,
                                        CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   if (!State)
     return;
 
   if (isTainted(State, E, C.getLocationContext())) {
     if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-      initBugType();
       auto report = std::make_unique<PathSensitiveBugReport>(*BT, "tainted", N);
       report->addRange(E->getSourceRange());
       C.emitReport(std::move(report));
     }
   }
 }
 
 void ento::registerTaintTesterChecker(CheckerManager &mgr) {
   mgr.registerChecker<TaintTesterChecker>();
 }
 
 bool ento::shouldRegisterTaintTesterChecker(const CheckerManager &mgr) {
   return true;
 }

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

NoStoreFuncVisitor(const SubRegion *R, bugreporter::TrackingKind TKind)
         PP(MmrMgr.getContext().getPrintingPolicy()) {}
 
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     static int Tag = 0;
     ID.AddPointer(&Tag);
     ID.AddPointer(RegionOfInterest);
   }
 
-  void *getTag() const {
-    static int Tag = 0;
-    return static_cast<void *>(&Tag);
-  }
-
 private:
   /// \return Whether \c RegionOfInterest was modified at \p CurrN compared to
   /// the value it holds in \p CallExitBeginN.
   virtual bool
   wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
                             const ExplodedNode *CallExitBeginN) override;
 
   /// Attempts to find the region of interest in a given record decl,

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

 /// ConstructedObjectKey is used for being able to find the path-sensitive
 /// memory region of a freshly constructed object while modeling the AST node
 /// that syntactically represents the object that is being constructed.
 /// Semantics of such nodes may sometimes require access to the region that's
 /// not otherwise present in the program state, or to the very fact that
 /// the construction context was present and contained references to these
 /// AST nodes.
 class ConstructedObjectKey {
-  typedef std::pair<ConstructionContextItem, const LocationContext *>
-      ConstructedObjectKeyImpl;
-
+  using ConstructedObjectKeyImpl =
+      std::pair<ConstructionContextItem, const LocationContext *>;
   const ConstructedObjectKeyImpl Impl;
 
-  const void *getAnyASTNodePtr() const {
-    if (const Stmt *S = getItem().getStmtOrNull())
-      return S;
-    else
-      return getItem().getCXXCtorInitializer();
-  }
-
 public:
   explicit ConstructedObjectKey(const ConstructionContextItem &Item,
                        const LocationContext *LC)
       : Impl(Item, LC) {}
 
   const ConstructionContextItem &getItem() const { return Impl.first; }
   const LocationContext *getLocationContext() const { return Impl.second; }
 

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

#include "clang/StaticAnalyzer/Core/Analyses.def"
   }
 
   void AddCheckerRegistrationFn(std::function<void(CheckerRegistry&)> Fn) override {
     CheckerRegistrationFns.push_back(std::move(Fn));
   }
 
 private:
   void storeTopLevelDecls(DeclGroupRef DG);
-  std::string getFunctionName(const Decl *D);
 
   /// Check if we should skip (not analyze) the given function.
   AnalysisMode getModeForDecl(Decl *D, AnalysisMode Mode);
   void runAnalysisOnTranslationUnit(ASTContext &C);
 
   /// Print \p S to stderr if \c Opts->AnalyzerDisplayProgress is set.
   void reportAnalyzerProgress(StringRef S);
 }; // namespace