[NFC][SpecialCaseList] Move most of implementation in cpp file #167280

vitalybuka · 2025-11-10T07:19:59Z

This commit moves the RegexMatcher, GlobMatcher, Matcher and
Section classes into an anonymous namespace within
SpecialCaseList.cpp. These classes are implementation details of
SpecialCaseList and do not need to be exposed in the header.

Created using spr 1.3.7

llvmbot · 2025-11-10T07:20:31Z

@llvm/pr-subscribers-llvm-support

Author: Vitaly Buka (vitalybuka)

Changes

This commit moves the RegexMatcher, GlobMatcher, Matcher and
Section classes into an anonymous namespace within
SpecialCaseList.cpp. These classes are implementation details of
SpecialCaseList and do not need to be exposed in the header.

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

2 Files Affected:

(modified) llvm/include/llvm/Support/SpecialCaseList.h (+7-90)
(modified) llvm/lib/Support/SpecialCaseList.cpp (+136-34)

diff --git a/llvm/include/llvm/Support/SpecialCaseList.h b/llvm/include/llvm/Support/SpecialCaseList.h
index 8c4dc94ae54ce..5785dad7406df 100644
--- a/llvm/include/llvm/Support/SpecialCaseList.h
+++ b/llvm/include/llvm/Support/SpecialCaseList.h
@@ -12,19 +12,11 @@
 #ifndef LLVM_SUPPORT_SPECIALCASELIST_H
 #define LLVM_SUPPORT_SPECIALCASELIST_H
 
-#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/RadixTree.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
-#include "llvm/Support/Compiler.h"
-#include "llvm/Support/GlobPattern.h"
-#include "llvm/Support/Regex.h"
+#include "llvm/Support/Error.h"
 #include <memory>
 #include <string>
 #include <utility>
-#include <variant>
 #include <vector>
 
 namespace llvm {
@@ -125,83 +117,11 @@ class SpecialCaseList {
   SpecialCaseList(SpecialCaseList const &) = delete;
   SpecialCaseList &operator=(SpecialCaseList const &) = delete;
 
-private:
-  using Match = std::pair<StringRef, unsigned>;
-  static constexpr Match NotMatched = {"", 0};
-
-  // Lagacy v1 matcher.
-  class RegexMatcher {
-  public:
-    LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber);
-    LLVM_ABI void preprocess(bool BySize);
-
-    LLVM_ABI Match match(StringRef Query) const;
-
-    struct Reg {
-      Reg(StringRef Name, unsigned LineNo, Regex &&Rg)
-          : Name(Name), LineNo(LineNo), Rg(std::move(Rg)) {}
-      StringRef Name;
-      unsigned LineNo;
-      Regex Rg;
-    };
-
-    std::vector<Reg> RegExes;
-  };
-
-  class GlobMatcher {
-  public:
-    LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber);
-    LLVM_ABI void preprocess(bool BySize);
-
-    LLVM_ABI Match match(StringRef Query) const;
-
-    struct Glob {
-      Glob(StringRef Name, unsigned LineNo, GlobPattern &&Pattern)
-          : Name(Name), LineNo(LineNo), Pattern(std::move(Pattern)) {}
-      StringRef Name;
-      unsigned LineNo;
-      GlobPattern Pattern;
-    };
-
-    std::vector<GlobMatcher::Glob> Globs;
-
-    RadixTree<iterator_range<StringRef::const_iterator>,
-              RadixTree<iterator_range<StringRef::const_reverse_iterator>,
-                        SmallVector<int, 1>>>
-        PrefixSuffixToGlob;
-
-    RadixTree<iterator_range<StringRef::const_iterator>, SmallVector<int, 1>>
-        SubstrToGlob;
-  };
-
-  /// Represents a set of patterns and their line numbers
-  class Matcher {
-  public:
-    LLVM_ABI Matcher(bool UseGlobs, bool RemoveDotSlash);
-
-    LLVM_ABI Error insert(StringRef Pattern, unsigned LineNumber);
-    LLVM_ABI void preprocess(bool BySize);
-
-    LLVM_ABI Match match(StringRef Query) const;
-
-    LLVM_ABI bool matchAny(StringRef Query) const {
-      return match(Query) != NotMatched;
-    }
-
-    std::variant<RegexMatcher, GlobMatcher> M;
-    bool RemoveDotSlash;
-  };
-
-  using SectionEntries = StringMap<StringMap<Matcher>>;
-
-protected:
   class Section {
   public:
-    Section(StringRef Name, unsigned FileIdx, bool UseGlobs)
-        : SectionMatcher(UseGlobs, /*RemoveDotSlash=*/false), Name(Name),
-          FileIdx(FileIdx) {}
-
-    Section(Section &&) = default;
+    LLVM_ABI Section(StringRef Name, unsigned FileIdx, bool UseGlobs);
+    LLVM_ABI Section(Section &&);
+    LLVM_ABI ~Section();
 
     // Return name of the section, it's entire string in [].
     StringRef name() const { return Name; }
@@ -227,17 +147,14 @@ class SpecialCaseList {
 
   private:
     friend class SpecialCaseList;
-    LLVM_ABI void preprocess(bool OrderBySize);
-    LLVM_ABI const SpecialCaseList::Matcher *
-    findMatcher(StringRef Prefix, StringRef Category) const;
+    class SectionImpl;
 
-    Matcher SectionMatcher;
     StringRef Name;
-    SectionEntries Entries;
     unsigned FileIdx;
+    std::unique_ptr<SectionImpl> Impl;
   };
 
-  ArrayRef<const Section> sections() const { return Sections; }
+  const std::vector<Section> &sections() const;
 
 private:
   BumpPtrAllocator StrAlloc;
diff --git a/llvm/lib/Support/SpecialCaseList.cpp b/llvm/lib/Support/SpecialCaseList.cpp
index 42c8933a43399..cf01f59fa37f6 100644
--- a/llvm/lib/Support/SpecialCaseList.cpp
+++ b/llvm/lib/Support/SpecialCaseList.cpp
@@ -14,26 +14,94 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/SpecialCaseList.h"
+#include "llvm/ADT/RadixTree.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SetVector.h"
-#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/GlobPattern.h"
 #include "llvm/Support/LineIterator.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
-#include <algorithm>
-#include <limits>
 #include <memory>
 #include <stdio.h>
 #include <string>
 #include <system_error>
 #include <utility>
+#include <variant>
+#include <vector>
 
 namespace llvm {
 
-Error SpecialCaseList::RegexMatcher::insert(StringRef Pattern,
-                                            unsigned LineNumber) {
+namespace {
+
+using Match = std::pair<StringRef, unsigned>;
+static constexpr Match NotMatched = {"", 0};
+
+// Lagacy v1 matcher.
+class RegexMatcher {
+public:
+  Error insert(StringRef Pattern, unsigned LineNumber);
+  void preprocess(bool BySize);
+
+  Match match(StringRef Query) const;
+
+  struct Reg {
+    Reg(StringRef Name, unsigned LineNo, Regex &&Rg)
+        : Name(Name), LineNo(LineNo), Rg(std::move(Rg)) {}
+    StringRef Name;
+    unsigned LineNo;
+    Regex Rg;
+  };
+
+  std::vector<Reg> RegExes;
+};
+
+class GlobMatcher {
+public:
+  Error insert(StringRef Pattern, unsigned LineNumber);
+  void preprocess(bool BySize);
+
+  Match match(StringRef Query) const;
+
+  struct Glob {
+    Glob(StringRef Name, unsigned LineNo, GlobPattern &&Pattern)
+        : Name(Name), LineNo(LineNo), Pattern(std::move(Pattern)) {}
+    StringRef Name;
+    unsigned LineNo;
+    GlobPattern Pattern;
+  };
+
+  std::vector<GlobMatcher::Glob> Globs;
+
+  RadixTree<iterator_range<StringRef::const_iterator>,
+            RadixTree<iterator_range<StringRef::const_reverse_iterator>,
+                      SmallVector<int, 1>>>
+      PrefixSuffixToGlob;
+
+  RadixTree<iterator_range<StringRef::const_iterator>, SmallVector<int, 1>>
+      SubstrToGlob;
+};
+
+/// Represents a set of patterns and their line numbers
+class Matcher {
+public:
+  Matcher(bool UseGlobs, bool RemoveDotSlash);
+
+  Error insert(StringRef Pattern, unsigned LineNumber);
+  void preprocess(bool BySize);
+  Match match(StringRef Query) const;
+
+  bool matchAny(StringRef Query) const { return match(Query).second > 0; }
+
+  std::variant<RegexMatcher, GlobMatcher> M;
+  bool RemoveDotSlash;
+};
+
+Error RegexMatcher::insert(StringRef Pattern, unsigned LineNumber) {
   if (Pattern.empty())
     return createStringError(errc::invalid_argument,
                              "Supplied regex was blank");
@@ -57,7 +125,7 @@ Error SpecialCaseList::RegexMatcher::insert(StringRef Pattern,
   return Error::success();
 }
 
-void SpecialCaseList::RegexMatcher::preprocess(bool BySize) {
+void RegexMatcher::preprocess(bool BySize) {
   if (BySize) {
     llvm::stable_sort(RegExes, [](const Reg &A, const Reg &B) {
       return A.Name.size() < B.Name.size();
@@ -65,16 +133,14 @@ void SpecialCaseList::RegexMatcher::preprocess(bool BySize) {
   }
 }
 
-SpecialCaseList::Match
-SpecialCaseList::RegexMatcher::match(StringRef Query) const {
+Match RegexMatcher::match(StringRef Query) const {
   for (const auto &R : reverse(RegExes))
     if (R.Rg.match(Query))
       return {R.Name, R.LineNo};
   return NotMatched;
 }
 
-Error SpecialCaseList::GlobMatcher::insert(StringRef Pattern,
-                                           unsigned LineNumber) {
+Error GlobMatcher::insert(StringRef Pattern, unsigned LineNumber) {
   if (Pattern.empty())
     return createStringError(errc::invalid_argument, "Supplied glob was blank");
 
@@ -85,7 +151,7 @@ Error SpecialCaseList::GlobMatcher::insert(StringRef Pattern,
   return Error::success();
 }
 
-void SpecialCaseList::GlobMatcher::preprocess(bool BySize) {
+void GlobMatcher::preprocess(bool BySize) {
   if (BySize) {
     llvm::stable_sort(Globs, [](const Glob &A, const Glob &B) {
       return A.Name.size() < B.Name.size();
@@ -115,8 +181,7 @@ void SpecialCaseList::GlobMatcher::preprocess(bool BySize) {
   }
 }
 
-SpecialCaseList::Match
-SpecialCaseList::GlobMatcher::match(StringRef Query) const {
+Match GlobMatcher::match(StringRef Query) const {
   int Best = -1;
   if (!PrefixSuffixToGlob.empty()) {
     for (const auto &[_, SToGlob] : PrefixSuffixToGlob.find_prefixes(Query)) {
@@ -164,7 +229,7 @@ SpecialCaseList::GlobMatcher::match(StringRef Query) const {
   return {Globs[Best].Name, Globs[Best].LineNo};
 }
 
-SpecialCaseList::Matcher::Matcher(bool UseGlobs, bool RemoveDotSlash)
+Matcher::Matcher(bool UseGlobs, bool RemoveDotSlash)
     : RemoveDotSlash(RemoveDotSlash) {
   if (UseGlobs)
     M.emplace<GlobMatcher>();
@@ -172,20 +237,45 @@ SpecialCaseList::Matcher::Matcher(bool UseGlobs, bool RemoveDotSlash)
     M.emplace<RegexMatcher>();
 }
 
-Error SpecialCaseList::Matcher::insert(StringRef Pattern, unsigned LineNumber) {
+Error Matcher::insert(StringRef Pattern, unsigned LineNumber) {
   return std::visit([&](auto &V) { return V.insert(Pattern, LineNumber); }, M);
 }
 
-void SpecialCaseList::Matcher::preprocess(bool BySize) {
+void Matcher::preprocess(bool BySize) {
   return std::visit([&](auto &V) { return V.preprocess(BySize); }, M);
 }
 
-SpecialCaseList::Match SpecialCaseList::Matcher::match(StringRef Query) const {
+Match Matcher::match(StringRef Query) const {
   if (RemoveDotSlash)
     Query = llvm::sys::path::remove_leading_dotslash(Query);
-  return std::visit(
-      [&](auto &V) -> SpecialCaseList::Match { return V.match(Query); }, M);
+  return std::visit([&](auto &V) -> Match { return V.match(Query); }, M);
 }
+} // namespace
+
+class SpecialCaseList::Section::SectionImpl {
+  friend class SpecialCaseList;
+  void preprocess(bool OrderBySize);
+  const Matcher *findMatcher(StringRef Prefix, StringRef Category) const;
+
+public:
+  using SectionEntries = StringMap<StringMap<Matcher>>;
+
+  SectionImpl(StringRef Str, bool UseGlobs)
+      : SectionMatcher(UseGlobs, /*RemoveDotSlash=*/false) {}
+
+  Matcher SectionMatcher;
+  SectionEntries Entries;
+
+  // Helper method to search by Prefix, Query, and Category. Returns
+  // 1-based line number on which rule is defined, or 0 if there is no match.
+  unsigned getLastMatch(StringRef Prefix, StringRef Query,
+                        StringRef Category) const;
+
+  // Helper method to search by Prefix, Query, and Category. Returns
+  // matching rule, or empty string if there is no match.
+  StringRef getLongestMatch(StringRef Prefix, StringRef Query,
+                            StringRef Category) const;
+};
 
 // TODO: Refactor this to return Expected<...>
 std::unique_ptr<SpecialCaseList>
@@ -240,6 +330,10 @@ bool SpecialCaseList::createInternal(const MemoryBuffer *MB, std::string &Error,
   return true;
 }
 
+const std::vector<SpecialCaseList::Section> &SpecialCaseList::sections() const {
+  return Sections;
+}
+
 Expected<SpecialCaseList::Section *>
 SpecialCaseList::addSection(StringRef SectionStr, unsigned FileNo,
                             unsigned LineNo, bool UseGlobs) {
@@ -247,7 +341,7 @@ SpecialCaseList::addSection(StringRef SectionStr, unsigned FileNo,
   Sections.emplace_back(SectionStr, FileNo, UseGlobs);
   auto &Section = Sections.back();
 
-  if (auto Err = Section.SectionMatcher.insert(SectionStr, LineNo)) {
+  if (auto Err = Section.Impl->SectionMatcher.insert(SectionStr, LineNo)) {
     return createStringError(errc::invalid_argument,
                              "malformed section at line " + Twine(LineNo) +
                                  ": '" + SectionStr +
@@ -279,7 +373,7 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
     Error = toString(std::move(Err));
     return false;
   }
-  Section *CurrentSection = ErrOrSection.get();
+  Section::SectionImpl *CurrentImpl = ErrOrSection.get()->Impl.get();
 
   // This is the current list of prefixes for all existing users matching file
   // path. We may need parametrization in constructor in future.
@@ -307,7 +401,7 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
         Error = toString(std::move(Err));
         return false;
       }
-      CurrentSection = ErrOrSection.get();
+      CurrentImpl = ErrOrSection.get()->Impl.get();
       continue;
     }
 
@@ -320,7 +414,7 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
     }
 
     auto [Pattern, Category] = Postfix.split("=");
-    auto [It, _] = CurrentSection->Entries[Prefix].try_emplace(
+    auto [It, _] = CurrentImpl->Entries[Prefix].try_emplace(
         Category, UseGlobs,
         RemoveDotSlash && llvm::is_contained(PathPrefixes, Prefix));
     Pattern = Pattern.copy(StrAlloc);
@@ -334,7 +428,7 @@ bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
   }
 
   for (Section &S : Sections)
-    S.preprocess(OrderBySize);
+    S.Impl->preprocess(OrderBySize);
 
   return true;
 }
@@ -351,7 +445,7 @@ std::pair<unsigned, unsigned>
 SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
                                 StringRef Query, StringRef Category) const {
   for (const auto &S : reverse(Sections)) {
-    if (S.SectionMatcher.matchAny(Section)) {
+    if (S.Impl->SectionMatcher.matchAny(Section)) {
       unsigned Blame = S.getLastMatch(Prefix, Query, Category);
       if (Blame)
         return {S.FileIdx, Blame};
@@ -360,13 +454,21 @@ SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
   return NotFound;
 }
 
+SpecialCaseList::Section::Section(StringRef Str, unsigned FileIdx,
+                                  bool UseGlobs)
+    : Name(Str), FileIdx(FileIdx),
+      Impl(std::make_unique<SectionImpl>(Str, UseGlobs)) {}
+
+SpecialCaseList::Section::Section(Section &&) = default;
+SpecialCaseList::Section::~Section() = default;
+
 bool SpecialCaseList::Section::matchName(StringRef Name) const {
-  return SectionMatcher.matchAny(Name);
+  return Impl->SectionMatcher.matchAny(Name);
 }
 
-const SpecialCaseList::Matcher *
-SpecialCaseList::Section::findMatcher(StringRef Prefix,
-                                      StringRef Category) const {
+const Matcher *
+SpecialCaseList::Section::SectionImpl::findMatcher(StringRef Prefix,
+                                                   StringRef Category) const {
   SectionEntries::const_iterator I = Entries.find(Prefix);
   if (I == Entries.end())
     return nullptr;
@@ -377,7 +479,7 @@ SpecialCaseList::Section::findMatcher(StringRef Prefix,
   return &II->second;
 }
 
-LLVM_ABI void SpecialCaseList::Section::preprocess(bool OrderBySize) {
+void SpecialCaseList::Section::SectionImpl::preprocess(bool OrderBySize) {
   SectionMatcher.preprocess(false);
   for (auto &[K1, E] : Entries)
     for (auto &[K2, M] : E)
@@ -387,7 +489,7 @@ LLVM_ABI void SpecialCaseList::Section::preprocess(bool OrderBySize) {
 unsigned SpecialCaseList::Section::getLastMatch(StringRef Prefix,
                                                 StringRef Query,
                                                 StringRef Category) const {
-  if (const Matcher *M = findMatcher(Prefix, Category))
+  if (const Matcher *M = Impl->findMatcher(Prefix, Category))
     return M->match(Query).second;
   return 0;
 }
@@ -395,13 +497,13 @@ unsigned SpecialCaseList::Section::getLastMatch(StringRef Prefix,
 StringRef SpecialCaseList::Section::getLongestMatch(StringRef Prefix,
                                                     StringRef Query,
                                                     StringRef Category) const {
-  if (const Matcher *M = findMatcher(Prefix, Category))
+  if (const Matcher *M = Impl->findMatcher(Prefix, Category))
     return M->match(Query).first;
   return {};
 }
 
 bool SpecialCaseList::Section::hasPrefix(StringRef Prefix) const {
-  return Entries.find(Prefix) != Entries.end();
+  return Impl->Entries.find(Prefix) != Impl->Entries.end();
 }
 
 } // namespace llvm

This commit moves the `RegexMatcher`, `GlobMatcher`, `Matcher` and `Section` classes into an anonymous namespace within `SpecialCaseList.cpp`. These classes are implementation details of `SpecialCaseList` and do not need to be exposed in the header. Pull Request: llvm#167280

Copilot

Pull Request Overview

This PR refactors the SpecialCaseList class to use the Pimpl idiom, moving implementation details from the public header to the implementation file. This reduces header dependencies and improves encapsulation.

Key changes:

Moved RegexMatcher, GlobMatcher, and Matcher classes from header to anonymous namespace in the .cpp file
Introduced Section::SectionImpl as a forward-declared implementation class
Updated the sections() method signature to return a const reference instead of ArrayRef

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
llvm/lib/Support/SpecialCaseList.cpp	Moved matcher classes to anonymous namespace; added `SectionImpl` nested class with implementation details
llvm/include/llvm/Support/SpecialCaseList.h	Removed matcher class definitions from header; added forward declaration of `SectionImpl`; updated includes and method signatures

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-10T19:01:10Z

llvm/lib/Support/SpecialCaseList.cpp

+using Match = std::pair<StringRef, unsigned>;
+static constexpr Match NotMatched = {"", 0};
+
+// Lagacy v1 matcher.


Corrected spelling of 'Lagacy' to 'Legacy'.

Suggested change

// Lagacy v1 matcher.

// Legacy v1 matcher.

…7276) Preparing to moving most of implementation out of the header file. * #167280 --------- Co-authored-by: Naveen Seth Hanig <[email protected]> Co-authored-by: Copilot <[email protected]>

…ection (#167276) Preparing to moving most of implementation out of the header file. * llvm/llvm-project#167280 --------- Co-authored-by: Naveen Seth Hanig <[email protected]> Co-authored-by: Copilot <[email protected]>

[𝘀𝗽𝗿] initial version

d54faf9

Created using spr 1.3.7

llvmbot added the llvm:support label Nov 10, 2025

vitalybuka requested review from Copilot, fmayer, qinkunbao and thurstond November 10, 2025 18:58

Copilot AI reviewed Nov 10, 2025

View reviewed changes

thurstond approved these changes Nov 10, 2025

View reviewed changes

vitalybuka mentioned this pull request Nov 10, 2025

[NFC][SpecialCaseList] Hide Section internals in private section #167276

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC][SpecialCaseList] Move most of implementation in cpp file #167280

[NFC][SpecialCaseList] Move most of implementation in cpp file #167280

vitalybuka commented Nov 10, 2025

Uh oh!

llvmbot commented Nov 10, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[NFC][SpecialCaseList] Move most of implementation in cpp file #167280

Are you sure you want to change the base?

[NFC][SpecialCaseList] Move most of implementation in cpp file #167280

Conversation

vitalybuka commented Nov 10, 2025

Uh oh!

llvmbot commented Nov 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants