[analyzer] Clean checker options from DefaultBool to bool (NFC)

[vabridgers]

Summary

A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up some of the
instances where bool was used, and could be changed to DefaultBool.

[steakhal]

A recent review emphasized the preference to use DefaultBool instead of bool for checker options.

What I wanted to highlight is that we should aim for consistency. We should either use that everywhere or nowhere.

To be fair, we should probably use raw bools instead.
They could not only default to false, but to true as well.

In addition to this, we should prefer the c++ way of initializing these at the declaration directly to either of those values.
IMO we don't need this DefaultBool abstraction.

[vabridgers]

refactor patch to remove DefaultBool, replace with bool as suggested

[vabridgers]

Hi @steakhal, I believe I've addressed the comments. Please have a look and let me know if this is what you were after. Best!

[steakhal]

Good stuff. Update the summary and title, and I think good to go.

In addition to this, we should do something similar with the rest of the checker options and initialize them at the declaration. But that is a different story.

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

struct ImplicitNullDerefEvent {
   // When true, the dereference is in the source code directly. When false, the
   // dereference might happen later (for example pointer passed to a parameter
   // that is marked with nonnull attribute.)
   bool IsDirectDereference;
 
   static int Tag;
 };
 
-/// A helper class which wraps a boolean value set to false by default.
-///
-/// This class should behave exactly like 'bool' except that it doesn't need to
-/// be explicitly initialized.
-struct DefaultBool {
-  bool val;
-  DefaultBool() : val(false) {}
-  /*implicit*/ operator bool&() { return val; }
-  /*implicit*/ operator const bool&() const { return val; }
-  DefaultBool &operator=(bool b) { val = b; return *this; }
-};
-
 } // end ento namespace
 
 } // end clang namespace
 
 #endif

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

mutable std::unique_ptr<BugType> BT_Null, BT_Bounds, BT_Overlap,
       BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
   mutable const char *CurrentFunctionDescription;
 
 public:
   /// The filter is used to filter out the diagnostics which are not enabled by
   /// the user.
   struct CStringChecksFilter {
-    DefaultBool CheckCStringNullArg;
-    DefaultBool CheckCStringOutOfBounds;
-    DefaultBool CheckCStringBufferOverlap;
-    DefaultBool CheckCStringNotNullTerm;
-    DefaultBool CheckCStringUninitializedRead;
+    bool CheckCStringNullArg = false;
+    bool CheckCStringOutOfBounds = false;
+    bool CheckCStringBufferOverlap = false;
+    bool CheckCStringNotNullTerm = false;
+    bool CheckCStringUninitializedRead = false;
 
     CheckerNameRef CheckNameCStringNullArg;
     CheckerNameRef CheckNameCStringOutOfBounds;
     CheckerNameRef CheckNameCStringBufferOverlap;
     CheckerNameRef CheckNameCStringNotNullTerm;
     CheckerNameRef CheckNameCStringUninitializedRead;
   };
 
#define REGISTER_CHECKER(name)                                                 \
   }                                                                            \
                                                                                \
   bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
 
 REGISTER_CHECKER(CStringNullArg)
 REGISTER_CHECKER(CStringOutOfBounds)
 REGISTER_CHECKER(CStringBufferOverlap)
 REGISTER_CHECKER(CStringNotNullTerm)
-REGISTER_CHECKER(CStringUninitializedRead)
+REGISTER_CHECKER(CStringUninitializedRead)
 

clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

enum CheckKind {
     CK_CXXDeallocationArg,
     CK_ArgInitializedness,
     CK_ArgPointeeInitializedness,
     CK_NilReceiver,
     CK_UndefReceiver,
     CK_NumCheckKinds
   };
 
-  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  bool ChecksEnabled[CK_NumCheckKinds] = {false};
   // The original core.CallAndMessage checker name. This should rather be an
   // array, as seen in MallocChecker and CStringChecker.
   CheckerNameRef OriginalName;
 
   void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;
 
   /// Fill in the return value that results from messaging nil based on the
   /// return type and architecture and diagnose if the return value will be

clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

return T.getVendor() == llvm::Triple::Apple ||
          T.isOSFreeBSD() ||
          T.isOSNetBSD() ||
          T.isOSOpenBSD() ||
          T.isOSDragonFly();
 }
 
 namespace {
 struct ChecksFilter {
-  DefaultBool check_bcmp;
-  DefaultBool check_bcopy;
-  DefaultBool check_bzero;
-  DefaultBool check_gets;
-  DefaultBool check_getpw;
-  DefaultBool check_mktemp;
-  DefaultBool check_mkstemp;
-  DefaultBool check_strcpy;
-  DefaultBool check_DeprecatedOrUnsafeBufferHandling;
-  DefaultBool check_rand;
-  DefaultBool check_vfork;
-  DefaultBool check_FloatLoopCounter;
-  DefaultBool check_UncheckedReturn;
-  DefaultBool check_decodeValueOfObjCType;
+  bool check_bcmp = false;
+  bool check_bcopy = false;
+  bool check_bzero = false;
+  bool check_gets = false;
+  bool check_getpw = false;
+  bool check_mktemp = false;
+  bool check_mkstemp = false;
+  bool check_strcpy = false;
+  bool check_DeprecatedOrUnsafeBufferHandling = false;
+  bool check_rand = false;
+  bool check_vfork = false;
+  bool check_FloatLoopCounter = false;
+  bool check_UncheckedReturn = false;
+  bool check_decodeValueOfObjCType = false;
 
   CheckerNameRef checkName_bcmp;
   CheckerNameRef checkName_bcopy;
   CheckerNameRef checkName_bzero;
   CheckerNameRef checkName_gets;
   CheckerNameRef checkName_getpw;
   CheckerNameRef checkName_mktemp;
   CheckerNameRef checkName_mkstemp;

clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp

 using namespace ento;
 
 namespace {
 class CloneChecker
     : public Checker<check::ASTCodeBody, check::EndOfTranslationUnit> {
 public:
   // Checker options.
   int MinComplexity;
-  bool ReportNormalClones;
+  bool ReportNormalClones = false;
   StringRef IgnoredFilesPattern;
 
 private:
   mutable CloneDetector Detector;
   mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
 
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,

clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostStmt(const CastExpr *CastE, CheckerContext &C) const;
   void checkPostStmt(const CXXNewExpr *NewE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
 
   /// This value is set to true, when the Generics checker is turned on.
-  DefaultBool CheckGenerics;
+  bool CheckGenerics = false;
   CheckerNameRef GenericCheckName;
 };
 
 bool isObjCClassType(QualType Type) {
   if (const auto *PointerType = dyn_cast<ObjCObjectPointerType>(Type)) {
     return PointerType->getObjectType()->isObjCClass();
   }
   return false;

clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp

 #include "llvm/ADT/SmallString.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 struct ChecksFilter {
   /// Check for missing invalidation method declarations.
-  DefaultBool check_MissingInvalidationMethod;
+  bool check_MissingInvalidationMethod = false;
   /// Check that all ivars are invalidated.
-  DefaultBool check_InstanceVariableInvalidation;
+  bool check_InstanceVariableInvalidation = false;
 
   CheckerNameRef checkName_MissingInvalidationMethod;
   CheckerNameRef checkName_InstanceVariableInvalidation;
 };
 
 class IvarInvalidationCheckerImpl {
   typedef llvm::SmallSetVector<const ObjCMethodDecl*, 2> MethodSet;
   typedef llvm::DenseMap<const ObjCMethodDecl*,

clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

int getLocalizedArgumentForSelector(const IdentifierInfo *Receiver,
                                       Selector S) const;
 
 public:
   NonLocalizedStringChecker();
 
   // When this parameter is set to true, the checker assumes all
   // methods that return NSStrings are unlocalized. Thus, more false
   // positives will be reported.
-  DefaultBool IsAggressive;
+  bool IsAggressive = false;
 
   void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;
   void checkPostStmt(const ObjCStringLiteral *SL, CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
 };
 

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

: public Checker<check::DeadSymbols, check::PointerEscape,
                      check::EndFunction, check::PreCall, check::PostCall,
                      check::NewAllocator, check::PostStmt<BlockExpr>,
                      check::PostObjCMessage, check::Location, eval::Assume> {
 public:
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
   /// In optimistic mode, the checker assumes that all user-defined functions
   /// which might free a pointer are annotated.
-  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
+  bool ShouldIncludeOwnershipAnnotatedFunctions = false;
 
-  DefaultBool ShouldRegisterNoOwnershipChangeVisitor;
+  bool ShouldRegisterNoOwnershipChangeVisitor = false;
 
   /// Many checkers are essentially built into this one, so enabling them will
   /// make MallocChecker perform additional modeling and reporting.
   enum CheckKind {
     /// When a subchecker is enabled but MallocChecker isn't, model memory
     /// management but do not emit warnings emitted with MallocChecker only
     /// enabled.
     CK_MallocChecker,
     CK_NewDeleteChecker,
     CK_NewDeleteLeaksChecker,
     CK_MismatchedDeallocatorChecker,
     CK_InnerPointerChecker,
     CK_NumCheckKinds
   };
 
   using LeakInfo = std::pair<const ExplodedNode *, const MemRegion *>;
 
-  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  bool ChecksEnabled[CK_NumCheckKinds] = {false};
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkNewAllocator(const CXXAllocatorCall &Call, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;

clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp

 namespace {
 class NSOrCFErrorDerefChecker
     : public Checker< check::Location,
                         check::Event<ImplicitNullDerefEvent> > {
   mutable IdentifierInfo *NSErrorII, *CFErrorII;
   mutable std::unique_ptr<NSErrorDerefBug> NSBT;
   mutable std::unique_ptr<CFErrorDerefBug> CFBT;
 public:
-  DefaultBool ShouldCheckNSError, ShouldCheckCFError;
+  bool ShouldCheckNSError = false, ShouldCheckCFError = false;
   CheckerNameRef NSErrorName, CFErrorName;
   NSOrCFErrorDerefChecker() : NSErrorII(nullptr), CFErrorII(nullptr) {}
 
   void checkLocation(SVal loc, bool isLoad, const Stmt *S,
                      CheckerContext &C) const;
   void checkEvent(ImplicitNullDerefEvent event) const;
 };
 }

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

 
 public:
   // If true, the checker will not diagnose nullabilility issues for calls
   // to system headers. This option is motivated by the observation that large
   // projects may have many nullability warnings. These projects may
   // find warnings about nullability annotations that they have explicitly
   // added themselves higher priority to fix than warnings on calls to system
   // libraries.
-  DefaultBool NoDiagnoseCallsToSystemHeaders;
+  bool NoDiagnoseCallsToSystemHeaders = false;
 
   void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
   void checkPostStmt(const ExplicitCastExpr *CE, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   void checkEvent(ImplicitNullDerefEvent Event) const;
   void checkLocation(SVal Location, bool IsLoad, const Stmt *S,
                      CheckerContext &C) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
                   const char *Sep) const override;
 
   enum CheckKind {
     CK_NullPassedToNonnull,
     CK_NullReturnedFromNonnull,
     CK_NullableDereferenced,
     CK_NullablePassedToNonnull,
     CK_NullableReturnedFromNonnull,
     CK_NumCheckKinds
   };
 
-  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  bool ChecksEnabled[CK_NumCheckKinds] = {false};
   CheckerNameRef CheckNames[CK_NumCheckKinds];
   mutable std::unique_ptr<BugType> BTs[CK_NumCheckKinds];
 
   const std::unique_ptr<BugType> &getBugType(CheckKind Kind) const {
     if (!BTs[Kind])
       BTs[Kind].reset(new BugType(CheckNames[Kind], "Nullability",
                                   categories::MemoryError));
     return BTs[Kind];
   }
 
   // When set to false no nullability information will be tracked in
   // NullabilityMap. It is possible to catch errors like passing a null pointer
   // to a callee that expects nonnull argument without the information that is
   // stroed in the NullabilityMap. This is an optimization.
-  DefaultBool NeedTracking;
+  bool NeedTracking = false;
 
 private:
   class NullabilityBugVisitor : public BugReporterVisitor {
   public:
     NullabilityBugVisitor(const MemRegion *M) : Region(M) {}
 
     void Profile(llvm::FoldingSetNodeID &ID) const override {
       static int X = 0;

clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

 public:
   enum LockingSemantics { NotApplicable = 0, PthreadSemantics, XNUSemantics };
   enum CheckerKind {
     CK_PthreadLockChecker,
     CK_FuchsiaLockChecker,
     CK_C11LockChecker,
     CK_NumCheckKinds
   };
-  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  bool ChecksEnabled[CK_NumCheckKinds] = {false};
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
 private:
   typedef void (PthreadLockChecker::*FnCheck)(const CallEvent &Call,
                                               CheckerContext &C,
                                               CheckerKind CheckKind) const;
   CallDescriptionMap<FnCheck> PThreadCallbacks = {
       // Init.

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

 class SmartPtrModeling
     : 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.
-  DefaultBool ModelSmartPtrDereference;
+  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,

clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp

 
 public:
   enum CheckKind {
     CK_StackAddrEscapeChecker,
     CK_StackAddrAsyncEscapeChecker,
     CK_NumCheckKinds
   };
 
-  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  bool ChecksEnabled[CK_NumCheckKinds] = {false};
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &Ctx) const;
 
 private:
   void checkReturnedBlockCaptures(const BlockDataRegion &B,

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
 
   enum CheckKind {
     CK_StdCLibraryFunctionArgsChecker,
     CK_StdCLibraryFunctionsTesterChecker,
     CK_NumCheckKinds
   };
-  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  bool ChecksEnabled[CK_NumCheckKinds] = {false};
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   bool DisplayLoadedSummaries = false;
   bool ModelPOSIX = false;
   bool ShouldAssumeControlledEnvironment = false;
 
 private:
   Optional<Summary> findFunctionSummary(const FunctionDecl *FD,

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

 
 namespace {
 
 class UnixAPIMisuseChecker : public Checker< check::PreStmt<CallExpr> > {
   mutable std::unique_ptr<BugType> BT_open, BT_pthreadOnce;
   mutable Optional<uint64_t> Val_O_CREAT;
 
 public:
-  DefaultBool CheckMisuse, CheckPortability;
+  bool CheckMisuse = false, CheckPortability = false;
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 
   void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
   void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
   void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
 
   void CheckOpenVariant(CheckerContext &C,

clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp

 public:
   enum CheckKind {
     CK_Uninitialized,
     CK_Unterminated,
     CK_CopyToSelf,
     CK_NumCheckKinds
   };
 
-  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  bool ChecksEnabled[CK_NumCheckKinds] = {false};
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   void checkPreStmt(const VAArgExpr *VAA, CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
 
 private:
   const MemRegion *getVAListAsRegion(SVal SV, const Expr *VAExpr,