From b0d11f9da199623d136fc919522a3841d080da00 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 28 Sep 2025 00:02:20 +0200 Subject: [PATCH] libexpr: heap-allocate thunk control state Change-Id: I20d8ab1d6f683c0a2f3b77edf9bdad147d62c8fa --- lix/libexpr/eval-inline.hh | 14 ++++++++-- lix/libexpr/eval.cc | 17 ++++++++---- lix/libexpr/nixexpr.cc | 3 +- lix/libexpr/value.hh | 46 +++++++++++-------------------- tests/unit/libexpr/value/print.cc | 16 +++++------ 5 files changed, 50 insertions(+), 46 deletions(-) diff --git a/lix/libexpr/eval-inline.hh b/lix/libexpr/eval-inline.hh index b095d3d93..f8627e9ba 100644 --- a/lix/libexpr/eval-inline.hh +++ b/lix/libexpr/eval-inline.hh @@ -26,6 +26,15 @@ inline Value::Value(app_t, EvalMemory & mem, Value & lhs, std::span arg } } +inline Value::Value(thunk_t, EvalMemory & mem, Env & env, Expr & expr) + : internalType(tThunk) + , _thunk_pad(0) +{ + auto thunk = mem.allocType(); + *thunk = {.env = &env, .expr = &expr}; + _thunk = thunk; +} + inline Value::Value(lambda_t, EvalMemory & mem, Env & env, ExprLambda & lambda) : internalType(tAuxiliary) , _aux_pad(0) @@ -105,13 +114,14 @@ Env & EvalMemory::allocEnv(size_t size) void EvalState::forceValue(Value & v, const PosIdx pos) { if (v.isThunk()) { + const auto backup = v; Env * env = v.thunk().env; Expr & expr = *v.thunk().expr; + v = Value{NewValueAs::blackhole}; try { - v.mkBlackhole(); expr.eval(*this, *env, v); } catch (...) { - v.mkThunk(env, expr); + v = backup; tryFixupBlackHolePos(v, pos); throw; } diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index bd70e033b..95a832433 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -879,7 +879,7 @@ Value::List * EvalMemory::newList(size_t size) void Evaluator::evalLazily(Expr & e, Value & v) { - v.mkThunk(&builtins.env, e); + v = {NewValueAs::thunk, mem, builtins.env, e}; stats.nrThunks++; } @@ -961,7 +961,7 @@ void EvalState::mkSingleDerivedPathString( Value * Expr::maybeThunk(EvalState & state, Env & env) { Value * v = state.ctx.mem.allocValue(); - v->mkThunk(&env, *this); + *v = {NewValueAs::thunk, state.ctx.mem, env, *this}; state.ctx.stats.nrThunks++; return v; } @@ -1121,10 +1121,17 @@ void ExprSet::eval(EvalState & state, Env & env, Value & v) Value * vAttr; if (hasOverrides && i.second.kind != ExprAttrs::AttrDef::Kind::Inherited) { vAttr = state.ctx.mem.allocValue(); - vAttr->mkThunk(i.second.chooseByKind(&env2, &env, inheritEnv), *i.second.e); + *vAttr = { + NewValueAs::thunk, + state.ctx.mem, + *i.second.chooseByKind(&env2, &env, inheritEnv), + *i.second.e + }; state.ctx.stats.nrThunks++; - } else - vAttr = i.second.e->maybeThunk(state, *i.second.chooseByKind(&env2, &env, inheritEnv)); + } else { + vAttr = + i.second.e->maybeThunk(state, *i.second.chooseByKind(&env2, &env, inheritEnv)); + } env2.values[displ++] = vAttr; v.attrs()->push_back(Attr(i.first, vAttr, i.second.pos)); } diff --git a/lix/libexpr/nixexpr.cc b/lix/libexpr/nixexpr.cc index 8e96e8803..dc5d234e2 100644 --- a/lix/libexpr/nixexpr.cc +++ b/lix/libexpr/nixexpr.cc @@ -9,7 +9,8 @@ namespace nix { ExprBlackHole eBlackHole; -Expr *eBlackHoleAddr = &eBlackHole; + +Value::Thunk Value::blackHole{nullptr, &eBlackHole}; // FIXME: remove, because *symbols* are abstract and do not have a single // textual representation; see printIdentifier() diff --git a/lix/libexpr/value.hh b/lix/libexpr/value.hh index b7e672172..99e87114d 100644 --- a/lix/libexpr/value.hh +++ b/lix/libexpr/value.hh @@ -192,10 +192,6 @@ class ExternalValueBase std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); -/** This is just the address of eBlackHole. It exists because eBlackHole has an - * incomplete type at usage sites so is not possible to cast. */ -extern Expr *eBlackHoleAddr; - struct NewValueAs { struct integer_t { }; @@ -259,9 +255,13 @@ public: struct Acb; struct Null; struct Lambda; + struct Thunk; static const Null NULL_ACB; + /** Single, unforceable black hole thunk control block. */ + static Thunk blackHole; + // Discount `using NewValueAs::*;` // NOLINTNEXTLINE(bugprone-macro-parentheses) #define USING_VALUETYPE(name) using name = NewValueAs::name @@ -481,10 +481,7 @@ public: /// /// The thunk stores the environment it will be computed in @ref env, and /// the expression that will need to be evaluated @ref expr. - Value(thunk_t, Env & env, Expr & expr) - : internalType(tThunk) - , _thunk({ .env = &env, .expr = &expr }) - { } + Value(thunk_t, EvalMemory & mem, Env & env, Expr & expr); /// Constructs a nix language value of type "lambda", which represents /// a builtin, primitive operation ("primop"), from the primop @@ -524,10 +521,7 @@ public: Value(lambda_t, EvalMemory & mem, Env & env, ExprLambda & lambda); /// Constructs an evil thunk, whose evaluation represents infinite recursion. - explicit Value(blackhole_t) - : internalType(tThunk) - , _thunk({ .env = nullptr, .expr = eBlackHoleAddr }) - { } + explicit Value(blackhole_t) : internalType(tThunk), _thunk(&blackHole), _thunk_pad(0) {} explicit Value(const Acb & backing) : internalType(tAuxiliary) @@ -575,7 +569,7 @@ public: inline bool isApp() const { return internalType == tApp; }; inline bool isBlackhole() const { - return internalType == tThunk && _thunk.expr == eBlackHoleAddr; + return internalType == tThunk && _thunk == &blackHole; } // type() == nFunction @@ -712,6 +706,11 @@ public: Env * env; ExprLambda * fun; }; + struct Thunk + { + Env * env; + Expr * expr; + }; union { @@ -739,9 +738,9 @@ public: uintptr_t _list_pad; }; struct { - Env * env; - Expr * expr; - } _thunk; + Thunk * _thunk; + uintptr_t _thunk_pad; + }; App _app; struct { const Acb * _auxiliary; @@ -856,24 +855,11 @@ public: Value & mkAttrs(BindingsBuilder & bindings); - inline void mkThunk(Env * e, Expr & ex) - { - internalType = tThunk; - _thunk.env = e; - _thunk.expr = &ex; - } - inline void mkApp(Value * l, Value * r) { *this = {NewValueAs::app, *l, *r}; } - inline void mkBlackhole() - { - internalType = tThunk; - _thunk.expr = eBlackHoleAddr; - } - void mkPrimOp(PrimOp * p); inline void mkExternal(ExternalValueBase * e) @@ -970,7 +956,7 @@ public: const auto & thunk() const { - return _thunk; + return *_thunk; } App & app() diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index 4bb4f26d1..4e5661b0a 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -92,9 +92,10 @@ TEST_F(ValuePrintingTests, tList) TEST_F(ValuePrintingTests, vThunk) { - Value vThunk; + EvalMemory mem; + Env env; ExprLiteral e(noPos, NewValueAs::integer, 0); - vThunk.mkThunk(nullptr, e); + Value vThunk{NewValueAs::thunk, mem, env, e}; test(vThunk, "«thunk»"); } @@ -186,8 +187,7 @@ TEST_F(ValuePrintingTests, vFloat) TEST_F(ValuePrintingTests, vBlackhole) { - Value vBlackhole; - vBlackhole.mkBlackhole(); + Value vBlackhole{NewValueAs::blackhole}; test(vBlackhole, "«potential infinite recursion»"); } @@ -607,9 +607,10 @@ TEST_F(ValuePrintingTests, ansiColorsPrimOpApp) TEST_F(ValuePrintingTests, ansiColorsThunk) { - Value v; + EvalMemory mem; + Env env; ExprLiteral e(noPos, NewValueAs::integer, 0); - v.mkThunk(nullptr, e); + Value v{NewValueAs::thunk, mem, env, e}; test(v, ANSI_MAGENTA "«thunk»" ANSI_NORMAL, @@ -620,8 +621,7 @@ TEST_F(ValuePrintingTests, ansiColorsThunk) TEST_F(ValuePrintingTests, ansiColorsBlackhole) { - Value v; - v.mkBlackhole(); + Value v{NewValueAs::blackhole}; test(v, ANSI_RED "«potential infinite recursion»" ANSI_NORMAL,