From aa39e14fcf36949dd0cbac4d19e499edcf515ae4 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 28 Sep 2025 00:02:21 +0200 Subject: [PATCH] libexpr: heap-allocate app nodes Change-Id: I9a39dcf0be7589cedf494757e21665c5d50e446b --- lix/legacy/nix-env.cc | 6 ++-- lix/legacy/user-env.cc | 3 +- lix/libexpr/eval-inline.hh | 26 ++++++++++------- lix/libexpr/eval.cc | 3 +- lix/libexpr/primops.cc | 25 +++++++++------- lix/libexpr/value.hh | 48 +++++++++---------------------- tests/unit/libexpr/value/print.cc | 12 ++++---- 7 files changed, 56 insertions(+), 67 deletions(-) diff --git a/lix/legacy/nix-env.cc b/lix/legacy/nix-env.cc index 36ae06776..f59159f05 100644 --- a/lix/legacy/nix-env.cc +++ b/lix/legacy/nix-env.cc @@ -1,6 +1,7 @@ #include "lix/libcmd/cmd-profiles.hh" #include "lix/libexpr/attr-path.hh" #include "lix/libcmd/common-eval-args.hh" +#include "lix/libexpr/value.hh" #include "lix/libstore/derivations.hh" #include "lix/libutil/terminal.hh" #include "lix/libexpr/eval.hh" @@ -154,7 +155,8 @@ static void getAllExprs(Evaluator & state, vArg->mkString(path2.canonical().abs()); if (seen.size() == maxAttrs) throw Error("too many Nix expressions in directory '%1%'", path); - attrs.alloc(attrName).mkApp(&state.builtins.get("import"), vArg); + attrs.alloc(attrName + ) = {NewValueAs::app, state.mem, state.builtins.get("import"), *vArg}; } else if (st.type == InputAccessor::tDirectory) /* `path2' is a directory (with no default.nix in it); @@ -425,7 +427,7 @@ static void queryInstSources(EvalState & state, Expr & eFun = state.ctx.parseExprFromString(i, CanonPath::fromCwd()); Value vFun, vTmp; state.eval(eFun, vFun); - vTmp.mkApp(&vFun, &vArg); + vTmp = {NewValueAs::app, state.ctx.mem, vFun, vArg}; getDerivations(state, vTmp, "", *instSource.autoArgs, elems, true); } diff --git a/lix/legacy/user-env.cc b/lix/legacy/user-env.cc index 326e13578..5a8d84350 100644 --- a/lix/legacy/user-env.cc +++ b/lix/legacy/user-env.cc @@ -110,8 +110,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, Value args; args.mkAttrs(attrs); - Value topLevel; - topLevel.mkApp(&envBuilder, &args); + Value topLevel{NewValueAs::app, state.ctx.mem, envBuilder, args}; /* Evaluate it. */ debug("evaluating user environment builder"); diff --git a/lix/libexpr/eval-inline.hh b/lix/libexpr/eval-inline.hh index f8627e9ba..c5a0fcdd9 100644 --- a/lix/libexpr/eval-inline.hh +++ b/lix/libexpr/eval-inline.hh @@ -10,20 +10,24 @@ namespace nix { +inline Value::Value(app_t, EvalMemory & mem, Value & lhs, Value & rhs) + : internalType(tApp) + , _app_pad(0) +{ + _app = static_cast(mem.allocBytes(sizeof(Value::App) + sizeof(Value *))); + _app->_left = &lhs; + _app->_n = 1; + _app->_args[0] = &rhs; +} + inline Value::Value(app_t, EvalMemory & mem, Value & lhs, std::span args) : internalType(tApp) + , _app_pad(0) { - if (args.size() == 1) { - _app._left = reinterpret_cast(&lhs); - _app._right = args[0]; - } else { - auto app = - static_cast(mem.allocBytes(sizeof(Value::AppN) + args.size_bytes())); - app->nargs = args.size(); - memcpy(app->args, args.data(), args.size_bytes()); - _app._left = reinterpret_cast(&lhs) | 1; - _app._appn = app; - } + _app = static_cast(mem.allocBytes(sizeof(Value::App) + args.size_bytes())); + _app->_left = &lhs; + _app->_n = args.size(); + memcpy(_app->_args, args.data(), args.size_bytes()); } inline Value::Value(thunk_t, EvalMemory & mem, Env & env, Expr & expr) diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 95a832433..6fd971ac0 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -587,8 +587,7 @@ Value * EvalBuiltins::addPrimOp(PrimOpDetails && primOp) primOp.arity = 1; auto vPrimOp = mem.allocValue(); vPrimOp->mkPrimOp(new PrimOp(std::move(primOp))); - Value v; - v.mkApp(vPrimOp, vPrimOp); + Value v{NewValueAs::app, mem, *vPrimOp, *vPrimOp}; return addConstant( vPrimOp->primOp()->name, v, diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index 0a29b5961..fc1aed5ba 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -212,7 +212,7 @@ static void import(EvalState & state, Value & vPath, Value * vScope, Value & v) noPos, "while evaluating imported-drv-to-derivation.nix.gen.hh" ); - v.mkApp(*state.ctx.caches.vImportedDrvToDerivation, w); + v = {NewValueAs::app, state.ctx.mem, **state.ctx.caches.vImportedDrvToDerivation, *w}; state.forceAttrs(v, noPos, "while calling imported-drv-to-derivation.nix.gen.hh"); } @@ -1405,7 +1405,7 @@ static void prim_readDir(EvalState & state, Value * * args, Value & v) epath->mkPath(path + name); if (!readFileType) readFileType = &state.ctx.builtins.get("readFileType"); - attr.mkApp(readFileType, epath); + attr = {NewValueAs::app, state.ctx.mem, *readFileType, *epath}; } else { // This branch of the conditional is much more likely. // Here we just stringize the directory entry type. @@ -1748,8 +1748,8 @@ static struct LazyPosAcessors { { Value * posV = state.ctx.mem.allocValue(); posV->mkInt(pos.id); - line.mkApp(&lineOfPos, posV); - column.mkApp(&columnOfPos, posV); + line = {NewValueAs::app, state.ctx.mem, lineOfPos, *posV}; + column = {NewValueAs::app, state.ctx.mem, columnOfPos, *posV}; } } makeLazyPosAccessors; @@ -2079,8 +2079,10 @@ static void prim_map(EvalState & state, Value * * args, Value & v) auto result = state.ctx.mem.newList(args[1]->listSize()); v = {NewValueAs::list, result}; - for (unsigned int n = 0; n < v.listSize(); ++n) - (result->elems[n] = state.ctx.mem.allocValue())->mkApp(args[0], args[1]->listElems()[n]); + for (unsigned int n = 0; n < v.listSize(); ++n) { + result->elems[n] = state.ctx.mem.allocValue(); + *result->elems[n] = {NewValueAs::app, state.ctx.mem, *args[0], *args[1]->listElems()[n]}; + } } /* Filter a list using a predicate; that is, return a list containing @@ -2224,7 +2226,8 @@ static void prim_genList(EvalState & state, Value * * args, Value & v) for (size_t n = 0; n < len; ++n) { auto arg = state.ctx.mem.allocValue(); arg->mkInt(n); - (result->elems[n] = state.ctx.mem.allocValue())->mkApp(args[0], arg); + result->elems[n] = state.ctx.mem.allocValue(); + *result->elems[n] = {NewValueAs::app, state.ctx.mem, *args[0], *arg}; } } @@ -2898,9 +2901,11 @@ void EvalBuiltins::createBaseEnv(const SearchPath & searchPath, const Path & sto Null docs because it is documented separately. App instead of PrimopApp to have eval immediately force it when accessed. */ - addConstant("derivation", {NewValueAs::app, initializeDerivation, initializeDerivation}, { - .type = nFunction, - }); + addConstant( + "derivation", + {NewValueAs::app, mem, initializeDerivation, initializeDerivation}, + {.type = nFunction} + ); /* Now that we've added all primops, sort the `builtins' set, because attribute lookups expect it to be sorted. */ diff --git a/lix/libexpr/value.hh b/lix/libexpr/value.hh index 99e87114d..cfd9525da 100644 --- a/lix/libexpr/value.hh +++ b/lix/libexpr/value.hh @@ -490,11 +490,7 @@ public: /// Constructs a nix language value of type "lambda", which represents a /// lazy and/or partial application of a function. - Value(app_t, Value & lhs, Value & rhs) - : internalType(tApp) - , _app{._left = reinterpret_cast(&lhs), ._right = &rhs} - { - } + Value(app_t, EvalMemory & mem, Value & lhs, Value & rhs); /// Constructs a nix language value of type "lambda", which represents a /// lazy and/or partial application of a function. @@ -633,29 +629,15 @@ public: } }; - struct AppN - { - size_t nargs; - Value * args[0]; - - std::span argsSpan() - { - return {args, nargs}; - } - }; - struct App { - uintptr_t _left; - union - { - Value * _right; - AppN * _appn; - }; + Value * _left; + size_t _n; + Value * _args[0]; Value * left() const { - return reinterpret_cast(_left & ~uintptr_t(1)); + return _left; } Value * target() const @@ -665,13 +647,12 @@ public: std::span args() { - return _left & 1 ? _appn->argsSpan() : std::span{&_right, 1}; + return std::span{_args, _n}; } size_t totalArgs() const { - return (_left & 1 ? _appn->nargs : 1) - + (left()->isApp() ? left()->app().totalArgs() : 0); + return _n + (left()->isApp() ? left()->app().totalArgs() : 0); } }; @@ -741,7 +722,11 @@ public: Thunk * _thunk; uintptr_t _thunk_pad; }; - App _app; + struct + { + App * _app; + uintptr_t _app_pad; + }; struct { const Acb * _auxiliary; uintptr_t _aux_pad; @@ -855,11 +840,6 @@ public: Value & mkAttrs(BindingsBuilder & bindings); - inline void mkApp(Value * l, Value * r) - { - *this = {NewValueAs::app, *l, *r}; - } - void mkPrimOp(PrimOp * p); inline void mkExternal(ExternalValueBase * e) @@ -961,12 +941,12 @@ public: App & app() { - return _app; + return *_app; } const App & app() const { - return _app; + return *_app; } const auto & lambda() const diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index 4e5661b0a..01dd554f5 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -102,9 +102,9 @@ TEST_F(ValuePrintingTests, vThunk) TEST_F(ValuePrintingTests, vApp) { - Value vApp; + EvalMemory mem; Value vFn{NewValueAs::null}; - vApp.mkApp(&vFn, &vFn); + Value vApp{NewValueAs::app, mem, vFn, vFn}; test(vApp, "«thunk»"); } @@ -142,12 +142,12 @@ TEST_F(ValuePrintingTests, vPrimOp) TEST_F(ValuePrintingTests, vPrimOpApp) { + EvalMemory mem; PrimOp primOp{{.name = "puppy"}}; Value vPrimOp; vPrimOp.mkPrimOp(&primOp); - Value vPrimOpApp; - vPrimOpApp.mkApp(&vPrimOp, &vPrimOp); + Value vPrimOpApp{NewValueAs::app, mem, vPrimOp, vPrimOp}; test(vPrimOpApp, "«partially applied primop puppy»"); } @@ -591,12 +591,12 @@ TEST_F(ValuePrintingTests, ansiColorsPrimOp) TEST_F(ValuePrintingTests, ansiColorsPrimOpApp) { + EvalMemory mem; PrimOp primOp{{.name = "puppy"}}; Value vPrimOp; vPrimOp.mkPrimOp(&primOp); - Value v; - v.mkApp(&vPrimOp, &vPrimOp); + Value v{NewValueAs::app, mem, vPrimOp, vPrimOp}; test(v, ANSI_BLUE "«partially applied primop puppy»" ANSI_NORMAL,