Skip to content

Conversation

@vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Oct 21, 2025

Replace two StringRefs with One StringRef + 2 x size_t.

Prepare for:

Created using spr 1.3.6
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2025

@llvm/pr-subscribers-llvm-support

Author: Vitaly Buka (vitalybuka)

Changes

Replace two StringRefs with One StringRef + 2 x size_t.

Prepare for #164512.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Support/GlobPattern.h (+12-6)
  • (modified) llvm/lib/Support/GlobPattern.cpp (+11-8)
diff --git a/llvm/include/llvm/Support/GlobPattern.h b/llvm/include/llvm/Support/GlobPattern.h
index c1b44849b9794..230898edf1881 100644
--- a/llvm/include/llvm/Support/GlobPattern.h
+++ b/llvm/include/llvm/Support/GlobPattern.h
@@ -63,21 +63,27 @@ class GlobPattern {
   // Returns true for glob pattern "*". Can be used to avoid expensive
   // preparation/acquisition of the input for match().
   bool isTrivialMatchAll() const {
-    if (!Prefix.empty())
+    if (PrefixSize)
       return false;
-    if (!Suffix.empty())
+    if (SuffixSize)
       return false;
     if (SubGlobs.size() != 1)
       return false;
     return SubGlobs[0].getPat() == "*";
   }
 
-  StringRef prefix() const { return Prefix; }
-  StringRef suffix() const { return Suffix; }
+  // The followind functions are just shortcuts for faster matching. They are
+  // conservative to simplify implementations.
+
+  // Returns plain prefix of the pattern.
+  StringRef prefix() const { return Pattern.take_front(PrefixSize); }
+  // Returns plain suffix of the pattern.
+  StringRef suffix() const { return Pattern.take_back(SuffixSize); }
 
 private:
-  StringRef Prefix;
-  StringRef Suffix;
+  StringRef Pattern;
+  size_t PrefixSize = 0;
+  size_t SuffixSize = 0;
 
   struct SubGlobPattern {
     /// \param Pat the pattern to match against
diff --git a/llvm/lib/Support/GlobPattern.cpp b/llvm/lib/Support/GlobPattern.cpp
index 0ecf47dc1d3d1..f56a8fcf4bf9d 100644
--- a/llvm/lib/Support/GlobPattern.cpp
+++ b/llvm/lib/Support/GlobPattern.cpp
@@ -135,21 +135,24 @@ parseBraceExpansions(StringRef S, std::optional<size_t> MaxSubPatterns) {
 Expected<GlobPattern>
 GlobPattern::create(StringRef S, std::optional<size_t> MaxSubPatterns) {
   GlobPattern Pat;
+  Pat.Pattern = S;
 
   // Store the prefix that does not contain any metacharacter.
-  size_t PrefixSize = S.find_first_of("?*[{\\");
-  Pat.Prefix = S.substr(0, PrefixSize);
-  if (PrefixSize == std::string::npos)
+  Pat.PrefixSize = S.find_first_of("?*[{\\");
+  if (Pat.PrefixSize == std::string::npos) {
+    Pat.PrefixSize = S.size();
     return Pat;
-  S = S.substr(PrefixSize);
+  }
+  S = S.substr(Pat.PrefixSize);
 
   // Just in case we stop on unmatched opening brackets.
   size_t SuffixStart = S.find_last_of("?*[]{}\\");
   assert(SuffixStart != std::string::npos);
   if (S[SuffixStart] == '\\')
     ++SuffixStart;
-  ++SuffixStart;
-  Pat.Suffix = S.substr(SuffixStart);
+  if (SuffixStart < S.size())
+    ++SuffixStart;
+  Pat.SuffixSize = S.size() - SuffixStart;
   S = S.substr(0, SuffixStart);
 
   SmallVector<std::string, 1> SubPats;
@@ -200,9 +203,9 @@ GlobPattern::SubGlobPattern::create(StringRef S) {
 }
 
 bool GlobPattern::match(StringRef S) const {
-  if (!S.consume_front(Prefix))
+  if (!S.consume_front(prefix()))
     return false;
-  if (!S.consume_back(Suffix))
+  if (!S.consume_back(suffix()))
     return false;
   if (SubGlobs.empty() && S.empty())
     return true;

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the internal structure of GlobPattern to replace two StringRef members (Prefix and Suffix) with a single StringRef (Pattern) and two size_t members (PrefixSize and SuffixSize). This change maintains the same functionality while storing the full pattern string once and using size values to compute prefix and suffix substrings on demand.

Key changes:

  • Internal data members restructured to store sizes instead of string references
  • Accessor methods prefix() and suffix() now compute substrings using take_front() and take_back()
  • Pattern construction logic updated to store sizes and the full pattern string

Reviewed Changes

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

File Description
llvm/lib/Support/GlobPattern.cpp Updates pattern construction to store the full pattern string and compute prefix/suffix sizes instead of storing substring references
llvm/include/llvm/Support/GlobPattern.h Changes data members from StringRef Prefix/Suffix to StringRef Pattern with size_t PrefixSize/SuffixSize, and updates accessor methods to compute substrings

Created using spr 1.3.6
@vitalybuka vitalybuka requested a review from qinkunbao October 21, 2025 23:54
Created using spr 1.3.7
@vitalybuka vitalybuka merged commit d01fe9d into main Oct 23, 2025
10 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcglobpattern-change-internal-structure-of-globpattern branch October 23, 2025 00:39
mikolaj-pirog pushed a commit to mikolaj-pirog/llvm-project that referenced this pull request Oct 23, 2025
)

Replace two StringRefs with One StringRef + 2 x size_t.

Prepare for:
* llvm#164512
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
)

Replace two StringRefs with One StringRef + 2 x size_t.

Prepare for:
* llvm#164512
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
)

Replace two StringRefs with One StringRef + 2 x size_t.

Prepare for:
* llvm#164512
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
)

Replace two StringRefs with One StringRef + 2 x size_t.

Prepare for:
* llvm#164512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants