libexpr: heap-allocate thunk control state

Change-Id: I20d8ab1d6f683c0a2f3b77edf9bdad147d62c8fa

lix/libexpr/eval-inline.hh

@@ -26,6 +26,15 @@ inline Value::Value(app_t, EvalMemory & mem, Value & lhs, std::span<Value *> arg
     }
 }
 
+inline Value::Value(thunk_t, EvalMemory & mem, Env & env, Expr & expr)
+    : internalType(tThunk)
+    , _thunk_pad(0)
+{
+    auto thunk = mem.allocType<Thunk>();
+    *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;
         }

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));
         }

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()

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()

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,