[analyzer][UninitializedObjectChecker] Refactored checker options

[Szelethus]

Summary

Since I plan to add a number of new flags, it made sense to encapsulate them in a new struct, in order not to pollute FindUninitializedFieldss constructor with new boolean options with super long names.

This revision practically reverts D50508, since FindUninitializedFields now accesses the pedantic flag anyways. I guess that revision was a poor design choice :^).

[NoQ]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h

61–63

... = false;, just to initialize fields in the constructor :]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h

 //===----- UninitializedObject.h ---------------------------------*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines helper classes for UninitializedObjectChecker and
 // documentation about the logic of it.
 //
-// To read about command line options and a description what this checker does,
-// refer to UninitializedObjectChecker.cpp.
+// The checker reports uninitialized fields in objects created after a
+// constructor call.
+//
+// This checker has several options:
+//   - "Pedantic" (boolean). If its not set or is set to false, the checker
+//     won't emit warnings for objects that don't have at least one initialized
+//     field. This may be set with
+//
+//     `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`.
+//
+//   - "NotesAsWarnings" (boolean). If set to true, the checker will emit a
+//     warning for each uninitalized field, as opposed to emitting one warning
+//     per constructor call, and listing the uninitialized fields that belongs
+//     to it in notes. Defaults to false.
+//
+//     `-analyzer-config \
+//         alpha.cplusplus.UninitializedObject:NotesAsWarnings=true`.
+//
+//   - "CheckPointeeInitialization" (boolean). If set to false, the checker will
+//     not analyze the pointee of pointer/reference fields, and will only check
+//     whether the object itself is initialized. Defaults to false.
+//
+//     `-analyzer-config \
+//         alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true`.
+//
+//     TODO: With some clever heuristics, some pointers should be dereferenced
+//     by default. For example, if the pointee is constructed within the
+//     constructor call, it's reasonable to say that no external object
+//     references it, and we wouldn't generate multiple report on the same
+//     pointee.
+//
+// Most of the following methods as well as the checker itself is defined in
+// UninitializedObjectChecker.cpp.
 //
 // Some methods are implemented in UninitializedPointee.cpp, to reduce the
 // complexity of the main checker file.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_UNINITIALIZEDOBJECT_H
 #define LLVM_CLANG_STATICANALYZER_UNINITIALIZEDOBJECT_H
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 namespace clang {
 namespace ento {
 
+struct UninitObjCheckerOptions {
+  bool IsPedantic = false;
+  bool ShouldConvertNotesToWarnings = false;
+  bool CheckPointeeInitialization =  false;
+};
+
 /// A lightweight polymorphic wrapper around FieldRegion *. We'll use this
 /// interface to store addinitional information about fields. As described
 /// later, a list of these objects (i.e. "fieldchain") will be constructed and
 /// used for printing note messages should an uninitialized value be found.
 class FieldNode {
 protected:
   const FieldRegion *FR;
 

 
 using UninitFieldMap = std::map<const FieldRegion *, llvm::SmallString<50>>;
 
 /// Searches for and stores uninitialized fields in a non-union object.
 class FindUninitializedFields {
   ProgramStateRef State;
   const TypedValueRegion *const ObjectR;
 
-  const bool CheckPointeeInitialization;
+  const UninitObjCheckerOptions Opts;
   bool IsAnyFieldInitialized = false;
 
   FieldChainInfo::FieldChain::Factory ChainFactory;
 
   /// A map for assigning uninitialized regions to note messages. For example,
   ///
   ///   struct A {
   ///     int x;
   ///   };
   ///
   ///   A a;
   ///
   /// After analyzing `a`, the map will contain a pair for `a.x`'s region and
   /// the note message "uninitialized field 'this->x'.
   UninitFieldMap UninitFields;
 
 public:
   /// Constructs the FindUninitializedField object, searches for and stores
   /// uninitialized fields in R.
   FindUninitializedFields(ProgramStateRef State,
                           const TypedValueRegion *const R,
-                          bool CheckPointeeInitialization);
+                          const UninitObjCheckerOptions &Opts);
 
   const UninitFieldMap &getUninitFields() { return UninitFields; }
 
   /// Returns whether the analyzed region contains at least one initialized
   /// field. Note that this includes subfields as well, not just direct ones,
   /// and will return false if an uninitialized pointee is found with
   /// CheckPointeeInitialization enabled.
   bool isAnyFieldInitialized() { return IsAnyFieldInitialized; }

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

 //===----- UninitializedObjectChecker.cpp ------------------------*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines a checker that reports uninitialized fields in objects
 // created after a constructor call.
 //
-// This checker has several options:
-//   - "Pedantic" (boolean). If its not set or is set to false, the checker
-//     won't emit warnings for objects that don't have at least one initialized
-//     field. This may be set with
-//
-//     `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`.
-//
-//   - "NotesAsWarnings" (boolean). If set to true, the checker will emit a
-//     warning for each uninitalized field, as opposed to emitting one warning
-//     per constructor call, and listing the uninitialized fields that belongs
-//     to it in notes. Defaults to false.
-//
-//     `-analyzer-config \
-//         alpha.cplusplus.UninitializedObject:NotesAsWarnings=true`.
-//
-//   - "CheckPointeeInitialization" (boolean). If set to false, the checker will
-//     not analyze the pointee of pointer/reference fields, and will only check
-//     whether the object itself is initialized. Defaults to false.
-//
-//     `-analyzer-config \
-//         alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true`.
-//
-//     TODO: With some clever heuristics, some pointers should be dereferenced
-//     by default. For example, if the pointee is constructed within the
-//     constructor call, it's reasonable to say that no external object
-//     references it, and we wouldn't generate multiple report on the same
-//     pointee.
-//
-// To read about how the checker works, refer to the comments in
-// UninitializedObject.h.
+// To read about command line options and how the checker works, refer to the
+// top of the file and inline comments in UninitializedObject.h.
 //
 // Some of the logic is implemented in UninitializedPointee.cpp, to reduce the
 // complexity of this file.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangSACheckers.h"
 #include "UninitializedObject.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
 
 using namespace clang;
 using namespace clang::ento;
 
 namespace {
 
 class UninitializedObjectChecker : public Checker<check::EndFunction> {
   std::unique_ptr<BuiltinBug> BT_uninitField;
 
 public:
-  // These fields will be initialized when registering the checker.
-  bool IsPedantic;
-  bool ShouldConvertNotesToWarnings;
-  bool CheckPointeeInitialization;
+  // The fields of this struct will be initialized when registering the checker.
+  UninitObjCheckerOptions Opts;
 
   UninitializedObjectChecker()
       : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {}
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
 };
 
 /// A basic field type, that is not a pointer or a reference, it's dynamic and
 /// static type is the same.
void UninitializedObjectChecker::checkEndFunction(
   // This avoids essentially the same error being reported multiple times.
   if (willObjectBeAnalyzedLater(CtorDecl, Context))
     return;
 
   Optional<nonloc::LazyCompoundVal> Object = getObjectVal(CtorDecl, Context);
   if (!Object)
     return;
 
-  FindUninitializedFields F(Context.getState(), Object->getRegion(),
-                            CheckPointeeInitialization);
+  FindUninitializedFields F(Context.getState(), Object->getRegion(), Opts);
 
   const UninitFieldMap &UninitFields = F.getUninitFields();
 
   if (UninitFields.empty())
     return;
 
-  // In non-pedantic mode, if Object's region doesn't contain a single
-  // initialized field, we'll assume that Object was intentionally left
-  // uninitialized.
-  if (!IsPedantic && !F.isAnyFieldInitialized())
-    return;
-
   // There are uninitialized fields in the record.
 
   ExplodedNode *Node = Context.generateNonFatalErrorNode(Context.getState());
   if (!Node)
     return;
 
   PathDiagnosticLocation LocUsedForUniqueing;
   const Stmt *CallSite = Context.getStackFrame()->getCallSite();
   if (CallSite)
     LocUsedForUniqueing = PathDiagnosticLocation::createBegin(
         CallSite, Context.getSourceManager(), Node->getLocationContext());
 
   // For Plist consumers that don't support notes just yet, we'll convert notes
   // to warnings.
-  if (ShouldConvertNotesToWarnings) {
+  if (Opts.ShouldConvertNotesToWarnings) {
     for (const auto &Pair : UninitFields) {
 
       auto Report = llvm::make_unique<BugReport>(
           *BT_uninitField, Pair.second, Node, LocUsedForUniqueing,
           Node->getLocationContext()->getDecl());
       Context.emitReport(std::move(Report));
     }
     return;

 }
 
 //===----------------------------------------------------------------------===//
 //                   Methods for FindUninitializedFields.
 //===----------------------------------------------------------------------===//
 
 FindUninitializedFields::FindUninitializedFields(
     ProgramStateRef State, const TypedValueRegion *const R,
-    bool CheckPointeeInitialization)
-    : State(State), ObjectR(R),
-      CheckPointeeInitialization(CheckPointeeInitialization) {
+    const UninitObjCheckerOptions &Opts)
+    : State(State), ObjectR(R), Opts(Opts) {
 
   isNonUnionUninit(ObjectR, FieldChainInfo(ChainFactory));
+
+  // In non-pedantic mode, if ObjectR doesn't contain a single initialized
+  // field, we'll assume that Object was intentionally left uninitialized.
+  if (!Opts.IsPedantic && !isAnyFieldInitialized())
+    UninitFields.clear();
 }
 
 bool FindUninitializedFields::addFieldToUninits(FieldChainInfo Chain) {
   if (State->getStateManager().getContext().getSourceManager().isInSystemHeader(
           Chain.getUninitRegion()->getDecl()->getLocation()))
     return false;
 
   UninitFieldMap::mapped_type NoteMsgBuf;
std::string clang::ento::getVariableName(const FieldDecl *Field) {
   }
 
   return Field->getName();
 }
 
 void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
   auto Chk = Mgr.registerChecker<UninitializedObjectChecker>();
 
-  Chk->IsPedantic = Mgr.getAnalyzerOptions().getBooleanOption(
+  AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
+  UninitObjCheckerOptions &ChOpts = Chk->Opts;
+
+  ChOpts.IsPedantic = AnOpts.getBooleanOption(
       "Pedantic", /*DefaultVal*/ false, Chk);
-  Chk->ShouldConvertNotesToWarnings = Mgr.getAnalyzerOptions().getBooleanOption(
+  ChOpts.ShouldConvertNotesToWarnings = AnOpts.getBooleanOption(
       "NotesAsWarnings", /*DefaultVal*/ false, Chk);
-  Chk->CheckPointeeInitialization = Mgr.getAnalyzerOptions().getBooleanOption(
+  ChOpts.CheckPointeeInitialization = AnOpts.getBooleanOption(
       "CheckPointeeInitialization", /*DefaultVal*/ false, Chk);
 }

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp

 //===----- UninitializedPointee.cpp ------------------------------*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines functions and methods for handling pointers and references
 // to reduce the size and complexity of UninitializedObjectChecker.cpp.
 //
-// To read about command line options and a description what this checker does,
-// refer to UninitializedObjectChecker.cpp.
-//
-// To read about how the checker works, refer to the comments in
-// UninitializedObject.h.
+// To read about command line options and documentation about how the checker
+// works, refer to UninitializedObjectChecker.h.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangSACheckers.h"
 #include "UninitializedObject.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
if (V.isUnknown() || V.getAs<loc::ConcreteInt>()) {
     return false;
   }
 
   if (V.isUndef()) {
     return addFieldToUninits(
         LocalChain.add(LocField(FR, /*IsDereferenced*/ false)));
   }
 
-  if (!CheckPointeeInitialization) {
+  if (!Opts.CheckPointeeInitialization) {
     IsAnyFieldInitialized = true;
     return false;
   }
 
   // At this point the pointer itself is initialized and points to a valid
   // location, we'll now check the pointee.
   llvm::Optional<DereferenceInfo> DerefInfo = dereference(State, FR);
   if (!DerefInfo) {