Skip to content

Commit

Permalink
Add a ListBuilder helper for constructing list values
Browse files Browse the repository at this point in the history
Previously, `state.mkList()` would set the type of the value to tList
and allocate the list vector, but it would not initialize the values
in the list. This has two problems:

* If an exception occurs, the list is left in an undefined state.

* More importantly, for multithreaded evaluation, if a value
  transitions from thunk to non-thunk, it should be final (i.e. other
  threads should be able to access the value safely).

To address this, there now is a `ListBuilder` class (analogous to
`BindingsBuilder`) to build the list vector prior to the call to
`Value::mkList()`. Typical usage:

   auto list = state.buildList(size);
   for (auto & v : list)
       v = ... set value ...;
   vRes.mkList(list);
  • Loading branch information
edolstra committed Mar 15, 2024
1 parent bff5c94 commit fecff52
Show file tree
Hide file tree
Showing 10 changed files with 227 additions and 156 deletions.
24 changes: 13 additions & 11 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,8 @@ EvalState::EvalState(

static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes");

vEmptyList.mkList(0);
vEmptyList.mkList(buildList(0));
vNull.mkNull();

/* Initialise the Nix expression search path. */
if (!evalSettings.pureEval) {
Expand Down Expand Up @@ -923,12 +924,11 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval)
}
}

void EvalState::mkList(Value & v, size_t size)
ListBuilder::ListBuilder(EvalState & state, size_t size)
: size(size)
, elems(size <= 2 ? inlineElems : (Value * *) allocBytes(size * sizeof(Value *)))
{
v.mkList(size);
if (size > 2)
v.bigList.elems = (Value * *) allocBytes(size * sizeof(Value *));
nrListElems += size;
state.nrListElems += size;
}


Expand Down Expand Up @@ -1353,9 +1353,10 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)

void ExprList::eval(EvalState & state, Env & env, Value & v)
{
state.mkList(v, elems.size());
for (auto [n, v2] : enumerate(v.listItems()))
const_cast<Value * &>(v2) = elems[n]->maybeThunk(state, env);
auto list = state.buildList(elems.size());
for (const auto & [n, v2] : enumerate(list))
v2 = elems[n]->maybeThunk(state, env);
v.mkList(list);
}


Expand Down Expand Up @@ -1963,14 +1964,15 @@ void EvalState::concatLists(Value & v, size_t nrLists, Value * * lists, const Po
return;
}

mkList(v, len);
auto out = v.listElems();
auto list = buildList(len);
auto out = list.elems;
for (size_t n = 0, pos = 0; n < nrLists; ++n) {
auto l = lists[n]->listSize();
if (l)
memcpy(out + pos, lists[n]->listElems(), l * sizeof(Value *));
pos += l;
}
v.mkList(list);
}


Expand Down
12 changes: 11 additions & 1 deletion src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ public:
*/
Value vEmptyList;

/**
* Null constant.
*/
Value vNull;

/**
* The accessor for the root filesystem.
*/
Expand Down Expand Up @@ -615,7 +620,11 @@ public:
return BindingsBuilder(*this, allocBindings(capacity));
}

void mkList(Value & v, size_t length);
ListBuilder buildList(size_t size)
{
return ListBuilder(*this, size);
}

void mkThunk_(Value & v, Expr * expr);
void mkPos(Value & v, PosIdx pos);

Expand Down Expand Up @@ -756,6 +765,7 @@ private:
friend void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v);

friend struct Value;
friend class ListBuilder;
};

struct DebugTraceStacker {
Expand Down
9 changes: 4 additions & 5 deletions src/libexpr/json-to-value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,10 @@ class JSONSax : nlohmann::json_sax<json> {
ValueVector values;
std::unique_ptr<JSONState> resolve(EvalState & state) override
{
Value & v = parent->value(state);
state.mkList(v, values.size());
for (size_t n = 0; n < values.size(); ++n) {
v.listElems()[n] = values[n];
}
auto list = state.buildList(values.size());
for (const auto & [n, v2] : enumerate(list))
v2 = values[n];
parent->value(state).mkList(list);
return std::move(parent);
}
void add() override {
Expand Down
Loading

0 comments on commit fecff52

Please sign in to comment.