From 97ce7759d07fc44967e7fb3030fe9cbb8ebc2c92 Mon Sep 17 00:00:00 2001
From: Sergei Zimmerman <sergei@zimmerman.foo>
Date: Wed, 24 Sep 2025 21:47:59 +0300
Subject: [PATCH] libexpr: Use same naive iterative merging but with
 `evalForUpdate`

---
 src/libexpr/eval.cc                           | 38 ++++++++++++++++---
 src/libexpr/include/nix/expr/nixexpr.hh       | 22 ++++++++++-
 .../lang/eval-fail-recursion.err.exp          |  4 +-
 3 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 18212940e..87b1e73a5 100644
--- a/src/libexpr/eval.cc
+++ b/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;
diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh
index e04e4f23c..7721918c3 100644
--- a/src/libexpr/include/nix/expr/nixexpr.hh
+++ b/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) {};
 
@@ -607,7 +618,7 @@ struct ExprOpNot : Expr
     struct name : Expr            \
     {                             \
         MakeBinOpMembers(name, s) \
-    };
+    }
 
 MakeBinOp(ExprOpEq, "==");
 MakeBinOp(ExprOpNEq, "!=");
@@ -618,7 +629,14 @@ MakeBinOp(ExprOpConcatLists, "++");
 
 struct ExprOpUpdate : Expr
 {
-    MakeBinOpMembers(ExprOpUpdate, "//")
+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
diff --git a/tests/functional/lang/eval-fail-recursion.err.exp b/tests/functional/lang/eval-fail-recursion.err.exp
index 8bfb4e12e..ee41ff46b 100644
--- a/tests/functional/lang/eval-fail-recursion.err.exp
+++ b/tests/functional/lang/eval-fail-recursion.err.exp
@@ -1,9 +1,9 @@
 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;
-             |           ^
+             |              ^
             3| in
 
        error: infinite recursion encountered