From e67dac1d7493741cf88b411f68e31fc496179bf2 Mon Sep 17 00:00:00 2001
From: Qyriad <qyriad@qyriad.me>
Date: Mon, 15 Jul 2024 18:19:54 -0600
Subject: [PATCH] libexpr: rename confusing makeImmutableString ->
 gcCopyStringIfNeeded

The purpose of this function has little to do with immutability. Value's
strings are never mutated, and the point of this function is to
singleton empty strings.

Change-Id: Ifd41dd952409d54e4d3de9ab59064e6928b0e480
---
 src/libexpr/eval.cc     | 21 +++------------------
 src/libexpr/gc-alloc.cc | 23 +++++++++++++++++++++++
 src/libexpr/gc-alloc.hh |  5 +++++
 src/libexpr/meson.build |  1 +
 4 files changed, 32 insertions(+), 18 deletions(-)
 create mode 100644 src/libexpr/gc-alloc.cc

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index ce469e5c5..06b1f27f5 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -49,21 +49,6 @@ using json = nlohmann::json;
 
 namespace nix {
 
-// When there's no need to write to the string, we can optimize away empty
-// string allocations.
-// This function handles makeImmutableString(std::string_view()) by returning
-// the empty string.
-static const char * makeImmutableString(std::string_view s)
-{
-    const size_t size = s.size();
-    if (size == 0)
-        return "";
-    auto t = gcAllocString(size + 1);
-    memcpy(t, s.data(), size);
-    t[size] = '\0';
-    return t;
-}
-
 RootValue allocRootValue(Value * v)
 {
 #if HAVE_BOEHMGC
@@ -784,7 +769,7 @@ DebugTraceStacker::DebugTraceStacker(EvalState & evalState, DebugTrace t)
 
 void Value::mkString(std::string_view s)
 {
-    mkString(makeImmutableString(s));
+    mkString(gcCopyStringIfNeeded(s));
 }
 
 
@@ -795,7 +780,7 @@ static void copyContextToValue(Value & v, const NixStringContext & context)
         v.string.context = (const char * *)
             gcAllocBytes((context.size() + 1) * sizeof(char *));
         for (auto & i : context)
-            v.string.context[n++] = makeImmutableString(i.to_string());
+            v.string.context[n++] = gcCopyStringIfNeeded(i.to_string());
         v.string.context[n] = 0;
     }
 }
@@ -815,7 +800,7 @@ void Value::mkStringMove(const char * s, const NixStringContext & context)
 
 void Value::mkPath(const SourcePath & path)
 {
-    mkPath(makeImmutableString(path.path.abs()));
+    mkPath(gcCopyStringIfNeeded(path.path.abs()));
 }
 
 
diff --git a/src/libexpr/gc-alloc.cc b/src/libexpr/gc-alloc.cc
new file mode 100644
index 000000000..558a2f2bc
--- /dev/null
+++ b/src/libexpr/gc-alloc.cc
@@ -0,0 +1,23 @@
+#include "gc-alloc.hh"
+
+#include <cstring>
+#include <string_view>
+
+namespace nix
+{
+
+char const * gcCopyStringIfNeeded(std::string_view toCopyFrom)
+{
+    if (toCopyFrom.empty()) {
+        return "";
+    }
+
+    size_t const size = toCopyFrom.size();
+    char * cstr = gcAllocString(size + 1);
+    memcpy(cstr, toCopyFrom.data(), size);
+    cstr[size] = '\0';
+
+    return cstr;
+}
+
+}
diff --git a/src/libexpr/gc-alloc.hh b/src/libexpr/gc-alloc.hh
index a16bc65e4..f691bfc4b 100644
--- a/src/libexpr/gc-alloc.hh
+++ b/src/libexpr/gc-alloc.hh
@@ -7,6 +7,7 @@
 #include <list>
 #include <map>
 #include <new>
+#include <string_view>
 #include <vector>
 
 #if HAVE_BOEHMGC
@@ -116,4 +117,8 @@ inline char * gcAllocString(size_t size)
     return cstr;
 }
 
+/// Returns a C-string copied from @ref toCopyFrom, or a single, static empty
+/// string if @ref toCopyFrom is also empty.
+char const * gcCopyStringIfNeeded(std::string_view toCopyFrom);
+
 }
diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build
index d29359298..b11e4c2e0 100644
--- a/src/libexpr/meson.build
+++ b/src/libexpr/meson.build
@@ -22,6 +22,7 @@ libexpr_sources = files(
   'eval.cc',
   'function-trace.cc',
   'get-drvs.cc',
+  'gc-alloc.cc',
   'json-to-value.cc',
   'nixexpr.cc',
   'parser/parser.cc',