[clang-tools-extra] [clang-tidy][NFC] Devirtualize `GlobList` and don't heap allocate it (PR #164212)

Victor Chernyakin via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 20 00:35:24 PDT 2025


https://github.com/localspook updated https://github.com/llvm/llvm-project/pull/164212

>From 89da548f5fcab8b7b6de33c47cac79c527aa6cfa Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Mon, 20 Oct 2025 00:27:59 -0700
Subject: [PATCH] [clang-tidy][NFC] Devirtualize `GlobList` and don't heap
 allocate it

---
 .../clang-tidy/ClangTidyDiagnosticConsumer.cpp   |  9 ++++-----
 .../clang-tidy/ClangTidyDiagnosticConsumer.h     |  6 +++---
 clang-tools-extra/clang-tidy/GlobList.h          |  8 ++++----
 .../clang-tidy/NoLintDirectiveHandler.cpp        | 16 +++++++---------
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 65fd09f99ef0f..001b330689b11 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -238,9 +238,8 @@ static bool parseFileExtensions(llvm::ArrayRef<std::string> AllFileExtensions,
 void ClangTidyContext::setCurrentFile(StringRef File) {
   CurrentFile = std::string(File);
   CurrentOptions = getOptionsForFile(CurrentFile);
-  CheckFilter = std::make_unique<CachedGlobList>(*getOptions().Checks);
-  WarningAsErrorFilter =
-      std::make_unique<CachedGlobList>(*getOptions().WarningsAsErrors);
+  CheckFilter = {*getOptions().Checks};
+  WarningAsErrorFilter = {*getOptions().WarningsAsErrors};
   if (!parseFileExtensions(*getOptions().HeaderFileExtensions,
                            HeaderFileExtensions))
     this->configurationDiag("Invalid header file extensions");
@@ -285,12 +284,12 @@ ClangTidyContext::getProfileStorageParams() const {
 
 bool ClangTidyContext::isCheckEnabled(StringRef CheckName) const {
   assert(CheckFilter != nullptr);
-  return CheckFilter->contains(CheckName);
+  return CheckFilter.contains(CheckName);
 }
 
 bool ClangTidyContext::treatAsError(StringRef CheckName) const {
   assert(WarningAsErrorFilter != nullptr);
-  return WarningAsErrorFilter->contains(CheckName);
+  return WarningAsErrorFilter.contains(CheckName);
 }
 
 std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 21ffd9de35c19..ddf2c378b9ff0 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -12,6 +12,7 @@
 #include "ClangTidyOptions.h"
 #include "ClangTidyProfiling.h"
 #include "FileExtensionsSet.h"
+#include "GlobList.h"
 #include "NoLintDirectiveHandler.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Tooling/Core/Diagnostic.h"
@@ -27,7 +28,6 @@ class ASTContext;
 class SourceManager;
 
 namespace tidy {
-class CachedGlobList;
 
 /// A detected error complete with information to display diagnostic and
 /// automatic fix.
@@ -247,8 +247,8 @@ class ClangTidyContext {
   std::string CurrentFile;
   ClangTidyOptions CurrentOptions;
 
-  std::unique_ptr<CachedGlobList> CheckFilter;
-  std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
+  CachedGlobList CheckFilter;
+  CachedGlobList WarningAsErrorFilter;
 
   FileExtensionsSet HeaderFileExtensions;
   FileExtensionsSet ImplementationFileExtensions;
diff --git a/clang-tools-extra/clang-tidy/GlobList.h b/clang-tools-extra/clang-tidy/GlobList.h
index c9086df2b7973..fb6f387099b9e 100644
--- a/clang-tools-extra/clang-tidy/GlobList.h
+++ b/clang-tools-extra/clang-tidy/GlobList.h
@@ -24,7 +24,7 @@ namespace clang::tidy {
 /// them in the order of appearance in the list.
 class GlobList {
 public:
-  virtual ~GlobList() = default;
+  GlobList() = default;
 
   /// \p Globs is a comma-separated list of globs (only the '*' metacharacter is
   /// supported) with an optional '-' prefix to denote exclusion.
@@ -38,7 +38,7 @@ class GlobList {
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  virtual bool contains(StringRef S) const;
+  bool contains(StringRef S) const;
 
 private:
   struct GlobListItem {
@@ -54,12 +54,12 @@ class GlobList {
 
 /// A \p GlobList that caches search results, so that search is performed only
 /// once for the same query.
-class CachedGlobList final : public GlobList {
+class CachedGlobList : public GlobList {
 public:
   using GlobList::GlobList;
 
   /// \see GlobList::contains
-  bool contains(StringRef S) const override;
+  bool contains(StringRef S) const;
 
 private:
   mutable llvm::StringMap<bool> Cache;
diff --git a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp
index ef20ee18347df..bcaf38c6aa4dd 100644
--- a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp
+++ b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp
@@ -79,9 +79,8 @@ class NoLintToken {
   // - Negative globs ignored (which would effectively disable the suppression).
   NoLintToken(NoLintType Type, size_t Pos,
               const std::optional<StringRef> &Checks)
-      : Type(Type), Pos(Pos), ChecksGlob(std::make_unique<CachedGlobList>(
-                                  Checks.value_or("*"),
-                                  /*KeepNegativeGlobs=*/false)) {
+      : Type(Type), Pos(Pos), ChecksGlob(Checks.value_or("*"),
+                                         /*KeepNegativeGlobs=*/false) {
     if (Checks)
       this->Checks = trimWhitespace(*Checks);
   }
@@ -93,13 +92,13 @@ class NoLintToken {
   size_t Pos;
 
   // A glob of the checks this NOLINT token disables.
-  std::unique_ptr<CachedGlobList> ChecksGlob;
+  CachedGlobList ChecksGlob;
 
   // If this NOLINT specifies checks, return the checks.
   const std::optional<std::string> &checks() const { return Checks; }
 
   // Whether this NOLINT applies to the provided check.
-  bool suppresses(StringRef Check) const { return ChecksGlob->contains(Check); }
+  bool suppresses(StringRef Check) const { return ChecksGlob.contains(Check); }
 
 private:
   std::optional<std::string> Checks;
@@ -156,21 +155,20 @@ namespace {
 // Represents a source range within a pair of NOLINT(BEGIN/END) comments.
 class NoLintBlockToken {
 public:
-  NoLintBlockToken(size_t BeginPos, size_t EndPos,
-                   std::unique_ptr<CachedGlobList> ChecksGlob)
+  NoLintBlockToken(size_t BeginPos, size_t EndPos, CachedGlobList ChecksGlob)
       : BeginPos(BeginPos), EndPos(EndPos), ChecksGlob(std::move(ChecksGlob)) {}
 
   // Whether the provided diagnostic is within and is suppressible by this block
   // of NOLINT(BEGIN/END) comments.
   bool suppresses(size_t DiagPos, StringRef DiagName) const {
     return (BeginPos < DiagPos) && (DiagPos < EndPos) &&
-           ChecksGlob->contains(DiagName);
+           ChecksGlob.contains(DiagName);
   }
 
 private:
   size_t BeginPos;
   size_t EndPos;
-  std::unique_ptr<CachedGlobList> ChecksGlob;
+  CachedGlobList ChecksGlob;
 };
 
 } // namespace



More information about the cfe-commits mailing list