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" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
