Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 17, 2025

Per LLVM Programmer's Manual, StringRef should always be passed by value. Enforcing this for Twine functions.

Per LLVM Programmer's Manual, StringRef should always be passed by
value. Enforcing this for a few ADT functions.
@jurahul jurahul force-pushed the nfc_use_stringref_byval branch from 5fdfa83 to 067e30d Compare October 17, 2025 17:04
@jurahul jurahul marked this pull request as ready for review October 17, 2025 19:50
@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2025

@llvm/pr-subscribers-llvm-adt

Author: Rahul Joshi (jurahul)

Changes

Per LLVM Programmer's Manual, StringRef should always be passed by value. Enforcing this for a few ADT functions.


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

2 Files Affected:

  • (modified) llvm/include/llvm/ADT/STLExtras.h (+3-3)
  • (modified) llvm/include/llvm/ADT/Twine.h (+5-5)
diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index 658f26249122a..3de09131f8c03 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Config/abi-breaking.h"
@@ -2215,13 +2216,12 @@ inline void interleave(const Container &c, UnaryFunctor each_fn,
 template <typename Container, typename UnaryFunctor, typename StreamT,
           typename T = detail::ValueOfRange<Container>>
 inline void interleave(const Container &c, StreamT &os, UnaryFunctor each_fn,
-                       const StringRef &separator) {
+                       StringRef separator) {
   interleave(adl_begin(c), adl_end(c), each_fn, [&] { os << separator; });
 }
 template <typename Container, typename StreamT,
           typename T = detail::ValueOfRange<Container>>
-inline void interleave(const Container &c, StreamT &os,
-                       const StringRef &separator) {
+inline void interleave(const Container &c, StreamT &os, StringRef separator) {
   interleave(
       c, os, [&](const T &a) { os << a; }, separator);
 }
diff --git a/llvm/include/llvm/ADT/Twine.h b/llvm/include/llvm/ADT/Twine.h
index d9f9c0f0d5d9c..e3b4d5e26fa17 100644
--- a/llvm/include/llvm/ADT/Twine.h
+++ b/llvm/include/llvm/ADT/Twine.h
@@ -285,7 +285,7 @@ class Twine {
   }
 
   /// Construct from a StringRef.
-  /*implicit*/ Twine(const StringRef &Str) : LHSKind(PtrAndLengthKind) {
+  /*implicit*/ Twine(StringRef Str) : LHSKind(PtrAndLengthKind) {
     LHS.ptrAndLength.ptr = Str.data();
     LHS.ptrAndLength.length = Str.size();
     assert(isValid() && "Invalid twine!");
@@ -352,7 +352,7 @@ class Twine {
   // right thing. Yet.
 
   /// Construct as the concatenation of a C string and a StringRef.
-  /*implicit*/ Twine(const char *LHS, const StringRef &RHS)
+  /*implicit*/ Twine(const char *LHS, StringRef RHS)
       : LHSKind(CStringKind), RHSKind(PtrAndLengthKind) {
     this->LHS.cString = LHS;
     this->RHS.ptrAndLength.ptr = RHS.data();
@@ -361,7 +361,7 @@ class Twine {
   }
 
   /// Construct as the concatenation of a StringRef and a C string.
-  /*implicit*/ Twine(const StringRef &LHS, const char *RHS)
+  /*implicit*/ Twine(StringRef LHS, const char *RHS)
       : LHSKind(PtrAndLengthKind), RHSKind(CStringKind) {
     this->LHS.ptrAndLength.ptr = LHS.data();
     this->LHS.ptrAndLength.length = LHS.size();
@@ -530,14 +530,14 @@ inline Twine operator+(const Twine &LHS, const Twine &RHS) {
 /// Additional overload to guarantee simplified codegen; this is equivalent to
 /// concat().
 
-inline Twine operator+(const char *LHS, const StringRef &RHS) {
+inline Twine operator+(const char *LHS, StringRef RHS) {
   return Twine(LHS, RHS);
 }
 
 /// Additional overload to guarantee simplified codegen; this is equivalent to
 /// concat().
 
-inline Twine operator+(const StringRef &LHS, const char *RHS) {
+inline Twine operator+(StringRef LHS, const char *RHS) {
   return Twine(LHS, RHS);
 }
 

@jurahul jurahul requested a review from kuhar October 21, 2025 00:00
@jurahul
Copy link
Contributor Author

jurahul commented Oct 21, 2025

@kuhar PTAL

@jurahul jurahul changed the title [NFC][LLVM][ADT] Change a few const StringRef & arguments to value [NFC][LLVM][Twine] Change a few const StringRef & arguments to value Oct 21, 2025
@jurahul jurahul merged commit f6799d2 into llvm:main Oct 21, 2025
9 of 10 checks passed
@jurahul jurahul deleted the nfc_use_stringref_byval branch October 21, 2025 20:47
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.

3 participants