Merge pull request #14061 from xokdvium/k-way-update

libexpr: Preparation for more k-way updates of attribute sets (NFC)

src/libexpr/eval.cc

@@ -1866,12 +1866,8 @@ void ExprOpImpl::eval(EvalState & state, Env & env, Value & v)
         || state.evalBool(env, e2, pos, "in the right operand of the IMPL (->) operator"));
 }
 
-void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
+void ExprOpUpdate::eval(EvalState & state, Value & v, Value & v1, Value & v2)
 {
-    Value v1, v2;
-    state.evalAttrs(env, e1, v1, pos, "in the left operand of the update (//) operator");
-    state.evalAttrs(env, e2, v2, pos, "in the right operand of the update (//) operator");
-
     state.nrOpUpdates++;
 
     const Bindings & bindings1 = *v1.attrs();
@@ -1945,6 +1941,38 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
     state.nrOpUpdateValuesCopied += v.attrs()->size();
 }
 
+void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
+{
+    UpdateQueue q;
+    evalForUpdate(state, env, q);
+
+    v.mkAttrs(&Bindings::emptyBindings);
+    for (auto & rhs : std::views::reverse(q)) {
+        /* Remember that queue is sorted rightmost attrset first. */
+        eval(state, /*v=*/v, /*v1=*/v, /*v2=*/rhs);
+    }
+}
+
+void Expr::evalForUpdate(EvalState & state, Env & env, UpdateQueue & q, std::string_view errorCtx)
+{
+    Value v;
+    state.evalAttrs(env, this, v, getPos(), errorCtx);
+    q.push_back(v);
+}
+
+void ExprOpUpdate::evalForUpdate(EvalState & state, Env & env, UpdateQueue & q)
+{
+    /* Output rightmost attrset first to the merge queue as the one
+       with the most priority. */
+    e2->evalForUpdate(state, env, q, "in the right operand of the update (//) operator");
+    e1->evalForUpdate(state, env, q, "in the left operand of the update (//) operator");
+}
+
+void ExprOpUpdate::evalForUpdate(EvalState & state, Env & env, UpdateQueue & q, std::string_view errorCtx)
+{
+    evalForUpdate(state, env, q);
+}
+
 void ExprOpConcatLists::eval(EvalState & state, Env & env, Value & v)
 {
     Value v1;

src/libexpr/include/nix/expr/gc-small-vector.hh

@@ -26,4 +26,20 @@ using SmallValueVector = SmallVector<Value *, nItems>;
 template<size_t nItems>
 using SmallTemporaryValueVector = SmallVector<Value, nItems>;
 
+/**
+ * For functions where we do not expect deep recursion, we can use a sizable
+ * part of the stack a free allocation space.
+ *
+ * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
+ */
+constexpr size_t nonRecursiveStackReservation = 128;
+
+/**
+ * Functions that maybe applied to self-similar inputs, such as concatMap on a
+ * tree, should reserve a smaller part of the stack for allocation.
+ *
+ * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
+ */
+constexpr size_t conservativeStackReservation = 16;
+
 } // namespace nix

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

@@ -4,6 +4,7 @@
 #include <map>
 #include <vector>
 
+#include "nix/expr/gc-small-vector.hh"
 #include "nix/expr/value.hh"
 #include "nix/expr/symbol-table.hh"
 #include "nix/expr/eval-error.hh"
@@ -80,6 +81,8 @@ typedef std::vector<AttrName> AttrPath;
 
 std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath);
 
+using UpdateQueue = SmallTemporaryValueVector<conservativeStackReservation>;
+
 /* Abstract syntax of Nix expressions. */
 
 struct Expr
@@ -110,6 +113,14 @@ struct Expr
      * of thunks allocated.
      */
     virtual Value * maybeThunk(EvalState & state, Env & env);
+
+    /**
+     * Only called when performing an attrset update: `//` or similar.
+     * Instead of writing to a Value &, this function writes to an UpdateQueue.
+     * This allows the expression to perform multiple updates in a delayed manner, gathering up all the updates before
+     * applying them.
+     */
+    virtual void evalForUpdate(EvalState & state, Env & env, UpdateQueue & q, std::string_view errorCtx);
     virtual void setName(Symbol name);
     virtual void setDocComment(DocComment docComment) {};
 
@@ -574,9 +585,7 @@ struct ExprOpNot : Expr
     COMMON_METHODS
 };
 
-#define MakeBinOp(name, s)                                                                   \
-    struct name : Expr                                                                       \
-    {                                                                                        \
+#define MakeBinOpMembers(name, s)                                                        \
     PosIdx pos;                                                                          \
     Expr *e1, *e2;                                                                       \
     name(Expr * e1, Expr * e2)                                                           \
@@ -603,7 +612,12 @@ struct ExprOpNot : Expr
     PosIdx getPos() const override                                                       \
     {                                                                                    \
         return pos;                                                                      \
-        }                                                                                    \
+    }
+
+#define MakeBinOp(name, s)        \
+    struct name : Expr            \
+    {                             \
+        MakeBinOpMembers(name, s) \
     }
 
 MakeBinOp(ExprOpEq, "==");
@@ -611,9 +625,20 @@ MakeBinOp(ExprOpNEq, "!=");
 MakeBinOp(ExprOpAnd, "&&");
 MakeBinOp(ExprOpOr, "||");
 MakeBinOp(ExprOpImpl, "->");
-MakeBinOp(ExprOpUpdate, "//");
 MakeBinOp(ExprOpConcatLists, "++");
 
+struct ExprOpUpdate : Expr
+{
+private:
+    /** Special case for merging of two attrsets. */
+    void eval(EvalState & state, Value & v, Value & v1, Value & v2);
+    void evalForUpdate(EvalState & state, Env & env, UpdateQueue & q);
+
+public:
+    MakeBinOpMembers(ExprOpUpdate, "//");
+    virtual void evalForUpdate(EvalState & state, Env & env, UpdateQueue & q, std::string_view errorCtx) override;
+};
+
 struct ExprConcatStrings : Expr
 {
     PosIdx pos;

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

@@ -8,22 +8,6 @@
 
 namespace nix {
 
-/**
- * For functions where we do not expect deep recursion, we can use a sizable
- * part of the stack a free allocation space.
- *
- * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
- */
-constexpr size_t nonRecursiveStackReservation = 128;
-
-/**
- * Functions that maybe applied to self-similar inputs, such as concatMap on a
- * tree, should reserve a smaller part of the stack for allocation.
- *
- * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
- */
-constexpr size_t conservativeStackReservation = 16;
-
 struct RegisterPrimOp
 {
     typedef std::vector<PrimOp> PrimOps;

tests/functional/lang/eval-fail-recursion.err.exp

@@ -1,6 +1,6 @@
 error:
        … in the right operand of the update (//) operator
-         at /pwd/lang/eval-fail-recursion.nix:2:11:
+         at /pwd/lang/eval-fail-recursion.nix:2:14:
             1| let
             2|   a = { } // a;
              |              ^