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/gc-small-vector.hh b/src/libexpr/include/nix/expr/gc-small-vector.hh index fdd80b2c7..95c028e5a 100644 --- a/src/libexpr/include/nix/expr/gc-small-vector.hh +++ b/src/libexpr/include/nix/expr/gc-small-vector.hh @@ -26,4 +26,20 @@ using SmallValueVector = SmallVector; template using SmallTemporaryValueVector = SmallVector; +/** + * 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 diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index aa62760d8..7721918c3 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -4,6 +4,7 @@ #include #include +#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 AttrPath; std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath); +using UpdateQueue = SmallTemporaryValueVector; + /* 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,36 +585,39 @@ struct ExprOpNot : Expr COMMON_METHODS }; -#define MakeBinOp(name, s) \ - struct name : Expr \ - { \ - PosIdx pos; \ - Expr *e1, *e2; \ - name(Expr * e1, Expr * e2) \ - : e1(e1) \ - , e2(e2) {}; \ - name(const PosIdx & pos, Expr * e1, Expr * e2) \ - : pos(pos) \ - , e1(e1) \ - , e2(e2) {}; \ - void show(const SymbolTable & symbols, std::ostream & str) const override \ - { \ - str << "("; \ - e1->show(symbols, str); \ - str << " " s " "; \ - e2->show(symbols, str); \ - str << ")"; \ - } \ - void bindVars(EvalState & es, const std::shared_ptr & env) override \ - { \ - e1->bindVars(es, env); \ - e2->bindVars(es, env); \ - } \ - void eval(EvalState & state, Env & env, Value & v) override; \ - PosIdx getPos() const override \ - { \ - return pos; \ - } \ +#define MakeBinOpMembers(name, s) \ + PosIdx pos; \ + Expr *e1, *e2; \ + name(Expr * e1, Expr * e2) \ + : e1(e1) \ + , e2(e2){}; \ + name(const PosIdx & pos, Expr * e1, Expr * e2) \ + : pos(pos) \ + , e1(e1) \ + , e2(e2){}; \ + void show(const SymbolTable & symbols, std::ostream & str) const override \ + { \ + str << "("; \ + e1->show(symbols, str); \ + str << " " s " "; \ + e2->show(symbols, str); \ + str << ")"; \ + } \ + void bindVars(EvalState & es, const std::shared_ptr & env) override \ + { \ + e1->bindVars(es, env); \ + e2->bindVars(es, env); \ + } \ + void eval(EvalState & state, Env & env, Value & v) override; \ + 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; diff --git a/src/libexpr/include/nix/expr/primops.hh b/src/libexpr/include/nix/expr/primops.hh index 885a53e9a..6407ba84e 100644 --- a/src/libexpr/include/nix/expr/primops.hh +++ b/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 PrimOps; 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