[analyzer] Remove the default value arg from getChecker*Option

Since D57922, the config table contains every checker option, and it's default
value, so having it as an argument for getChecker*Option is redundant.

By the time any of the getChecker*Option function is called, we verified the
value in CheckerRegistry (after D57860), so we can confidently assert here, as
any irregularities detected at this point must be a programmer error. However,
in compatibility mode, verification won't happen, so the default value must be
restored.

This implies something else, other than adding removing one more potential point
of failure -- debug.ConfigDumper will always contain valid values for
checker/package options!

Differential Revision: https://reviews.llvm.org/D59195

llvm-svn: 361042

13 files changed, 55 insertions, 51 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
index 05e0cd81ac8..d0def691893 100644
--- a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -44,8 +44,8 @@ class AnalysisOrderChecker
                      check::LiveSymbols> {
 
   bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
-    return Opts.getCheckerBooleanOption(this, "*", false) ||
-        Opts.getCheckerBooleanOption(this, CallbackName, false);
+    return Opts.getCheckerBooleanOption(this, "*") ||
+           Opts.getCheckerBooleanOption(this, CallbackName);
   }
 
   bool isCallbackEnabled(CheckerContext &C, StringRef CallbackName) const {
diff --git a/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp
index 11a33e50bf5..4fc225056d4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -195,17 +195,17 @@ void ento::registerCloneChecker(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker<CloneChecker>();
 
   Checker->MinComplexity = Mgr.getAnalyzerOptions().getCheckerIntegerOption(
-      Checker, "MinimumCloneComplexity", 50);
+      Checker, "MinimumCloneComplexity");
 
   if (Checker->MinComplexity < 0)
     Mgr.reportInvalidCheckerOptionValue(
         Checker, "MinimumCloneComplexity", "a non-negative value");
 
   Checker->ReportNormalClones = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
-      Checker, "ReportNormalClones", true);
+      Checker, "ReportNormalClones");
 
   Checker->IgnoredFilesPattern = Mgr.getAnalyzerOptions()
-    .getCheckerStringOption(Checker, "IgnoredFilesPattern", "");
+    .getCheckerStringOption(Checker, "IgnoredFilesPattern");
 }
 
 bool ento::shouldRegisterCloneChecker(const LangOptions &LO) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
index b67d6095690..6927ba39c0a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
@@ -1398,7 +1398,7 @@ void ento::registerNonLocalizedStringChecker(CheckerManager &mgr) {
       mgr.registerChecker<NonLocalizedStringChecker>();
   checker->IsAggressive =
       mgr.getAnalyzerOptions().getCheckerBooleanOption(
-          checker, "AggressiveReport", false);
+          checker, "AggressiveReport");
 }
 
 bool ento::shouldRegisterNonLocalizedStringChecker(const LangOptions &LO) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 50d23422c8f..c416b98cf4b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3098,7 +3098,7 @@ void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) {
 void ento::registerDynamicMemoryModeling(CheckerManager &mgr) {
   auto *checker = mgr.registerChecker<MallocChecker>();
   checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption(
-                                                  checker, "Optimistic", false);
+                                                         checker, "Optimistic");
 }
 
 bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
index 2185561fcda..270efede838 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -82,10 +82,10 @@ void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
       mgr.registerChecker<MmapWriteExecChecker>();
   Mwec->ProtExecOv =
     mgr.getAnalyzerOptions()
-      .getCheckerIntegerOption(Mwec, "MmapProtExec", 0x04);
+      .getCheckerIntegerOption(Mwec, "MmapProtExec");
   Mwec->ProtReadOv =
     mgr.getAnalyzerOptions()
-      .getCheckerIntegerOption(Mwec, "MmapProtRead", 0x01);
+      .getCheckerIntegerOption(Mwec, "MmapProtRead");
 }
 
 bool ento::shouldRegisterMmapWriteExecChecker(const LangOptions &LO) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
index 891a350ff23..d8a9af78536 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -752,8 +752,7 @@ void MoveChecker::printState(raw_ostream &Out, ProgramStateRef State,
 void ento::registerMoveChecker(CheckerManager &mgr) {
   MoveChecker *chk = mgr.registerChecker<MoveChecker>();
   chk->setAggressiveness(
-      mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn",
-                                                      "KnownsAndLocals"), mgr);
+      mgr.getAnalyzerOptions().getCheckerStringOption(chk, "WarnOn"), mgr);
 }
 
 bool ento::shouldRegisterMoveChecker(const LangOptions &LO) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index e5beb0dad2f..b7bf9f3db3f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -1208,7 +1208,7 @@ bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) {
     checker->NoDiagnoseCallsToSystemHeaders =                                  \
         checker->NoDiagnoseCallsToSystemHeaders ||                             \
         mgr.getAnalyzerOptions().getCheckerBooleanOption(                      \
-                      checker, "NoDiagnoseCallsToSystemHeaders", false, true); \
+                      checker, "NoDiagnoseCallsToSystemHeaders", true);        \
   }                                                                            \
                                                                                \
   bool ento::shouldRegister##name##Checker(const LangOptions &LO) {            \
diff --git a/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
index 33119c6a18c..1053424ae6f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
@@ -346,7 +346,7 @@ void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
   NumberObjectConversionChecker *Chk =
       Mgr.registerChecker<NumberObjectConversionChecker>();
   Chk->Pedantic =
-      Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic", false);
+      Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic");
 }
 
 bool ento::shouldRegisterNumberObjectConversionChecker(const LangOptions &LO) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
index abc90986f40..09f2fd635b6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -348,7 +348,7 @@ public:
 void ento::registerPaddingChecker(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker<PaddingChecker>();
   Checker->AllowedPad = Mgr.getAnalyzerOptions()
-          .getCheckerIntegerOption(Checker, "AllowedPad", 24);
+          .getCheckerIntegerOption(Checker, "AllowedPad");
   if (Checker->AllowedPad < 0)
     Mgr.reportInvalidCheckerOptionValue(
         Checker, "AllowedPad", "a non-negative value");
diff --git a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
index 187e868fba0..9d608c12d19 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -611,18 +611,15 @@ void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
   AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
   UninitObjCheckerOptions &ChOpts = Chk->Opts;
 
-  ChOpts.IsPedantic =
-      AnOpts.getCheckerBooleanOption(Chk, "Pedantic", /*DefaultVal*/ false);
+  ChOpts.IsPedantic = AnOpts.getCheckerBooleanOption(Chk, "Pedantic");
   ChOpts.ShouldConvertNotesToWarnings = AnOpts.getCheckerBooleanOption(
-      Chk, "NotesAsWarnings", /*DefaultVal*/ false);
+      Chk, "NotesAsWarnings");
   ChOpts.CheckPointeeInitialization = AnOpts.getCheckerBooleanOption(
-      Chk, "CheckPointeeInitialization", /*DefaultVal*/ false);
+      Chk, "CheckPointeeInitialization");
   ChOpts.IgnoredRecordsWithFieldPattern =
-      AnOpts.getCheckerStringOption(Chk, "IgnoreRecordsWithField",
-                                    /*DefaultVal*/ "\"\"");
+      AnOpts.getCheckerStringOption(Chk, "IgnoreRecordsWithField");
   ChOpts.IgnoreGuardedFields =
-      AnOpts.getCheckerBooleanOption(Chk, "IgnoreGuardedFields",
-                                     /*DefaultVal*/ false);
+      AnOpts.getCheckerBooleanOption(Chk, "IgnoreGuardedFields");
 
   std::string ErrorMsg;
   if (!llvm::Regex(ChOpts.IgnoredRecordsWithFieldPattern).isValid(ErrorMsg))
diff --git a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
index c3a9ef8b56a..762c9c1c8d7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -279,8 +279,7 @@ void ento::registerVirtualCallChecker(CheckerManager &mgr) {
   VirtualCallChecker *checker = mgr.registerChecker<VirtualCallChecker>();
 
   checker->IsPureOnly =
-      mgr.getAnalyzerOptions().getCheckerBooleanOption(
-                                                    checker, "PureOnly", false);
+      mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "PureOnly");
 }
 
 bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) {
diff --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 1ac1cc214f0..68b2c052305 100644
--- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -103,8 +103,7 @@ AnalyzerOptions::mayInlineCXXMemberFunction(
 
 StringRef AnalyzerOptions::getCheckerStringOption(StringRef CheckerName,
                                                   StringRef OptionName,
-                                                  StringRef DefaultVal,
-                                                  bool SearchInParents ) const {
+                                                  bool SearchInParents) const {
   assert(!CheckerName.empty() &&
          "Empty checker name! Make sure the checker object (including it's "
          "bases!) if fully initialized before calling this function!");
@@ -117,62 +116,66 @@ StringRef AnalyzerOptions::getCheckerStringOption(StringRef CheckerName,
       return StringRef(I->getValue());
     size_t Pos = CheckerName.rfind('.');
     if (Pos == StringRef::npos)
-      return DefaultVal;
+      break;
+
     CheckerName = CheckerName.substr(0, Pos);
   } while (!CheckerName.empty() && SearchInParents);
-  return DefaultVal;
+
+  llvm_unreachable("Unknown checker option! Did you call getChecker*Option "
+                   "with incorrect parameters? User input must've been "
+                   "verified by CheckerRegistry.");
+
+  return "";
 }
 
 StringRef AnalyzerOptions::getCheckerStringOption(const ento::CheckerBase *C,
                                                   StringRef OptionName,
-                                                  StringRef DefaultVal,
-                                                  bool SearchInParents ) const {
+                                                  bool SearchInParents) const {
   return getCheckerStringOption(
-             C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+                           C->getTagDescription(), OptionName, SearchInParents);
 }
 
 bool AnalyzerOptions::getCheckerBooleanOption(StringRef CheckerName,
                                               StringRef OptionName,
-                                              bool DefaultVal,
-                                              bool SearchInParents ) const {
-  // FIXME: We should emit a warning here if the value is something other than
-  // "true", "false", or the empty string (meaning the default value),
-  // but the AnalyzerOptions doesn't have access to a diagnostic engine.
-  return llvm::StringSwitch<bool>(
+                                              bool SearchInParents) const {
+  auto Ret = llvm::StringSwitch<llvm::Optional<bool>>(
       getCheckerStringOption(CheckerName, OptionName,
-                             DefaultVal ? "true" : "false",
                              SearchInParents))
       .Case("true", true)
       .Case("false", false)
-      .Default(DefaultVal);
+      .Default(None);
+
+  assert(Ret &&
+         "This option should be either 'true' or 'false', and should've been "
+         "validated by CheckerRegistry!");
+
+  return *Ret;
 }
 
 bool AnalyzerOptions::getCheckerBooleanOption(const ento::CheckerBase *C,
                                               StringRef OptionName,
-                                              bool DefaultVal,
-                                              bool SearchInParents ) const {
+                                              bool SearchInParents) const {
   return getCheckerBooleanOption(
-             C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+             C->getTagDescription(), OptionName, SearchInParents);
 }
 
 int AnalyzerOptions::getCheckerIntegerOption(StringRef CheckerName,
                                              StringRef OptionName,
-                                             int DefaultVal,
-                                             bool SearchInParents ) const {
-  int Ret = DefaultVal;
+                                             bool SearchInParents) const {
+  int Ret = 0;
   bool HasFailed = getCheckerStringOption(CheckerName, OptionName,
-                                          std::to_string(DefaultVal),
                                           SearchInParents)
                      .getAsInteger(0, Ret);
-  assert(!HasFailed && "analyzer-config option should be numeric");
+  assert(!HasFailed &&
+         "This option should be numeric, and should've been validated by "
+         "CheckerRegistry!");
   (void)HasFailed;
   return Ret;
 }
 
 int AnalyzerOptions::getCheckerIntegerOption(const ento::CheckerBase *C,
                                              StringRef OptionName,
-                                             int DefaultVal,
-                                             bool SearchInParents ) const {
+                                             bool SearchInParents) const {
   return getCheckerIntegerOption(
-             C->getTagDescription(), OptionName, DefaultVal, SearchInParents);
+                           C->getTagDescription(), OptionName, SearchInParents);
 }
diff --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
index 437e4aaf1a2..d41ca0a8f32 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -324,7 +324,9 @@ static void insertAndValidate(StringRef FullName,
     return;
 
   // Insertion failed, the user supplied this package/checker option on the
-  // command line. If the supplied value is invalid, we'll emit an error.
+  // command line. If the supplied value is invalid, we'll restore the option
+  // to it's default value, and if we're in non-compatibility mode, we'll also
+  // emit an error.
 
   StringRef SuppliedValue = It.first->getValue();
 
@@ -334,6 +336,8 @@ static void insertAndValidate(StringRef FullName,
         Diags.Report(diag::err_analyzer_checker_option_invalid_input)
             << FullOption << "a boolean value";
       }
+
+      It.first->setValue(Option.DefaultValStr);
     }
     return;
   }
@@ -346,6 +350,8 @@ static void insertAndValidate(StringRef FullName,
         Diags.Report(diag::err_analyzer_checker_option_invalid_input)
             << FullOption << "an integer value";
       }
+
+      It.first->setValue(Option.DefaultValStr);
     }
     return;
   }