From f029b14eaaea40abaeb86f8ea794fae6bb59b463 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Mon, 3 Nov 2025 18:53:27 -0500
Subject: [PATCH] Factor out type alias for string contexts on the heap

This will make it easier to change this type in the future.

See new TODO on naming. The thing we already so-named is a builder type
for string contexts, not the on-heap type.

Co-Authored-By: Taeer Bar-Yam <taeer@bar-yam.me
---
 src/libexpr/eval-cache.cc                     |  4 ++--
 src/libexpr/eval.cc                           |  6 +++---
 src/libexpr/include/nix/expr/value.hh         | 17 ++++++++++++++---
 src/libexpr/include/nix/expr/value/context.hh | 13 +++++++++++++
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc
index de74d2143..887b2946f 100644
--- a/src/libexpr/eval-cache.cc
+++ b/src/libexpr/eval-cache.cc
@@ -136,14 +136,14 @@ struct AttrDb
         });
     }
 
-    AttrId setString(AttrKey key, std::string_view s, const char ** context = nullptr)
+    AttrId setString(AttrKey key, std::string_view s, Value::StringWithContext::Context context = nullptr)
     {
         return doSQLite([&]() {
             auto state(_state->lock());
 
             if (context) {
                 std::string ctx;
-                for (const char ** p = context; *p; ++p) {
+                for (Value::StringWithContext::Context p = context; *p; ++p) {
                     if (p != context)
                         ctx.push_back(' ');
                     ctx.append(*p);
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index e2687148b..f25a72511 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -821,11 +821,11 @@ void Value::mkString(std::string_view s)
     mkStringNoCopy(makeImmutableString(s));
 }
 
-static const char ** encodeContext(const NixStringContext & context)
+static Value::StringWithContext::Context encodeContext(const NixStringContext & context)
 {
     if (!context.empty()) {
         size_t n = 0;
-        auto ctx = (const char **) allocBytes((context.size() + 1) * sizeof(char *));
+        auto ctx = (Value::StringWithContext::Context) allocBytes((context.size() + 1) * sizeof(char *));
         for (auto & i : context) {
             ctx[n++] = makeImmutableString({i.to_string()});
         }
@@ -2288,7 +2288,7 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string
 void copyContext(const Value & v, NixStringContext & context, const ExperimentalFeatureSettings & xpSettings)
 {
     if (v.context())
-        for (const char ** p = v.context(); *p; ++p)
+        for (Value::StringWithContext::Context p = v.context(); *p; ++p)
             context.insert(NixStringContextElem::parse(*p, xpSettings));
 }
 
diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh
index 706a4fe3f..65204b642 100644
--- a/src/libexpr/include/nix/expr/value.hh
+++ b/src/libexpr/include/nix/expr/value.hh
@@ -220,7 +220,18 @@ struct ValueBase
     struct StringWithContext
     {
         const char * c_str;
-        const char ** context; // must be in sorted order
+        /**
+         * The type of the context itself.
+         *
+         * Currently, it is pointer to an array of pointers to strings.
+         * The strings are specially formatted to represent a flattening
+         * of the recursive sum type that is s context element.
+         *
+         * @See NixStringContext for an more easily understood type,
+         * that of the "builder" for this data structure.
+         */
+        using Context = const char **;
+        Context context; // must be in sorted order
     };
 
     struct Path
@@ -991,7 +1002,7 @@ public:
         setStorage(b);
     }
 
-    void mkStringNoCopy(const char * s, const char ** context = 0) noexcept
+    void mkStringNoCopy(const char * s, Value::StringWithContext::Context context = 0) noexcept
     {
         setStorage(StringWithContext{.c_str = s, .context = context});
     }
@@ -1117,7 +1128,7 @@ public:
         return getStorage<StringWithContext>().c_str;
     }
 
-    const char ** context() const noexcept
+    Value::StringWithContext::Context context() const noexcept
     {
         return getStorage<StringWithContext>().context;
     }
diff --git a/src/libexpr/include/nix/expr/value/context.hh b/src/libexpr/include/nix/expr/value/context.hh
index dcfacbb21..625adc37b 100644
--- a/src/libexpr/include/nix/expr/value/context.hh
+++ b/src/libexpr/include/nix/expr/value/context.hh
@@ -24,6 +24,14 @@ public:
     }
 };
 
+/**
+ * @todo This should be reamed to `StringContextBuilderElem`, since:
+ *
+ * 1. We use `*Builder` for off-heap temporary data structures
+ *
+ * 2. The `Nix*` is totally redundant. (And my mistake from a long time
+ * ago.)
+ */
 struct NixStringContextElem
 {
     /**
@@ -77,6 +85,11 @@ struct NixStringContextElem
     std::string to_string() const;
 };
 
+/**
+ * @todo This should be renamed to `StringContextBuilder`.
+ *
+ * @see NixStringContextElem for explanation why.
+ */
 typedef std::set<NixStringContextElem> NixStringContext;
 
 } // namespace nix