[analyzer] Conversion to CheckerFamily: NSOrCFErrorDerefChecker (#151171)

This commit converts the class `NSOrCFErrorDerefChecker` to the checker
family framework and simplifies some parts of the implementation (e.g.
removes two very trivial subclasses of `BugType`).

This commit is almost NFC, the only technically "functional" change is
that it removes the hidden modeling checker `NSOrCFErrorDerefChecker`
which was only relevant as an implementation detail of the old checker
registration procedure.

Author: NagyDonat

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

@@ -1047,11 +1047,6 @@ def RetainCountBase : Checker<"RetainCountBase">,
 
 let ParentPackage = OSX in {
 
-def NSOrCFErrorDerefChecker : Checker<"NSOrCFErrorDerefChecker">,
-  HelpText<"Implementation checker for NSErrorChecker and CFErrorChecker">,
-  Documentation<NotDocumented>,
-  Hidden;
-
 def NumberObjectConversionChecker : Checker<"NumberObjectConversion">,
   HelpText<"Check for erroneous conversions of objects representing numbers "
            "into numbers">,
@@ -1149,9 +1144,8 @@ def ObjCSuperCallChecker : Checker<"MissingSuperCall">,
   Documentation<NotDocumented>;
 
 def NSErrorChecker : Checker<"NSError">,
-  HelpText<"Check usage of NSError** parameters">,
-  Dependencies<[NSOrCFErrorDerefChecker]>,
-  Documentation<HasDocumentation>;
+                     HelpText<"Check usage of NSError** parameters">,
+                     Documentation<HasDocumentation>;
 
 def RetainCountChecker : Checker<"RetainCount">,
   HelpText<"Check for leaks and improper reference count management">,
@@ -1253,9 +1247,8 @@ def CFRetainReleaseChecker : Checker<"CFRetainRelease">,
   Documentation<HasDocumentation>;
 
 def CFErrorChecker : Checker<"CFError">,
-  HelpText<"Check usage of CFErrorRef* parameters">,
-  Dependencies<[NSOrCFErrorDerefChecker]>,
-  Documentation<HasDocumentation>;
+                     HelpText<"Check usage of CFErrorRef* parameters">,
+                     Documentation<HasDocumentation>;
 
 } // end "osx.coreFoundation"
 

clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp

@@ -142,34 +142,19 @@ void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
 //===----------------------------------------------------------------------===//
 
 namespace {
+class NSOrCFErrorDerefChecker
+    : public CheckerFamily<check::Location,
+                           check::Event<ImplicitNullDerefEvent>> {
+  mutable IdentifierInfo *NSErrorII = nullptr;
+  mutable IdentifierInfo *CFErrorII = nullptr;
 
-class NSErrorDerefBug : public BugType {
-public:
-  NSErrorDerefBug(const CheckerNameRef Checker)
-      : BugType(Checker, "NSError** null dereference",
-                "Coding conventions (Apple)") {}
-};
-
-class CFErrorDerefBug : public BugType {
 public:
-  CFErrorDerefBug(const CheckerNameRef Checker)
-      : BugType(Checker, "CFErrorRef* null dereference",
-                "Coding conventions (Apple)") {}
-};
-
-}
+  CheckerFrontendWithBugType NSError{"NSError** null dereference",
+                                     "Coding conventions (Apple)"};
+  CheckerFrontendWithBugType CFError{"CFErrorRef* null dereference",
+                                     "Coding conventions (Apple)"};
 
-namespace {
-class NSOrCFErrorDerefChecker
-    : public Checker< check::Location,
-                        check::Event<ImplicitNullDerefEvent> > {
-  mutable IdentifierInfo *NSErrorII, *CFErrorII;
-  mutable std::unique_ptr<NSErrorDerefBug> NSBT;
-  mutable std::unique_ptr<CFErrorDerefBug> CFBT;
-public:
-  bool ShouldCheckNSError = false, ShouldCheckCFError = false;
-  CheckerNameRef NSErrorName, CFErrorName;
-  NSOrCFErrorDerefChecker() : NSErrorII(nullptr), CFErrorII(nullptr) {}
+  StringRef getDebugTag() const override { return "NSOrCFErrorDerefChecker"; }
 
   void checkLocation(SVal loc, bool isLoad, const Stmt *S,
                      CheckerContext &C) const;
@@ -236,12 +221,12 @@ void NSOrCFErrorDerefChecker::checkLocation(SVal loc, bool isLoad,
   if (!CFErrorII)
     CFErrorII = &Ctx.Idents.get("CFErrorRef");
 
-  if (ShouldCheckNSError && IsNSError(parmT, NSErrorII)) {
+  if (NSError.isEnabled() && IsNSError(parmT, NSErrorII)) {
     setFlag<NSErrorOut>(state, state->getSVal(loc.castAs<Loc>()), C);
     return;
   }
 
-  if (ShouldCheckCFError && IsCFError(parmT, CFErrorII)) {
+  if (CFError.isEnabled() && IsCFError(parmT, CFErrorII)) {
     setFlag<CFErrorOut>(state, state->getSVal(loc.castAs<Loc>()), C);
     return;
   }
@@ -274,19 +259,9 @@ void NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const {
 
   os  << " may be null";
 
-  BugType *bug = nullptr;
-  if (isNSError) {
-    if (!NSBT)
-      NSBT.reset(new NSErrorDerefBug(NSErrorName));
-    bug = NSBT.get();
-  }
-  else {
-    if (!CFBT)
-      CFBT.reset(new CFErrorDerefBug(CFErrorName));
-    bug = CFBT.get();
-  }
+  const BugType &BT = isNSError ? NSError : CFError;
   BR.emitReport(
-      std::make_unique<PathSensitiveBugReport>(*bug, os.str(), event.SinkNode));
+      std::make_unique<PathSensitiveBugReport>(BT, os.str(), event.SinkNode));
 }
 
 static bool IsNSError(QualType T, IdentifierInfo *II) {
@@ -320,32 +295,21 @@ static bool IsCFError(QualType T, IdentifierInfo *II) {
   return TT->getDecl()->getIdentifier() == II;
 }
 
-void ento::registerNSOrCFErrorDerefChecker(CheckerManager &mgr) {
-  mgr.registerChecker<NSOrCFErrorDerefChecker>();
-}
-
-bool ento::shouldRegisterNSOrCFErrorDerefChecker(const CheckerManager &mgr) {
-  return true;
-}
-
-void ento::registerNSErrorChecker(CheckerManager &mgr) {
-  mgr.registerChecker<NSErrorMethodChecker>();
-  NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>();
-  checker->ShouldCheckNSError = true;
-  checker->NSErrorName = mgr.getCurrentCheckerName();
-}
-
-bool ento::shouldRegisterNSErrorChecker(const CheckerManager &mgr) {
-  return true;
-}
-
-void ento::registerCFErrorChecker(CheckerManager &mgr) {
-  mgr.registerChecker<CFErrorFunctionChecker>();
-  NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>();
-  checker->ShouldCheckCFError = true;
-  checker->CFErrorName = mgr.getCurrentCheckerName();
-}
+// This source file implements two user-facing checkers ("osx.cocoa.NSError"
+// and "osx.coreFoundation.CFError") which are both implemented as the
+// combination of two `CheckerFrontend`s that are registered under the same
+// name (but otherwise act independently). Among these 2+2 `CheckerFrontend`s
+// two are coming from the checker family `NSOrCFErrorDerefChecker` while the
+// other two (the `ADDITIONAL_PART`s) are small standalone checkers.
+#define REGISTER_CHECKER(NAME, ADDITIONAL_PART)                                \
+  void ento::register##NAME##Checker(CheckerManager &Mgr) {                    \
+    Mgr.getChecker<NSOrCFErrorDerefChecker>()->NAME.enable(Mgr);               \
+    Mgr.registerChecker<ADDITIONAL_PART>();                                    \
+  }                                                                            \
+                                                                               \
+  bool ento::shouldRegister##NAME##Checker(const CheckerManager &) {           \
+    return true;                                                               \
+  }
 
-bool ento::shouldRegisterCFErrorChecker(const CheckerManager &mgr) {
-  return true;
-}
+REGISTER_CHECKER(NSError, NSErrorMethodChecker)
+REGISTER_CHECKER(CFError, CFErrorFunctionChecker)