Merge pull request #14090 from Radvendii/expr-slim

nixexpr: introduce arena to hold ExprString strings

Author: Mic92

src/libexpr/eval.cc

@@ -3217,7 +3217,8 @@ Expr * EvalState::parse(
         docComments = &it->second;
     }
 
-    auto result = parseExprFromBuf(text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS);
+    auto result = parseExprFromBuf(
+        text, length, origin, basePath, mem.exprs.alloc, symbols, settings, positions, *docComments, rootFS);
 
     result->bindVars(*this, staticEnv);
 

src/libexpr/include/nix/expr/eval.hh

@@ -355,6 +355,11 @@ public:
         return stats;
     }
 
+    /**
+     * Storage for the AST nodes
+     */
+    Exprs exprs;
+
 private:
     Statistics stats;
 };

src/libexpr/include/nix/expr/nixexpr.hh

@@ -3,6 +3,7 @@
 
 #include <map>
 #include <vector>
+#include <memory_resource>
 
 #include "nix/expr/gc-small-vector.hh"
 #include "nix/expr/value.hh"
@@ -84,6 +85,13 @@ std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath)
 
 using UpdateQueue = SmallTemporaryValueVector<conservativeStackReservation>;
 
+class Exprs
+{
+    std::pmr::monotonic_buffer_resource buffer;
+public:
+    std::pmr::polymorphic_allocator<char> alloc{&buffer};
+};
+
 /* Abstract syntax of Nix expressions. */
 
 struct Expr
@@ -173,13 +181,28 @@ struct ExprFloat : Expr
 
 struct ExprString : Expr
 {
-    std::string s;
     Value v;
 
-    ExprString(std::string && s)
-        : s(std::move(s))
+    /**
+     * 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)
     {
-        v.mkStringNoCopy(this->s.data());
+        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);
     };
 
     Value * maybeThunk(EvalState & state, Env & env) override;

src/libexpr/include/nix/expr/parser-state.hh

@@ -78,6 +78,7 @@ struct LexerState
 struct ParserState
 {
     const LexerState & lexerState;
+    std::pmr::polymorphic_allocator<char> & alloc;
     SymbolTable & symbols;
     PosTable & positions;
     Expr * result;
@@ -323,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(std::move(s2)));
+            es2->emplace_back(i->first, new ExprString(alloc, s2));
         }
     };
     for (; i != es.end(); ++i, --n) {

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, s);
+    printLiteralString(str, v.string_view());
 }
 
 void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const

src/libexpr/parser.y

@@ -59,6 +59,7 @@ Expr * parseExprFromBuf(
     size_t length,
     Pos::Origin origin,
     const SourcePath & basePath,
+    std::pmr::polymorphic_allocator<char> & alloc,
     SymbolTable & symbols,
     const EvalSettings & settings,
     PosTable & positions,
@@ -135,6 +136,7 @@ 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;
 }
 
@@ -149,7 +151,8 @@ 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 string_parts string_attr
+%type <e> path_start
+%type <to_be_string> string_parts string_attr
 %type <id> attr
 %token <id> ID
 %token <str> STR IND_STR
@@ -304,7 +307,13 @@ expr_simple
   }
   | INT_LIT { $$ = new ExprInt($1); }
   | FLOAT_LIT { $$ = new ExprFloat($1); }
-  | '"' string_parts '"' { $$ = $2; }
+  | '"' string_parts '"' {
+      std::visit(overloaded{
+          [&](std::string_view str) { $$ = new ExprString(state->alloc, str); },
+          [&](Expr * expr) { $$ = expr; }},
+      *$2);
+      delete $2;
+  }
   | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE {
       $$ = state->stripIndentation(CUR_POS, std::move(*$2));
       delete $2;
@@ -315,11 +324,11 @@ expr_simple
       $$ = new ExprConcatStrings(CUR_POS, false, $2);
   }
   | SPATH {
-      std::string path($1.p + 1, $1.l - 2);
+      std::string_view path($1.p + 1, $1.l - 2);
       $$ = new ExprCall(CUR_POS,
           new ExprVar(state->s.findFile),
           {new ExprVar(state->s.nixPath),
-           new ExprString(std::move(path))});
+           new ExprString(state->alloc, path)});
   }
   | URI {
       static bool noURLLiterals = experimentalFeatureSettings.isEnabled(Xp::NoUrlLiterals);
@@ -328,7 +337,7 @@ expr_simple
               .msg = HintFmt("URL literals are disabled"),
               .pos = state->positions[CUR_POS]
           });
-      $$ = new ExprString(std::string($1));
+      $$ = new ExprString(state->alloc, $1);
   }
   | '(' expr ')' { $$ = $2; }
   /* Let expressions `let {..., body = ...}' are just desugared
@@ -345,19 +354,19 @@ expr_simple
   ;
 
 string_parts
-  : STR { $$ = new ExprString(std::string($1)); }
-  | string_parts_interpolated { $$ = new ExprConcatStrings(CUR_POS, true, $1); }
-  | { $$ = new ExprString(""); }
+  : 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()); }
   ;
 
 string_parts_interpolated
   : string_parts_interpolated STR
-  { $$ = $1; $1->emplace_back(state->at(@2), new ExprString(std::string($2))); }
+  { $$ = $1; $1->emplace_back(state->at(@2), new ExprString(state->alloc, $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(std::string($1)));
+      $$->emplace_back(state->at(@1), new ExprString(state->alloc, $1));
       $$->emplace_back(state->at(@2), $3);
     }
   ;
@@ -455,15 +464,16 @@ attrs
   : attrs attr { $$ = $1; $1->emplace_back(AttrName(state->symbols.create($2)), state->at(@2)); }
   | attrs string_attr
     { $$ = $1;
-      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)]
-          });
+      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;
     }
   | { $$ = new std::vector<std::pair<AttrName, PosIdx>>; }
   ;
@@ -472,22 +482,20 @@ attrpath
   : attrpath '.' attr { $$ = $1; $1->push_back(AttrName(state->symbols.create($3))); }
   | attrpath '.' string_attr
     { $$ = $1;
-      ExprString * str = dynamic_cast<ExprString *>($3);
-      if (str) {
-          $$->push_back(AttrName(state->symbols.create(str->s)));
-          delete str;
-      } else
-          $$->push_back(AttrName($3));
+      std::visit(overloaded {
+          [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); },
+          [&](Expr * expr) { $$->push_back(AttrName(expr)); }
+      }, *$3);
+      delete $3;
     }
   | attr { $$ = new std::vector<AttrName>; $$->push_back(AttrName(state->symbols.create($1))); }
   | string_attr
     { $$ = new std::vector<AttrName>;
-      ExprString *str = dynamic_cast<ExprString *>($1);
-      if (str) {
-          $$->push_back(AttrName(state->symbols.create(str->s)));
-          delete str;
-      } else
-          $$->push_back(AttrName($1));
+      std::visit(overloaded {
+          [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); },
+          [&](Expr * expr) { $$->push_back(AttrName(expr)); }
+      }, *$1);
+      delete $1;
     }
   ;
 
@@ -498,7 +506,7 @@ attr
 
 string_attr
   : '"' string_parts '"' { $$ = $2; }
-  | DOLLAR_CURLY expr '}' { $$ = $2; }
+  | DOLLAR_CURLY expr '}' { $$ = new std::variant<Expr *, std::string_view>($2); }
   ;
 
 expr_list
@@ -538,6 +546,7 @@ Expr * parseExprFromBuf(
     size_t length,
     Pos::Origin origin,
     const SourcePath & basePath,
+    std::pmr::polymorphic_allocator<char> & alloc,
     SymbolTable & symbols,
     const EvalSettings & settings,
     PosTable & positions,
@@ -552,6 +561,7 @@ Expr * parseExprFromBuf(
     };
     ParserState state {
         .lexerState = lexerState,
+        .alloc = alloc,
         .symbols = symbols,
         .positions = positions,
         .basePath = basePath,