Merge pull request #14124 from Radvendii/exprselect-alloc

libexpr: allocate ExprSelect's AttrPath in Expr::alloc

src/libexpr/eval.cc

@@ -1341,7 +1341,7 @@ void ExprVar::eval(EvalState & state, Env & env, Value & v)
     v = *v2;
 }
 
-static std::string showAttrPath(EvalState & state, Env & env, const AttrPath & attrPath)
+static std::string showAttrPath(EvalState & state, Env & env, std::span<const AttrName> attrPath)
 {
     std::ostringstream out;
     bool first = true;
@@ -1377,10 +1377,10 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
                                          env,
                                          getPos(),
                                          "while evaluating the attribute '%1%'",
-                                         showAttrPath(state, env, attrPath))
+                                         showAttrPath(state, env, getAttrPath()))
                                    : nullptr;
 
-        for (auto & i : attrPath) {
+        for (auto & i : getAttrPath()) {
             state.nrLookups++;
             const Attr * j;
             auto name = getName(i, state, env);
@@ -1418,7 +1418,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
             auto origin = std::get_if<SourcePath>(&pos2r.origin);
             if (!(origin && *origin == state.derivationInternal))
                 state.addErrorTrace(
-                    e, pos2, "while evaluating the attribute '%1%'", showAttrPath(state, env, attrPath));
+                    e, pos2, "while evaluating the attribute '%1%'", showAttrPath(state, env, getAttrPath()));
         }
         throw;
     }
@@ -1429,13 +1429,13 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
 Symbol ExprSelect::evalExceptFinalSelect(EvalState & state, Env & env, Value & attrs)
 {
     Value vTmp;
-    Symbol name = getName(attrPath[attrPath.size() - 1], state, env);
+    Symbol name = getName(attrPathStart[nAttrPath - 1], state, env);
 
-    if (attrPath.size() == 1) {
+    if (nAttrPath == 1) {
         e->eval(state, env, vTmp);
     } else {
         ExprSelect init(*this);
-        init.attrPath.pop_back();
+        init.nAttrPath--;
         init.eval(state, env, vTmp);
     }
     attrs = vTmp;

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

@@ -2,8 +2,10 @@
 ///@file
 
 #include <map>
+#include <span>
 #include <vector>
 #include <memory_resource>
+#include <algorithm>
 
 #include "nix/expr/gc-small-vector.hh"
 #include "nix/expr/value.hh"
@@ -79,9 +81,11 @@ struct AttrName
         : expr(e) {};
 };
 
+static_assert(std::is_trivially_copy_constructible_v<AttrName>);
+
 typedef std::vector<AttrName> AttrPath;
 
-std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath);
+std::string showAttrPath(const SymbolTable & symbols, std::span<const AttrName> attrPath);
 
 using UpdateQueue = SmallTemporaryValueVector<conservativeStackReservation>;
 
@@ -288,20 +292,33 @@ struct ExprInheritFrom : ExprVar
 struct ExprSelect : Expr
 {
     PosIdx pos;
+    uint32_t nAttrPath;
     Expr *e, *def;
-    AttrPath attrPath;
+    AttrName * attrPathStart;
-    ExprSelect(const PosIdx & pos, Expr * e, AttrPath attrPath, Expr * def)
+
+    ExprSelect(
+        std::pmr::polymorphic_allocator<char> & alloc,
+        const PosIdx & pos,
+        Expr * e,
+        std::span<const AttrName> attrPath,
+        Expr * def)
         : pos(pos)
+        , nAttrPath(attrPath.size())
         , e(e)
         , def(def)
-        , attrPath(std::move(attrPath)) {};
+        , attrPathStart(alloc.allocate_object<AttrName>(nAttrPath))
+    {
+        std::ranges::copy(attrPath, attrPathStart);
+    };
 
-    ExprSelect(const PosIdx & pos, Expr * e, Symbol name)
+    ExprSelect(std::pmr::polymorphic_allocator<char> & alloc, const PosIdx & pos, Expr * e, Symbol name)
         : pos(pos)
+        , nAttrPath(1)
         , e(e)
         , def(0)
+        , attrPathStart((alloc.allocate_object<AttrName>()))
     {
-        attrPath.push_back(AttrName(name));
+        *attrPathStart = AttrName(name);
     };
 
     PosIdx getPos() const override
@@ -309,6 +326,11 @@ struct ExprSelect : Expr
         return pos;
     }
 
+    std::span<const AttrName> getAttrPath() const
+    {
+        return {attrPathStart, nAttrPath};
+    }
+
     /**
      * Evaluate the `a.b.c` part of `a.b.c.d`. This exists mostly for the purpose of :doc in the repl.
      *

src/libexpr/nixexpr.cc

@@ -57,7 +57,7 @@ void ExprSelect::show(const SymbolTable & symbols, std::ostream & str) const
 {
     str << "(";
     e->show(symbols, str);
-    str << ")." << showAttrPath(symbols, attrPath);
+    str << ")." << showAttrPath(symbols, getAttrPath());
     if (def) {
         str << " or (";
         def->show(symbols, str);
@@ -261,7 +261,7 @@ void ExprPos::show(const SymbolTable & symbols, std::ostream & str) const
     str << "__curPos";
 }
 
-std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath)
+std::string showAttrPath(const SymbolTable & symbols, std::span<const AttrName> attrPath)
 {
     std::ostringstream out;
     bool first = true;
@@ -362,7 +362,7 @@ void ExprSelect::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv>
     e->bindVars(es, env);
     if (def)
         def->bindVars(es, env);
-    for (auto & i : attrPath)
+    for (auto & i : getAttrPath())
         if (!i.symbol)
             i.expr->bindVars(es, env);
 }

src/libexpr/parser.y

@@ -282,9 +282,9 @@ expr_app
 
 expr_select
   : expr_simple '.' attrpath
-    { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), nullptr); delete $3; }
+    { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move(*$3), nullptr); delete $3; }
   | expr_simple '.' attrpath OR_KW expr_select
-    { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; $5->warnIfCursedOr(state->symbols, state->positions); }
+    { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move(*$3), $5); delete $3; $5->warnIfCursedOr(state->symbols, state->positions); }
   | /* Backwards compatibility: because Nixpkgs has a function named ‘or’,
        allow stuff like ‘map or [...]’. This production is problematic (see
        https://github.com/NixOS/nix/issues/11118) and will be refactored in the
@@ -343,7 +343,7 @@ expr_simple
   /* Let expressions `let {..., body = ...}' are just desugared
      into `(rec {..., body = ...}).body'. */
   | LET '{' binds '}'
-    { $3->recursive = true; $3->pos = CUR_POS; $$ = new ExprSelect(noPos, $3, state->s.body); }
+    { $3->recursive = true; $3->pos = CUR_POS; $$ = new ExprSelect(state->alloc, noPos, $3, state->s.body); }
   | REC '{' binds '}'
     { $3->recursive = true; $3->pos = CUR_POS; $$ = $3; }
   | '{' binds1 '}'
@@ -447,7 +447,7 @@ binds1
           $accum->attrs.emplace(
               i.symbol,
               ExprAttrs::AttrDef(
-                  new ExprSelect(iPos, from, i.symbol),
+                  new ExprSelect(state->alloc, iPos, from, i.symbol),
                   iPos,
                   ExprAttrs::AttrDef::Kind::InheritedFrom));
       }