From 1e190d840a1f655057375ab1b77467f226c65ae2 Mon Sep 17 00:00:00 2001
From: Sergei Zimmerman <sergei@zimmerman.foo>
Date: Wed, 26 Nov 2025 00:39:36 +0300
Subject: [PATCH] Revert "libexpr: introduce arena to hold ExprString strings"

This reverts commit eab467ecfb829182548276df7d56a4d1c525057a.

Simpler to revert the offending patch that lead to dynamic attributes
regression than to fix it.
---
 src/libexpr/include/nix/expr/nixexpr.hh      | 23 ++-----
 src/libexpr/include/nix/expr/parser-state.hh |  2 +-
 src/libexpr/nixexpr.cc                       |  2 +-
 src/libexpr/parser.y                         | 71 +++++++++-----------
 4 files changed, 38 insertions(+), 60 deletions(-)

diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh
index 863a1369d..5f18a3966 100644
--- a/src/libexpr/include/nix/expr/nixexpr.hh
+++ b/src/libexpr/include/nix/expr/nixexpr.hh
@@ -185,28 +185,13 @@ struct ExprFloat : Expr
 
 struct ExprString : Expr
 {
+    std::string s;
     Value v;
 
-    /**
-     * This is only for strings already allocated in our polymorphic allocator,
-     * or that live at least that long (e.g. c++ string literals)
-     */
-    ExprString(const char * s)
+    ExprString(std::string && s)
+        : s(std::move(s))
     {
-        v.mkStringNoCopy(s);
-    };
-
-    ExprString(std::pmr::polymorphic_allocator<char> & alloc, std::string_view sv)
-    {
-        auto len = sv.length();
-        if (len == 0) {
-            v.mkStringNoCopy("");
-            return;
-        }
-        char * s = alloc.allocate(len + 1);
-        sv.copy(s, len);
-        s[len] = '\0';
-        v.mkStringNoCopy(s);
+        v.mkStringNoCopy(this->s.data());
     };
 
     Value * maybeThunk(EvalState & state, Env & env) override;
diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh
index 55dce3047..a5d84b3dd 100644
--- a/src/libexpr/include/nix/expr/parser-state.hh
+++ b/src/libexpr/include/nix/expr/parser-state.hh
@@ -324,7 +324,7 @@ ParserState::stripIndentation(const PosIdx pos, std::vector<std::pair<PosIdx, st
 
         // Ignore empty strings for a minor optimisation and AST simplification
         if (s2 != "") {
-            es2->emplace_back(i->first, new ExprString(alloc, s2));
+            es2->emplace_back(i->first, new ExprString(std::move(s2)));
         }
     };
     for (; i != es.end(); ++i, --n) {
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 5b9d17d49..5e07a59c4 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -40,7 +40,7 @@ void ExprFloat::show(const SymbolTable & symbols, std::ostream & str) const
 
 void ExprString::show(const SymbolTable & symbols, std::ostream & str) const
 {
-    printLiteralString(str, v.string_view());
+    printLiteralString(str, s);
 }
 
 void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index 9186fcf4b..3d5a98b00 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -136,7 +136,6 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
   std::vector<nix::AttrName> * attrNames;
   std::vector<std::pair<nix::AttrName, nix::PosIdx>> * inheritAttrs;
   std::vector<std::pair<nix::PosIdx, nix::Expr *>> * string_parts;
-  std::variant<nix::Expr *, std::string_view> * to_be_string;
   std::vector<std::pair<nix::PosIdx, std::variant<nix::Expr *, nix::StringToken>>> * ind_string_parts;
 }
 
@@ -151,8 +150,7 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
 %type <inheritAttrs> attrs
 %type <string_parts> string_parts_interpolated
 %type <ind_string_parts> ind_string_parts
-%type <e> path_start
-%type <to_be_string> string_parts string_attr
+%type <e> path_start string_parts string_attr
 %type <id> attr
 %token <id> ID
 %token <str> STR IND_STR
@@ -307,13 +305,7 @@ expr_simple
   }
   | INT_LIT { $$ = new ExprInt($1); }
   | FLOAT_LIT { $$ = new ExprFloat($1); }
-  | '"' string_parts '"' {
-      std::visit(overloaded{
-          [&](std::string_view str) { $$ = new ExprString(state->alloc, str); },
-          [&](Expr * expr) { $$ = expr; }},
-      *$2);
-      delete $2;
-  }
+  | '"' string_parts '"' { $$ = $2; }
   | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE {
       $$ = state->stripIndentation(CUR_POS, std::move(*$2));
       delete $2;
@@ -324,11 +316,11 @@ expr_simple
       $$ = new ExprConcatStrings(CUR_POS, false, $2);
   }
   | SPATH {
-      std::string_view path($1.p + 1, $1.l - 2);
+      std::string path($1.p + 1, $1.l - 2);
       $$ = new ExprCall(CUR_POS,
           new ExprVar(state->s.findFile),
           {new ExprVar(state->s.nixPath),
-           new ExprString(state->alloc, path)});
+           new ExprString(std::move(path))});
   }
   | URI {
       static bool noURLLiterals = experimentalFeatureSettings.isEnabled(Xp::NoUrlLiterals);
@@ -337,7 +329,7 @@ expr_simple
               .msg = HintFmt("URL literals are disabled"),
               .pos = state->positions[CUR_POS]
           });
-      $$ = new ExprString(state->alloc, $1);
+      $$ = new ExprString(std::string($1));
   }
   | '(' expr ')' { $$ = $2; }
   /* Let expressions `let {..., body = ...}' are just desugared
@@ -354,19 +346,19 @@ expr_simple
   ;
 
 string_parts
-  : STR { $$ = new std::variant<Expr *, std::string_view>($1); }
-  | string_parts_interpolated { $$ = new std::variant<Expr *, std::string_view>(new ExprConcatStrings(CUR_POS, true, $1)); }
-  | { $$ = new std::variant<Expr *, std::string_view>(std::string_view()); }
+  : STR { $$ = new ExprString(std::string($1)); }
+  | string_parts_interpolated { $$ = new ExprConcatStrings(CUR_POS, true, $1); }
+  | { $$ = new ExprString(""); }
   ;
 
 string_parts_interpolated
   : string_parts_interpolated STR
-  { $$ = $1; $1->emplace_back(state->at(@2), new ExprString(state->alloc, $2)); }
+  { $$ = $1; $1->emplace_back(state->at(@2), new ExprString(std::string($2))); }
   | string_parts_interpolated DOLLAR_CURLY expr '}' { $$ = $1; $1->emplace_back(state->at(@2), $3); }
   | DOLLAR_CURLY expr '}' { $$ = new std::vector<std::pair<PosIdx, Expr *>>; $$->emplace_back(state->at(@1), $2); }
   | STR DOLLAR_CURLY expr '}' {
       $$ = new std::vector<std::pair<PosIdx, Expr *>>;
-      $$->emplace_back(state->at(@1), new ExprString(state->alloc, $1));
+      $$->emplace_back(state->at(@1), new ExprString(std::string($1)));
       $$->emplace_back(state->at(@2), $3);
     }
   ;
@@ -464,16 +456,15 @@ attrs
   : attrs attr { $$ = $1; $1->emplace_back(AttrName(state->symbols.create($2)), state->at(@2)); }
   | attrs string_attr
     { $$ = $1;
-      std::visit(overloaded {
-          [&](std::string_view str) { $$->emplace_back(AttrName(state->symbols.create(str)), state->at(@2)); },
-          [&](Expr * expr) {
-              throw ParseError({
-                  .msg = HintFmt("dynamic attributes not allowed in inherit"),
-                  .pos = state->positions[state->at(@2)]
-              });
-          }
-      }, *$2);
-      delete $2;
+      ExprString * str = dynamic_cast<ExprString *>($2);
+      if (str) {
+          $$->emplace_back(AttrName(state->symbols.create(str->s)), state->at(@2));
+          delete str;
+      } else
+          throw ParseError({
+              .msg = HintFmt("dynamic attributes not allowed in inherit"),
+              .pos = state->positions[state->at(@2)]
+          });
     }
   | { $$ = new std::vector<std::pair<AttrName, PosIdx>>; }
   ;
@@ -482,20 +473,22 @@ attrpath
   : attrpath '.' attr { $$ = $1; $1->push_back(AttrName(state->symbols.create($3))); }
   | attrpath '.' string_attr
     { $$ = $1;
-      std::visit(overloaded {
-          [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); },
-          [&](Expr * expr) { $$->push_back(AttrName(expr)); }
-      }, *$3);
-      delete $3;
+      ExprString * str = dynamic_cast<ExprString *>($3);
+      if (str) {
+          $$->push_back(AttrName(state->symbols.create(str->s)));
+          delete str;
+      } else
+          $$->push_back(AttrName($3));
     }
   | attr { $$ = new std::vector<AttrName>; $$->push_back(AttrName(state->symbols.create($1))); }
   | string_attr
     { $$ = new std::vector<AttrName>;
-      std::visit(overloaded {
-          [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); },
-          [&](Expr * expr) { $$->push_back(AttrName(expr)); }
-      }, *$1);
-      delete $1;
+      ExprString *str = dynamic_cast<ExprString *>($1);
+      if (str) {
+          $$->push_back(AttrName(state->symbols.create(str->s)));
+          delete str;
+      } else
+          $$->push_back(AttrName($1));
     }
   ;
 
@@ -506,7 +499,7 @@ attr
 
 string_attr
   : '"' string_parts '"' { $$ = $2; }
-  | DOLLAR_CURLY expr '}' { $$ = new std::variant<Expr *, std::string_view>($2); }
+  | DOLLAR_CURLY expr '}' { $$ = $2; }
   ;
 
 expr_list