[analyzer][RetainCount] Remove the CheckOSObject option

[Szelethus]

Summary

As per http://lists.llvm.org/pipermail/cfe-dev/2019-August/063215.html, lets get rid of this option.

It presents 2 issues that have bugged me for years now:

• OSObject is NOT a boolean option. It in fact has 3 states:
  • osx.OSObjectRetainCount is enabled but OSObject it set to false: RetainCount regards the option as disabled.
  • osx.OSObjectRetainCount is enabled and OSObject it set to true: RetainCount regards the option as enabled.
  • osx.OSObjectRetainCount is disabled: RetainCount regards the option as disabled.
• The hack involves directly modifying AnalyzerOptions::ConfigTable, which shouldn't even be public in the first place.

This still isn't really ideal, because it would be better to preserve the option and remove the checker (we want visible checkers to be associated with diagnostics, and hidden options like this one to be associated with changing how the modeling is done), but backwards compatibility is an issue.

[gamesh411]

I have no authority whatsoever, but if there is no breakage in buildbots, then it means explicitly providing CheckOSObject param is not really valid use-case. I vote for this cleanup.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

def NSErrorChecker : Checker<"NSError">,
   HelpText<"Check usage of NSError** parameters">,
   Dependencies<[NSOrCFErrorDerefChecker]>,
   Documentation<HasDocumentation>;
 
 def RetainCountChecker : Checker<"RetainCount">,
   HelpText<"Check for leaks and improper reference count management">,
   CheckerOptions<[
     CmdLineOption<Boolean,
-                  "CheckOSObject",
-                  "Find violations of retain-release rules applied to XNU "
-                  "OSObject instances. By default, the checker only checks "
-                  "retain-release rules for Objective-C NSObject instances "
-                  "and CoreFoundation objects.",
-                  "true",
-                  InAlpha,
-                  Hide>,
-    CmdLineOption<Boolean,
                   "TrackNSCFStartParam",
                   "Check not only that the code follows retain-release rules "
                   "with respect to objects it allocates or borrows from "
                   "elsewhere, but also that it fulfills its own retain count "
                   "specification with respect to objects that it receives as "
                   "arguments.",
                   "false",
                   Released>

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

 void ento::registerRetainCountBase(CheckerManager &Mgr) {
   Mgr.registerChecker<RetainCountChecker>();
 }
 
 bool ento::shouldRegisterRetainCountBase(const CheckerManager &mgr) {
   return true;
 }
 
-// FIXME: remove this, hack for backwards compatibility:
-// it should be possible to enable the NS/CF retain count checker as
-// osx.cocoa.RetainCount, and it should be possible to disable
-// osx.OSObjectRetainCount using osx.cocoa.RetainCount:CheckOSObject=false.
-static bool getOption(const AnalyzerOptions &Options,
-                      StringRef Postfix,
-                      StringRef Value) {
-  auto I = Options.Config.find(
-    (StringRef("osx.cocoa.RetainCount:") + Postfix).str());
-  if (I != Options.Config.end())
-    return I->getValue() == Value;
-  return false;
-}
-
 void ento::registerRetainCountChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.getChecker<RetainCountChecker>();
   Chk->TrackObjCAndCFObjects = true;
-  Chk->TrackNSCFStartParam = getOption(Mgr.getAnalyzerOptions(),
-                                       "TrackNSCFStartParam",
-                                       "true");
+  Chk->TrackNSCFStartParam = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+      Mgr.getCurrentCheckerName(), "TrackNSCFStartParam");
 }
 
 bool ento::shouldRegisterRetainCountChecker(const CheckerManager &mgr) {
   return true;
 }
 
 void ento::registerOSObjectRetainCountChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.getChecker<RetainCountChecker>();
-  if (!getOption(Mgr.getAnalyzerOptions(),
-                 "CheckOSObject",
-                 "false"))
-    Chk->TrackOSObjects = true;
 }
 
 bool ento::shouldRegisterOSObjectRetainCountChecker(const CheckerManager &mgr) {
   return true;
 }

clang/test/Analysis/analyzer-config.c

 // CHECK-NEXT: optin.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:NotesAsWarnings = false
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:Pedantic = false
 // CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
 // CHECK-NEXT: optin.cplusplus.VirtualCall:ShowFixIts = false
 // CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
 // CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
 // CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false
-// CHECK-NEXT: osx.cocoa.RetainCount:CheckOSObject = true
 // CHECK-NEXT: osx.cocoa.RetainCount:TrackNSCFStartParam = false
 // CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: report-in-main-source-file = false
 // CHECK-NEXT: serialize-stats = false
 // CHECK-NEXT: silence-checkers = ""
 // CHECK-NEXT: stable-report-filename = false
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: track-conditions = true
 // CHECK-NEXT: track-conditions-debug = false
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false

clang/test/Analysis/test-separate-retaincount.cpp

 // RUN: %clang_analyze_cc1 -std=c++14 -DNO_CF_OBJECT -verify %s \
 // RUN:   -analyzer-checker=core,osx \
 // RUN:   -analyzer-disable-checker osx.cocoa.RetainCount
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -DNO_OS_OBJECT -verify %s \
 // RUN:   -analyzer-checker=core,osx \
 // RUN:   -analyzer-disable-checker osx.OSObjectRetainCount
-//
-// RUN: %clang_analyze_cc1 -std=c++14 -DNO_OS_OBJECT -verify %s \
-// RUN:   -analyzer-checker=core,osx \
-// RUN:   -analyzer-config "osx.cocoa.RetainCount:CheckOSObject=false"
 
 #include "os_object_base.h"
 
 typedef const void * CFTypeRef;
 extern CFTypeRef CFRetain(CFTypeRef cf);
 extern void CFRelease(CFTypeRef cf);
 
 #define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))