Szelethus created this revision.
Szelethus added reviewers: NoQ, vsavchenko, martong, baloghadamsoftware,
xazax.hun, balazske, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso,
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet,
whisperity.
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 `bool`ean 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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78097
Files:
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
Index: clang/test/Analysis/test-separate-retaincount.cpp
===================================================================
--- clang/test/Analysis/test-separate-retaincount.cpp
+++ 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"
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -89,7 +89,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
@@ -106,4 +105,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 103
+// CHECK-NEXT: num-entries = 102
Index:
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1481,26 +1481,11 @@
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 @@
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)
{
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1027,15 +1027,6 @@
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 "
Index: clang/test/Analysis/test-separate-retaincount.cpp
===================================================================
--- clang/test/Analysis/test-separate-retaincount.cpp
+++ 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"
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -89,7 +89,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
@@ -106,4 +105,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 103
+// CHECK-NEXT: num-entries = 102
Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1481,26 +1481,11 @@
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 @@
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) {
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1027,15 +1027,6 @@
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 "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
