[clang] 6f54318 - [analyzer][RetainCount] Remove the CheckOSObject option

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-05-26T13:22:58+02:00
New Revision: 6f5431846bbf3270d8fc605324e8843c5aaf579b

URL: https://github.com/llvm/llvm-project/commit/6f5431846bbf3270d8fc605324e8843c5aaf579b
DIFF: https://github.com/llvm/llvm-project/commit/6f5431846bbf3270d8fc605324e8843c5aaf579b.diff

LOG: [analyzer][RetainCount] Remove the CheckOSObject option

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.
  * sx.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.

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

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
    clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
    clang/test/Analysis/analyzer-config.c
    clang/test/Analysis/test-separate-retaincount.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index f0ad8326929e..bc4b7d00e2d4 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1094,15 +1094,6 @@ def NSErrorChecker : Checker<"NSError">,
 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 "

diff  --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 4bf9beb365f6..280d511e87c5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1481,26 +1481,11 @@ 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) {
@@ -1509,10 +1494,7 @@ bool ento::shouldRegisterRetainCountChecker(const CheckerManager &mgr) {
 
 void ento::registerOSObjectRetainCountChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.getChecker<RetainCountChecker>();
-  if (!getOption(Mgr.getAnalyzerOptions(),
-                 "CheckOSObject",
-                 "false"))
-    Chk->TrackOSObjects = true;
+  Chk->TrackOSObjects = true;
 }
 
 bool ento::shouldRegisterOSObjectRetainCountChecker(const CheckerManager &mgr) {

diff  --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 778467387382..cb3d40688e91 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -99,7 +99,6 @@
 // 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

diff  --git a/clang/test/Analysis/test-separate-retaincount.cpp b/clang/test/Analysis/test-separate-retaincount.cpp
index 621e1d120bbb..41efad452e5a 100644
--- a/clang/test/Analysis/test-separate-retaincount.cpp
+++ b/clang/test/Analysis/test-separate-retaincount.cpp
@@ -5,10 +5,6 @@
 // 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"