Skip to content

Conversation

@Michael137
Copy link
Member

@Michael137 Michael137 commented Dec 18, 2025

This patch adds support for consuming a char from the front of a StringRef. Most of the time a user wanting to consume a single character off the front can just wrap the character in a string literal (i.e., consume_front("a")). But this doesn't work if we don't have a char literal, but instead a variable of type char. I.e., char c = 'a'; str.consume_front(c). There's at least one helper in LLDB that does this. Also there's plenty of example of consume_front being passed a single character via string literal. This patch adds the char overload. We already have a starts_with(char) overload, so there's at least some related precedent.

This patch adds support for consuming a `char` from the front of a `StringRef`. Most of the time a user wanting to consume a single character off the front can just wrap the character in a string literal (i.e., `consume_front("a")`). But this doesn't work if we don't have a `char` literal, but instead a variable of type `char`. I.e., `char c = 'a'; str.consume_front(c)`. This patch adds the `char` overload. We already have a `starts_with(char)` overload, so there's at least some related precedent.
@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2025

@llvm/pr-subscribers-llvm-adt

Author: Michael Buch (Michael137)

Changes

This patch adds support for consuming a char from the front of a StringRef. Most of the time a user wanting to consume a single character off the front can just wrap the character in a string literal (i.e., consume_front("a")). But this doesn't work if we don't have a char literal, but instead a variable of type char. I.e., char c = 'a'; str.consume_front(c). There's at least one helper in LLDB that uses a helper to do this. Also there's plenty of example of consume_front being passed a single character via string literal. This patch adds the char overload. We already have a starts_with(char) overload, so there's at least some related precedent.


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

2 Files Affected:

  • (modified) llvm/include/llvm/ADT/StringRef.h (+11)
  • (modified) llvm/unittests/ADT/StringRefTest.cpp (+20)
diff --git a/llvm/include/llvm/ADT/StringRef.h b/llvm/include/llvm/ADT/StringRef.h
index 7aee2aa67ddec..2f85bb1aefdb0 100644
--- a/llvm/include/llvm/ADT/StringRef.h
+++ b/llvm/include/llvm/ADT/StringRef.h
@@ -632,6 +632,17 @@ namespace llvm {
       return substr(find_if(F));
     }
 
+    /// Returns true if this StringRef has the given prefix and removes that
+    /// prefix.
+    bool consume_front(char Prefix) {
+      if (!starts_with(Prefix))
+        return false;
+
+      *this = drop_front();
+
+      return true;
+    }
+
     /// Returns true if this StringRef has the given prefix and removes that
     /// prefix.
     bool consume_front(StringRef Prefix) {
diff --git a/llvm/unittests/ADT/StringRefTest.cpp b/llvm/unittests/ADT/StringRefTest.cpp
index 1ace29e96dbb8..888cef9937e73 100644
--- a/llvm/unittests/ADT/StringRefTest.cpp
+++ b/llvm/unittests/ADT/StringRefTest.cpp
@@ -391,6 +391,26 @@ TEST(StringRefTest, ConsumeFront) {
   EXPECT_TRUE(Str.consume_front(""));
 }
 
+TEST(StringRefTest, ConsumeFrontChar) {
+  {
+    StringRef Str("hello");
+    EXPECT_EQ("hello", Str);
+    EXPECT_TRUE(Str.consume_front('h'));
+    EXPECT_EQ("ello", Str);
+    EXPECT_FALSE(Str.consume_front('h'));
+    EXPECT_EQ("ello", Str);
+  }
+
+  {
+    char Prefix = 'h';
+    StringRef Str("h");
+    EXPECT_TRUE(Str.consume_front(Prefix));
+    EXPECT_EQ("", Str);
+    EXPECT_FALSE(Str.consume_front('\0'));
+    EXPECT_EQ("", Str);
+  }
+}
+
 TEST(StringRefTest, ConsumeFrontInsensitive) {
   StringRef Str("heLLo");
   EXPECT_TRUE(Str.consume_front_insensitive(""));

@Michael137 Michael137 requested a review from kuhar December 18, 2025 10:33
Copy link
Member

@tgymnich tgymnich left a comment

Choose a reason for hiding this comment

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

Can we also add char versions of consume_front_insensitive, consume_back, consume_back_insensitive for the sake of completeness?

@Michael137
Copy link
Member Author

Can we also add char versions of consume_front_insensitive, consume_back, consume_back_insensitive for the sake of completeness?

Do people prefer landing them together or is it ok to do this incrementally?

@Michael137 Michael137 requested a review from dwblaikie December 18, 2025 19:29
@tgymnich
Copy link
Member

Either should be fine.

@Michael137
Copy link
Member Author

Either should be fine.

Sounds good! I'd prefer implementing them separately then to aid reviewability

Co-authored-by: Jakub Kuderski <kubakuderski@gmail.com>
@Michael137 Michael137 enabled auto-merge (squash) December 19, 2025 14:11
@Michael137 Michael137 disabled auto-merge December 19, 2025 14:12
@Michael137 Michael137 merged commit 15a63d4 into llvm:main Dec 19, 2025
10 checks passed
@Michael137 Michael137 deleted the llvm/stringref-consume_front-char branch December 19, 2025 14:58
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 19, 2025

LLVM Buildbot has detected a new failure on builder clang-m68k-linux-cross running on suse-gary-m68k-cross while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/20566

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
...
      |        ^
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1151:8: warning: suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
 1151 |     if (!ImplicitRanges.empty())
      |        ^
[188/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Driver/ToolChainTest.cpp.o
[189/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Parse/ParseHLSLRootSignatureTest.cpp.o
[190/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/CloneDetectionTest.cpp.o
[191/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/MacroExpansionContextTest.cpp.o
[192/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/DependencyScanning/DependencyScanningWorkerTest.cpp.o
[193/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp.o
FAILED: tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp.o 
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/llvm/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Tooling -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googletest/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googlemock/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-dangling-reference -Wno-redundant-move -Wno-pessimizing-move -Wno-array-bounds -Wno-stringop-overread -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -O3 -DNDEBUG -std=c++17  -Wno-variadic-macros -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp.o -MF tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp.o.d -o tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp.o -c /var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
c++: fatal error: Killed signal terminated program cc1plus
compilation terminated.
[194/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/ValueTest.cpp.o
[195/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o
FAILED: tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o 
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/llvm/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Tooling -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googletest/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googlemock/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-dangling-reference -Wno-redundant-move -Wno-pessimizing-move -Wno-array-bounds -Wno-stringop-overread -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -O3 -DNDEBUG -std=c++17  -Wno-variadic-macros -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o -MF tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o.d -o tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o -c /var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/IntervalPartitionTest.cpp
c++: fatal error: Killed signal terminated program cc1plus
compilation terminated.
[196/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/AnalyzerOptionsTest.cpp.o
[197/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MapLatticeTest.cpp.o
[198/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SmartPointerAccessorCachingTest.cpp.o
[199/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TestingSupport.cpp.o
[200/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SimplifyConstraintsTest.cpp.o
[201/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/BlockEntranceCallbackTest.cpp.o
[202/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTest.cpp.o
[203/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DeterminismTest.cpp.o
[204/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MatchSwitchTest.cpp.o
[205/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp.o
[206/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/LoggerTest.cpp.o
[207/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp.o
[208/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/CFGDominatorTree.cpp.o
[209/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DebugSupportTest.cpp.o
[210/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/RecordOpsTest.cpp.o
[211/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/CFGTest.cpp.o
[212/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/ASTOpsTest.cpp.o
[213/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp.o
[214/1239] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/LifetimeSafetyTest.cpp.o
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/LifetimeSafetyTest.cpp: In instantiation of ‘bool clang::lifetimes::internal::{anonymous}::AreLiveAtImplMatcherP2<Annotation_type, ConfFilter_type>::gmock_Impl<arg_type>::MatchAndExplain(const arg_type&, testing::MatchResultListener*) const [with arg_type = const clang::lifetimes::internal::{anonymous}::OriginsInfo&; Annotation_type = const char*; ConfFilter_type = clang::lifetimes::internal::{anonymous}::LivenessKindFilter]’:
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/LifetimeSafetyTest.cpp:310:1:   required from here
  310 | MATCHER_P2(AreLiveAtImpl, Annotation, ConfFilter, "") {
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/LifetimeSafetyTest.cpp:320:19: warning: loop variable ‘<structured bindings>’ creates a copy from type ‘const std::pair<clang::lifetimes::internal::utils::ID<clang::lifetimes::internal::OriginTag>, clang::lifetimes::internal::LivenessKind>’ [-Wrange-loop-construct]
  320 |   for (const auto [OID, ActualConfidence] : ActualLiveSetOpt.value()) {
      |                   ^~~~~~~~~~~~~~~~~~~~~~~
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/unittests/Analysis/LifetimeSafetyTest.cpp:320:19: note: use reference type to prevent copying
  320 |   for (const auto [OID, ActualConfidence] : ActualLiveSetOpt.value()) {
      |                   ^~~~~~~~~~~~~~~~~~~~~~~
      |                   &

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Dec 19, 2025
…72832)

This patch adds support for consuming a `char` from the front of a
`StringRef`. Most of the time a user wanting to consume a single
character off the front can just wrap the character in a string literal
(i.e., `consume_front("a")`). But this doesn't work if we don't have a
`char` literal, but instead a variable of type `char`. I.e., `char c =
'a'; str.consume_front(c)`. There's at least one helper in LLDB that
does this. Also there's plenty of example of `consume_front` being
passed a single character via string literal. This patch adds the `char`
overload. We already have a `starts_with(char)` overload, so there's at
least some related precedent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants