[analyzer] Conversion to CheckerFamily: DynamicTypePropagation

[NagyDonat]

This commit converts the class DynamicTypePropagation to a very simple checker family, which has only one checker frontend -- but also supports enabling the backend ("modeling checker") without the frontend.

As a tangentially related change, this commit adds the backend of DynamicTypePropagation as a dependency of alpha.core.DynamicTypeChecker in Checkers.td, because the header comment of DynamicTypeChecker.cpp claims that it depends on DynamicTypePropagation and the source code seems to confirm this.

(The lack of this dependency relationship didn't cause problems, because 'core.DynamicTypePropagation' is in the group 'core', so it is practically always active. However, explicitly declaring the dependency clarifies the fact that the separate existence of the modeling checker is warranted.)

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Donát Nagy (NagyDonat)

Changes

This commit converts the class DynamicTypePropagation to a very simple checker family, which has only one checker frontend -- but also supports enabling the backend ("modeling checker") without the frontend.

As a tangentially related change, this commit adds the backend of DynamicTypePropagation as a dependency of alpha.core.DynamicTypeChecker, in Checkers.td, because the header comment of DynamicTypeChecker.cpp claims that it depends on DynamicTypePropagation and a cursory reading of the source code seems to confirm this.

(The lack of this dependency relationship didn't cause problems, because 'core.DynamicTypePropagation' is in the group 'core', so it is practically always active. However, explicitly declaring the dependency clarifies the fact that the separate existence of the modeling checker is warranted.)

---

Full diff: https://github.com/llvm/llvm-project/pull/144735.diff

2 Files Affected:

• (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+6-4)
• (modified) clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp (+24-27)

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 2a96df80d1001..789a18ff11043 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -294,10 +294,12 @@ def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
            "Either the comparison is useless or there is division by zero.">,
   Documentation<HasDocumentation>;
 
-def DynamicTypeChecker : Checker<"DynamicTypeChecker">,
-  HelpText<"Check for cases where the dynamic and the static type of an object "
-           "are unrelated.">,
-  Documentation<HasDocumentation>;
+def DynamicTypeChecker
+    : Checker<"DynamicTypeChecker">,
+      HelpText<"Check for cases where the dynamic and the static type of an "
+               "object are unrelated.">,
+      Dependencies<[DynamicTypePropagation]>,
+      Documentation<HasDocumentation>;
 
 def StackAddrAsyncEscapeChecker : Checker<"StackAddressAsyncEscape">,
   HelpText<"Check that addresses to stack memory do not escape the function">,
diff --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
index 344be0b176c54..87fa233a94413 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
@@ -49,15 +49,19 @@ REGISTER_MAP_WITH_PROGRAMSTATE(MostSpecializedTypeArgsMap, SymbolRef,
                                const ObjCObjectPointerType *)
 
 namespace {
-class DynamicTypePropagation:
-    public Checker< check::PreCall,
-                    check::PostCall,
-                    check::DeadSymbols,
-                    check::PostStmt<CastExpr>,
-                    check::PostStmt<CXXNewExpr>,
-                    check::PreObjCMessage,
-                    check::PostObjCMessage > {
+class DynamicTypePropagation
+    : public CheckerFamily<check::PreCall, check::PostCall, check::DeadSymbols,
+                           check::PostStmt<CastExpr>,
+                           check::PostStmt<CXXNewExpr>, check::PreObjCMessage,
+                           check::PostObjCMessage> {
+public:
+  // This checker family implements only one frontend, but -- unlike a simple
+  // Checker -- its backend can be enabled (by the checker DynamicTypeChecker
+  // which depends on it) without enabling the frontend.
+  CheckerFrontendWithBugType ObjCGenericsChecker{
+      "Generics", categories::CoreFoundationObjectiveC};
 
+private:
   /// Return a better dynamic type if one can be derived from the cast.
   const ObjCObjectPointerType *getBetterObjCType(const Expr *CastE,
                                                  CheckerContext &C) const;
@@ -66,13 +70,6 @@ class DynamicTypePropagation:
                                               ProgramStateRef &State,
                                               CheckerContext &C) const;
 
-  mutable std::unique_ptr<BugType> ObjCGenericsBugType;
-  void initBugType() const {
-    if (!ObjCGenericsBugType)
-      ObjCGenericsBugType.reset(new BugType(
-          GenericCheckName, "Generics", categories::CoreFoundationObjectiveC));
-  }
-
   class GenericsBugVisitor : public BugReporterVisitor {
   public:
     GenericsBugVisitor(SymbolRef S) : Sym(S) {}
@@ -106,9 +103,8 @@ class DynamicTypePropagation:
   void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
 
-  /// This value is set to true, when the Generics checker is turned on.
-  bool CheckGenerics = false;
-  CheckerNameRef GenericCheckName;
+  /// Identifies this checker family for debugging purposes.
+  StringRef getDebugTag() const override { return "DynamicTypePropagation"; }
 };
 
 bool isObjCClassType(QualType Type) {
@@ -1026,10 +1022,9 @@ void DynamicTypePropagation::reportGenericsBug(
     const ObjCObjectPointerType *From, const ObjCObjectPointerType *To,
     ExplodedNode *N, SymbolRef Sym, CheckerContext &C,
     const Stmt *ReportedNode) const {
-  if (!CheckGenerics)
+  if (!ObjCGenericsChecker.isEnabled())
     return;
 
-  initBugType();
   SmallString<192> Buf;
   llvm::raw_svector_ostream OS(Buf);
   OS << "Conversion from value of type '";
@@ -1037,7 +1032,7 @@ void DynamicTypePropagation::reportGenericsBug(
   OS << "' to incompatible type '";
   QualType::print(To, Qualifiers(), OS, C.getLangOpts(), llvm::Twine());
   OS << "'";
-  auto R = std::make_unique<PathSensitiveBugReport>(*ObjCGenericsBugType,
+  auto R = std::make_unique<PathSensitiveBugReport>(ObjCGenericsChecker,
                                                     OS.str(), N);
   R->markInteresting(Sym);
   R->addVisitor(std::make_unique<GenericsBugVisitor>(Sym));
@@ -1102,18 +1097,20 @@ PathDiagnosticPieceRef DynamicTypePropagation::GenericsBugVisitor::VisitNode(
 }
 
 /// Register checkers.
-void ento::registerObjCGenericsChecker(CheckerManager &mgr) {
-  DynamicTypePropagation *checker = mgr.getChecker<DynamicTypePropagation>();
-  checker->CheckGenerics = true;
-  checker->GenericCheckName = mgr.getCurrentCheckerName();
+void ento::registerObjCGenericsChecker(CheckerManager &Mgr) {
+  Mgr.getChecker<DynamicTypePropagation>()->ObjCGenericsChecker.enable(Mgr);
 }
 
 bool ento::shouldRegisterObjCGenericsChecker(const CheckerManager &mgr) {
   return true;
 }
 
-void ento::registerDynamicTypePropagation(CheckerManager &mgr) {
-  mgr.registerChecker<DynamicTypePropagation>();
+void ento::registerDynamicTypePropagation(CheckerManager &Mgr) {
+  // The checker 'core.DynamicTypeChecker' relies on the modeling implemented
+  // in the class 'DynamicTypePropagation', so this "modeling checker" can
+  // register the 'DynamicTypePropagation' backend for its callbacks without
+  // enabling its frontend.
+  Mgr.getChecker<DynamicTypePropagation>();
 }
 
 bool ento::shouldRegisterDynamicTypePropagation(const CheckerManager &mgr) {