[clang] [NFC][analyzer] Multipart checker refactor 1: VirtualCallChecker (PR #132072)

[Donát Nagy via cfe-commits]

https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/132072

>From f4e583ff179faf03b571cbd0091af404e5c2d5c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 18 Mar 2025 13:44:19 +0100
Subject: [PATCH 1/2] [NFC][analyzer] Multipart checker refactor 1:
 VirtualCallChecker

Simplify `VirtualCallChecker.cpp` with the help of the new framework for
multipart checkers that was introduced by commit
27099982da2f5a6c2d282d6b385e79d080669546. This is part of a commit
series that will perform analogous changes in all checker classes that
implement multiple user-facing checker parts (with separate names).

In this commit I'm removing the undocumented hidden
`cplusplus.VirtualCallModeling` checker, because (to my best
understanding) it was just a hacky implementation detail within the old
way of registering the "real" checker parts.

Note that keeping or re-adding an extra checker part like this modeling
checker would be very easy within the new framework; I'm removing it
only because I'm convinced that it is no longer useful.
---
 .../clang/StaticAnalyzer/Checkers/Checkers.td | 15 +++-----
 .../Checkers/VirtualCallChecker.cpp           | 38 ++++++++-----------
 2 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 35df4e7003ac9..648dd9e28b83e 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -694,15 +694,11 @@ def MoveChecker: Checker<"Move">,
   ]>,
   Documentation<HasDocumentation>;
 
-def VirtualCallModeling : Checker<"VirtualCallModeling">,
-  HelpText<"Auxiliary modeling for the virtual method call checkers">,
-  Documentation<NotDocumented>,
-  Hidden;
-
-def PureVirtualCallChecker : Checker<"PureVirtualCall">,
-  HelpText<"Check pure virtual function calls during construction/destruction">,
-  Dependencies<[VirtualCallModeling]>,
-  Documentation<HasDocumentation>;
+def PureVirtualCallChecker
+    : Checker<"PureVirtualCall">,
+      HelpText<
+          "Check pure virtual function calls during construction/destruction">,
+      Documentation<HasDocumentation>;
 } // end: "cplusplus"
 
 let ParentPackage = CplusplusOptIn in {
@@ -756,7 +752,6 @@ def VirtualCallChecker
       CheckerOptions<[CmdLineOption<Boolean, "ShowFixIts",
                                     "Enable fix-it hints for this checker",
                                     "false", InAlpha>]>,
-      Dependencies<[VirtualCallModeling]>,
       Documentation<HasDocumentation>;
 
 } // end: "optin.cplusplus"
diff --git a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
index 6d9c52e966022..26ca23d3924fc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -42,8 +42,14 @@ namespace {
 class VirtualCallChecker
     : public Checker<check::BeginFunction, check::EndFunction, check::PreCall> {
 public:
-  // These are going to be null if the respective check is disabled.
-  mutable std::unique_ptr<BugType> BT_Pure, BT_Impure;
+  enum : CheckerPartIdx { PureChecker, ImpureChecker, NumCheckerParts };
+
+  BugType BugTypes[NumCheckerParts] = {
+      {this, PureChecker, "Pure virtual method call",
+       categories::CXXObjectLifecycle},
+      {this, ImpureChecker, "Unexpected loss of virtual dispatch",
+       categories::CXXObjectLifecycle}};
+
   bool ShowFixIts = false;
 
   void checkBeginFunction(CheckerContext &C) const;
@@ -141,13 +147,15 @@ void VirtualCallChecker::checkPreCall(const CallEvent &Call,
   if (!N)
     return;
 
-  const std::unique_ptr<BugType> &BT = IsPure ? BT_Pure : BT_Impure;
-  if (!BT) {
+  const CheckerPartIdx Part = IsPure ? PureChecker : ImpureChecker;
+
+  if (!isPartEnabled(Part)) {
     // The respective check is disabled.
     return;
   }
 
-  auto Report = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N);
+  auto Report =
+      std::make_unique<PathSensitiveBugReport>(BugTypes[Part], OS.str(), N);
 
   if (ShowFixIts && !IsPure) {
     // FIXME: These hints are valid only when the virtual call is made
@@ -201,31 +209,17 @@ void VirtualCallChecker::registerCtorDtorCallInState(bool IsBeginFunction,
   }
 }
 
-void ento::registerVirtualCallModeling(CheckerManager &Mgr) {
-  Mgr.registerChecker<VirtualCallChecker>();
-}
-
 void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) {
-  auto *Chk = Mgr.getChecker<VirtualCallChecker>();
-  Chk->BT_Pure = std::make_unique<BugType>(Mgr.getCurrentCheckerName(),
-                                           "Pure virtual method call",
-                                           categories::CXXObjectLifecycle);
+  Mgr.registerChecker<VirtualCallChecker, VirtualCallChecker::PureChecker>();
 }
 
 void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
-  auto *Chk = Mgr.getChecker<VirtualCallChecker>();
-  Chk->BT_Impure = std::make_unique<BugType>(
-      Mgr.getCurrentCheckerName(), "Unexpected loss of virtual dispatch",
-      categories::CXXObjectLifecycle);
+  auto *Chk = Mgr.registerChecker<VirtualCallChecker,
+                                  VirtualCallChecker::ImpureChecker>();
   Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
       Mgr.getCurrentCheckerName(), "ShowFixIts");
 }
 
-bool ento::shouldRegisterVirtualCallModeling(const CheckerManager &mgr) {
-  const LangOptions &LO = mgr.getLangOpts();
-  return LO.CPlusPlus;
-}
-
 bool ento::shouldRegisterPureVirtualCallChecker(const CheckerManager &mgr) {
   const LangOptions &LO = mgr.getLangOpts();
   return LO.CPlusPlus;

>From 853ae2b52cd309e2b107a65dbe8a6ece89db1423 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 19 Mar 2025 19:07:42 +0100
Subject: [PATCH 2/2] Drive-by cleanup of the `shouldRegister` methods

---
 .../StaticAnalyzer/Checkers/VirtualCallChecker.cpp | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
index 26ca23d3924fc..5b0d303ee5bbc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -213,6 +213,10 @@ void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<VirtualCallChecker, VirtualCallChecker::PureChecker>();
 }
 
+bool ento::shouldRegisterPureVirtualCallChecker(const CheckerManager &Mgr) {
+  return Mgr.getLangOpts().CPlusPlus;
+}
+
 void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.registerChecker<VirtualCallChecker,
                                   VirtualCallChecker::ImpureChecker>();
@@ -220,12 +224,6 @@ void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
       Mgr.getCurrentCheckerName(), "ShowFixIts");
 }
 
-bool ento::shouldRegisterPureVirtualCallChecker(const CheckerManager &mgr) {
-  const LangOptions &LO = mgr.getLangOpts();
-  return LO.CPlusPlus;
-}
-
-bool ento::shouldRegisterVirtualCallChecker(const CheckerManager &mgr) {
-  const LangOptions &LO = mgr.getLangOpts();
-  return LO.CPlusPlus;
+bool ento::shouldRegisterVirtualCallChecker(const CheckerManager &Mgr) {
+  return Mgr.getLangOpts().CPlusPlus;
 }