From fecff520d7ce6598319862efc50c2dc6e1f6e9d9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 14 Mar 2024 19:10:31 +0100 Subject: [PATCH] Add a ListBuilder helper for constructing list values 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); --- src/libexpr/eval.cc | 24 ++-- src/libexpr/eval.hh | 12 +- src/libexpr/json-to-value.cc | 9 +- src/libexpr/primops.cc | 202 ++++++++++++++++-------------- src/libexpr/primops/context.cc | 6 +- src/libexpr/primops/fromTOML.cc | 8 +- src/libexpr/value.hh | 43 ++++++- src/nix-env/nix-env.cc | 2 +- src/nix-env/user-env.cc | 17 +-- tests/unit/libexpr/value/print.cc | 62 +++++---- 10 files changed, 228 insertions(+), 157 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index bbccfcd29..297832818 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -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) { @@ -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; } @@ -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(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); } @@ -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); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 368bb17b3..4a271f4ef 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -186,6 +186,11 @@ public: */ Value vEmptyList; + /** + * Null constant. + */ + Value vNull; + /** * The accessor for the root filesystem. */ @@ -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); @@ -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 { diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 2d12c47c5..20bee193f 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -57,11 +57,10 @@ class JSONSax : nlohmann::json_sax { ValueVector values; std::unique_ptr 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 { diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index bc2a70496..32913d72e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -187,13 +187,13 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v NixStringContextElem::DrvDeep { .drvPath = *storePath }, }); attrs.alloc(state.sName).mkString(drv.env["name"]); - auto & outputsVal = attrs.alloc(state.sOutputs); - state.mkList(outputsVal, drv.outputs.size()); + auto list = state.buildList(drv.outputs.size()); for (const auto & [i, o] : enumerate(drv.outputs)) { mkOutputString(state, attrs, *storePath, o); - (outputsVal.listElems()[i] = state.allocValue())->mkString(o.first); + (list[i] = state.allocValue())->mkString(o.first); } + attrs.alloc(state.sOutputs).mkList(list); auto w = state.allocValue(); w->mkAttrs(attrs); @@ -694,10 +694,10 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a } /* Create the result list. */ - state.mkList(v, res.size()); - unsigned int n = 0; - for (auto & i : res) - v.listElems()[n++] = i; + auto list = state.buildList(res.size()); + for (const auto & [n, i] : enumerate(res)) + list[n] = i; + v.mkList(list); } static RegisterPrimOp primop_genericClosure(PrimOp { @@ -2423,14 +2423,15 @@ static void prim_attrNames(EvalState & state, const PosIdx pos, Value * * args, { state.forceAttrs(*args[0], pos, "while evaluating the argument passed to builtins.attrNames"); - state.mkList(v, args[0]->attrs->size()); + auto list = state.buildList(args[0]->attrs->size()); - size_t n = 0; - for (auto & i : *args[0]->attrs) - (v.listElems()[n++] = state.allocValue())->mkString(state.symbols[i.name]); + for (const auto & [n, i] : enumerate(*args[0]->attrs)) + (list[n] = state.allocValue())->mkString(state.symbols[i.name]); - std::sort(v.listElems(), v.listElems() + n, + std::sort(list.begin(), list.end(), [](Value * v1, Value * v2) { return strcmp(v1->c_str(), v2->c_str()) < 0; }); + + v.mkList(list); } static RegisterPrimOp primop_attrNames({ @@ -2450,21 +2451,22 @@ static void prim_attrValues(EvalState & state, const PosIdx pos, Value * * args, { state.forceAttrs(*args[0], pos, "while evaluating the argument passed to builtins.attrValues"); - state.mkList(v, args[0]->attrs->size()); + auto list = state.buildList(args[0]->attrs->size()); - unsigned int n = 0; - for (auto & i : *args[0]->attrs) - v.listElems()[n++] = (Value *) &i; + for (const auto & [n, i] : enumerate(*args[0]->attrs)) + list[n] = (Value *) &i; - std::sort(v.listElems(), v.listElems() + n, + std::sort(list.begin(), list.end(), [&](Value * v1, Value * v2) { std::string_view s1 = state.symbols[((Attr *) v1)->name], s2 = state.symbols[((Attr *) v2)->name]; return s1 < s2; }); - for (unsigned int i = 0; i < n; ++i) - v.listElems()[i] = ((Attr *) v.listElems()[i])->value; + for (auto & v : list) + v = ((Attr *) v)->value; + + v.mkList(list); } static RegisterPrimOp primop_attrValues({ @@ -2805,9 +2807,10 @@ static void prim_catAttrs(EvalState & state, const PosIdx pos, Value * * args, V res[found++] = i->value; } - state.mkList(v, found); + auto list = state.buildList(found); for (unsigned int n = 0; n < found; ++n) - v.listElems()[n] = res[n]; + list[n] = res[n]; + v.mkList(list); } static RegisterPrimOp primop_catAttrs({ @@ -2908,43 +2911,50 @@ static void prim_zipAttrsWith(EvalState & state, const PosIdx pos, Value * * arg // attribute with the merge function application. this way we need not // use (slightly slower) temporary storage the GC does not know about. - std::map> attrsSeen; + struct Item + { + size_t size = 0; + size_t pos = 0; + std::optional list; + }; + + std::map attrsSeen; state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.zipAttrsWith"); state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.zipAttrsWith"); - const auto listSize = args[1]->listSize(); - const auto listElems = args[1]->listElems(); + const auto listItems = args[1]->listItems(); - for (unsigned int n = 0; n < listSize; ++n) { - Value * vElem = listElems[n]; + for (auto & vElem : listItems) { state.forceAttrs(*vElem, noPos, "while evaluating a value of the list passed as second argument to builtins.zipAttrsWith"); for (auto & attr : *vElem->attrs) - attrsSeen[attr.name].first++; + attrsSeen.try_emplace(attr.name).first->second.size++; + } + + for (auto & [sym, elem] : attrsSeen) + elem.list.emplace(state.buildList(elem.size)); + + for (auto & vElem : listItems) { + for (auto & attr : *vElem->attrs) { + auto & item = attrsSeen.at(attr.name); + (*item.list)[item.pos++] = attr.value; + } } auto attrs = state.buildBindings(attrsSeen.size()); + for (auto & [sym, elem] : attrsSeen) { - auto & list = attrs.alloc(sym); - state.mkList(list, elem.first); - elem.second = list.listElems(); - } - v.mkAttrs(attrs.alreadySorted()); - - for (unsigned int n = 0; n < listSize; ++n) { - Value * vElem = listElems[n]; - for (auto & attr : *vElem->attrs) - *attrsSeen[attr.name].second++ = attr.value; - } - - for (auto & attr : *v.attrs) { auto name = state.allocValue(); - name->mkString(state.symbols[attr.name]); + name->mkString(state.symbols[sym]); auto call1 = state.allocValue(); call1->mkApp(args[0], name); auto call2 = state.allocValue(); - call2->mkApp(call1, attr.value); - attr.value = call2; + auto arg = state.allocValue(); + arg->mkList(*elem.list); + call2->mkApp(call1, arg); + attrs.insert(sym, call2); } + + v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_zipAttrsWith({ @@ -3055,9 +3065,10 @@ static void prim_tail(EvalState & state, const PosIdx pos, Value * * args, Value if (args[0]->listSize() == 0) state.error("'tail' called on an empty list").atPos(pos).debugThrow(); - state.mkList(v, args[0]->listSize() - 1); - for (unsigned int n = 0; n < v.listSize(); ++n) - v.listElems()[n] = args[0]->listElems()[n + 1]; + auto list = state.buildList(args[0]->listSize() - 1); + for (const auto & [n, v] : enumerate(list)) + v = args[0]->listElems()[n + 1]; + v.mkList(list); } static RegisterPrimOp primop_tail({ @@ -3088,10 +3099,11 @@ static void prim_map(EvalState & state, const PosIdx pos, Value * * args, Value state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.map"); - state.mkList(v, args[1]->listSize()); - for (unsigned int n = 0; n < v.listSize(); ++n) - (v.listElems()[n] = state.allocValue())->mkApp( + auto list = state.buildList(args[1]->listSize()); + for (const auto & [n, v] : enumerate(list)) + (v = state.allocValue())->mkApp( args[0], args[1]->listElems()[n]); + v.mkList(list); } static RegisterPrimOp primop_map({ @@ -3140,8 +3152,9 @@ static void prim_filter(EvalState & state, const PosIdx pos, Value * * args, Val if (same) v = *args[1]; else { - state.mkList(v, k); - for (unsigned int n = 0; n < k; ++n) v.listElems()[n] = vs[n]; + auto list = state.buildList(k); + for (const auto & [n, v] : enumerate(list)) v = vs[n]; + v.mkList(list); } } @@ -3316,12 +3329,13 @@ static void prim_genList(EvalState & state, const PosIdx pos, Value * * args, Va // as evaluating map without accessing any values makes little sense. state.forceFunction(*args[0], noPos, "while evaluating the first argument passed to builtins.genList"); - state.mkList(v, len); - for (unsigned int n = 0; n < (unsigned int) len; ++n) { + auto list = state.buildList(len); + for (const auto & [n, v] : enumerate(list)) { auto arg = state.allocValue(); arg->mkInt(n); - (v.listElems()[n] = state.allocValue())->mkApp(args[0], arg); + (v = state.allocValue())->mkApp(args[0], arg); } + v.mkList(list); } static RegisterPrimOp primop_genList({ @@ -3355,11 +3369,10 @@ static void prim_sort(EvalState & state, const PosIdx pos, Value * * args, Value state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.sort"); - state.mkList(v, len); - for (unsigned int n = 0; n < len; ++n) { - state.forceValue(*args[1]->listElems()[n], pos); - v.listElems()[n] = args[1]->listElems()[n]; - } + auto list = state.buildList(len); + for (const auto & [n, v] : enumerate(list)) + state.forceValue(*(v = args[1]->listElems()[n]), pos); + v.mkList(list); auto comparator = [&](Value * a, Value * b) { /* Optimization: if the comparator is lessThan, bypass @@ -3424,17 +3437,17 @@ static void prim_partition(EvalState & state, const PosIdx pos, Value * * args, auto attrs = state.buildBindings(2); - auto & vRight = attrs.alloc(state.sRight); auto rsize = right.size(); - state.mkList(vRight, rsize); + auto rlist = state.buildList(rsize); if (rsize) - memcpy(vRight.listElems(), right.data(), sizeof(Value *) * rsize); + memcpy(rlist.elems, right.data(), sizeof(Value *) * rsize); + attrs.alloc(state.sRight).mkList(rlist); - auto & vWrong = attrs.alloc(state.sWrong); auto wsize = wrong.size(); - state.mkList(vWrong, wsize); + auto wlist = state.buildList(wsize); if (wsize) - memcpy(vWrong.listElems(), wrong.data(), sizeof(Value *) * wsize); + memcpy(wlist.elems, wrong.data(), sizeof(Value *) * wsize); + attrs.alloc(state.sWrong).mkList(wlist); v.mkAttrs(attrs); } @@ -3481,10 +3494,10 @@ static void prim_groupBy(EvalState & state, const PosIdx pos, Value * * args, Va auto attrs2 = state.buildBindings(attrs.size()); for (auto & i : attrs) { - auto & list = attrs2.alloc(i.first); auto size = i.second.size(); - state.mkList(list, size); - memcpy(list.listElems(), i.second.data(), sizeof(Value *) * size); + auto list = state.buildList(size); + memcpy(list.elems, i.second.data(), sizeof(Value *) * size); + attrs2.alloc(i.first).mkList(list); } v.mkAttrs(attrs2.alreadySorted()); @@ -3531,14 +3544,15 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args, len += lists[n].listSize(); } - state.mkList(v, len); - auto out = v.listElems(); + auto list = state.buildList(len); + auto out = list.elems; for (unsigned int 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); } static RegisterPrimOp primop_concatMap({ @@ -3986,14 +4000,13 @@ void prim_match(EvalState & state, const PosIdx pos, Value * * args, Value & v) } // the first match is the whole string - const size_t len = match.size() - 1; - state.mkList(v, len); - for (size_t i = 0; i < len; ++i) { - if (!match[i+1].matched) - (v.listElems()[i] = state.allocValue())->mkNull(); + auto list = state.buildList(match.size() - 1); + for (const auto & [i, v2] : enumerate(list)) + if (!match[i + 1].matched) + (v2 = state.allocValue())->mkNull(); else - (v.listElems()[i] = state.allocValue())->mkString(match[i + 1].str()); - } + (v2 = state.allocValue())->mkString(match[i + 1].str()); + v.mkList(list); } catch (std::regex_error & e) { if (e.code() == std::regex_constants::error_space) { @@ -4062,11 +4075,12 @@ void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v) // Any matches results are surrounded by non-matching results. const size_t len = std::distance(begin, end); - state.mkList(v, 2 * len + 1); + auto list = state.buildList(2 * len + 1); size_t idx = 0; if (len == 0) { - v.listElems()[idx++] = args[1]; + list[0] = args[1]; + v.mkList(list); return; } @@ -4075,28 +4089,31 @@ void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v) auto match = *i; // Add a string for non-matched characters. - (v.listElems()[idx++] = state.allocValue())->mkString(match.prefix().str()); + (list[idx++] = state.allocValue())->mkString(match.prefix().str()); // Add a list for matched substrings. const size_t slen = match.size() - 1; - auto elem = v.listElems()[idx++] = state.allocValue(); // Start at 1, beacause the first match is the whole string. - state.mkList(*elem, slen); - for (size_t si = 0; si < slen; ++si) { + auto list2 = state.buildList(slen); + for (const auto & [si, v2] : enumerate(list2)) { if (!match[si + 1].matched) - (elem->listElems()[si] = state.allocValue())->mkNull(); + v2 = &state.vNull; else - (elem->listElems()[si] = state.allocValue())->mkString(match[si + 1].str()); + (v2 = state.allocValue())->mkString(match[si + 1].str()); } + (list[idx++] = state.allocValue())->mkList(list2); + // Add a string for non-matched suffix characters. if (idx == 2 * len) - (v.listElems()[idx++] = state.allocValue())->mkString(match.suffix().str()); + (list[idx++] = state.allocValue())->mkString(match.suffix().str()); } assert(idx == 2 * len + 1); + v.mkList(list); + } catch (std::regex_error & e) { if (e.code() == std::regex_constants::error_space) { // limit is _GLIBCXX_REGEX_STATE_LIMIT for libstdc++ @@ -4316,9 +4333,10 @@ static void prim_splitVersion(EvalState & state, const PosIdx pos, Value * * arg break; components.emplace_back(component); } - state.mkList(v, components.size()); + auto list = state.buildList(components.size()); for (const auto & [n, component] : enumerate(components)) - (v.listElems()[n] = state.allocValue())->mkString(std::move(component)); + (list[n] = state.allocValue())->mkString(std::move(component)); + v.mkList(list); } static RegisterPrimOp primop_splitVersion({ @@ -4559,14 +4577,14 @@ void EvalState::createBaseEnv() }); /* Add a value containing the current Nix expression search path. */ - mkList(v, searchPath.elements.size()); - int n = 0; - for (auto & i : searchPath.elements) { + auto list = buildList(searchPath.elements.size()); + for (const auto & [n, i] : enumerate(searchPath.elements)) { auto attrs = buildBindings(2); attrs.alloc("path").mkString(i.path.s); attrs.alloc("prefix").mkString(i.prefix.s); - (v.listElems()[n++] = allocValue())->mkAttrs(attrs); + (list[n] = allocValue())->mkAttrs(attrs); } + v.mkList(list); addConstant("__nixPath", v, { .type = nList, .doc = R"( diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 1eec8b316..4d000b2ce 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -207,10 +207,10 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args, if (info.second.allOutputs) infoAttrs.alloc(sAllOutputs).mkBool(true); if (!info.second.outputs.empty()) { - auto & outputsVal = infoAttrs.alloc(state.sOutputs); - state.mkList(outputsVal, info.second.outputs.size()); + auto list = state.buildList(info.second.outputs.size()); for (const auto & [i, output] : enumerate(info.second.outputs)) - (outputsVal.listElems()[i] = state.allocValue())->mkString(output); + (list[i] = state.allocValue())->mkString(output); + infoAttrs.alloc(state.sOutputs).mkList(list); } attrs.alloc(state.store->printStorePath(info.first)).mkAttrs(infoAttrs); } diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index 94be7960a..9bee8ca38 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -38,10 +38,10 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value * * args, V { auto array = toml::get>(t); - size_t size = array.size(); - state.mkList(v, size); - for (size_t i = 0; i < size; ++i) - visit(*(v.listElems()[i] = state.allocValue()), array[i]); + auto list = state.buildList(array.size()); + for (const auto & [n, v] : enumerate(list)) + visit(*(v = state.allocValue()), array[n]); + v.mkList(list); } break;; case toml::value_t::boolean: diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index e7aea4949..9f0600efb 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -18,6 +18,7 @@ namespace nix { +struct Value; class BindingsBuilder; @@ -134,6 +135,34 @@ class ExternalValueBase std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); +class ListBuilder +{ + const size_t size; + Value * inlineElems[2] = {nullptr, nullptr}; +public: + Value * * elems; + ListBuilder(EvalState & state, size_t size); + + ListBuilder(ListBuilder && x) + : size(x.size) + , inlineElems{x.inlineElems[0], x.inlineElems[1]} + , elems(size <= 2 ? inlineElems : x.elems) + { } + + Value * & operator [](size_t n) + { + return elems[n]; + } + + typedef Value * * iterator; + + iterator begin() { return &elems[0]; } + iterator end() { return &elems[size]; } + + friend class Value; +}; + + struct Value { private: @@ -323,16 +352,20 @@ public: Value & mkAttrs(BindingsBuilder & bindings); - inline void mkList(size_t size) + void mkList(const ListBuilder & builder) { clearValue(); - if (size == 1) + if (builder.size == 1) { + smallList[0] = builder.inlineElems[0]; internalType = tList1; - else if (size == 2) + } else if (builder.size == 2) { + smallList[0] = builder.inlineElems[0]; + smallList[1] = builder.inlineElems[1]; internalType = tList2; - else { + } else { + bigList.size = builder.size; + bigList.elems = builder.elems; internalType = tListN; - bigList.size = size; } } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 5e3de20c5..f79755375 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -172,7 +172,7 @@ static void loadSourceExpr(EvalState & state, const SourcePath & path, Value & v directory). */ else if (st.type == InputAccessor::tDirectory) { auto attrs = state.buildBindings(maxAttrs); - state.mkList(attrs.alloc("_combineChannels"), 0); + attrs.insert(state.symbols.create("_combineChannels"), &state.vEmptyList); StringSet seen; getAllExprs(state, path, seen, attrs); v.mkAttrs(attrs); diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 8bebe2b9e..dd27344aa 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -49,10 +49,8 @@ bool createUserEnv(EvalState & state, PackageInfos & elems, /* Construct the whole top level derivation. */ StorePathSet references; - Value manifest; - state.mkList(manifest, elems.size()); - size_t n = 0; - for (auto & i : elems) { + auto list = state.buildList(elems.size()); + for (const auto & [n, i] : enumerate(elems)) { /* Create a pseudo-derivation containing the name, system, output paths, and optionally the derivation path, as well as the meta attributes. */ @@ -72,10 +70,9 @@ bool createUserEnv(EvalState & state, PackageInfos & elems, attrs.alloc(state.sDrvPath).mkString(state.store->printStorePath(*drvPath)); // Copy each output meant for installation. - auto & vOutputs = attrs.alloc(state.sOutputs); - state.mkList(vOutputs, outputs.size()); + auto outputsList = state.buildList(outputs.size()); for (const auto & [m, j] : enumerate(outputs)) { - (vOutputs.listElems()[m] = state.allocValue())->mkString(j.first); + (outputsList[m] = state.allocValue())->mkString(j.first); auto outputAttrs = state.buildBindings(2); outputAttrs.alloc(state.sOutPath).mkString(state.store->printStorePath(*j.second)); attrs.alloc(j.first).mkAttrs(outputAttrs); @@ -87,6 +84,7 @@ bool createUserEnv(EvalState & state, PackageInfos & elems, references.insert(*j.second); } + attrs.alloc(state.sOutputs).mkList(outputsList); // Copy the meta attributes. auto meta = state.buildBindings(metaNames.size()); @@ -98,11 +96,14 @@ bool createUserEnv(EvalState & state, PackageInfos & elems, attrs.alloc(state.sMeta).mkAttrs(meta); - (manifest.listElems()[n++] = state.allocValue())->mkAttrs(attrs); + (list[n] = state.allocValue())->mkAttrs(attrs); if (drvPath) references.insert(*drvPath); } + Value manifest; + manifest.mkList(list); + /* Also write a copy of the list of user environment elements to the store; we need it for future modifications of the environment. */ diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index d2d699a64..43b545035 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -79,11 +79,11 @@ TEST_F(ValuePrintingTests, tList) Value vTwo; vTwo.mkInt(2); + auto list = state.buildList(3); + list.elems[0] = &vOne; + list.elems[1] = &vTwo; Value vList; - state.mkList(vList, 5); - vList.bigList.elems[0] = &vOne; - vList.bigList.elems[1] = &vTwo; - vList.bigList.size = 3; + vList.mkList(list); test(vList, "[ 1 2 «nullptr» ]"); } @@ -249,12 +249,12 @@ TEST_F(ValuePrintingTests, depthList) Value vNested; vNested.mkAttrs(builder2.finish()); + auto list = state.buildList(3); + list.elems[0] = &vOne; + list.elems[1] = &vTwo; + list.elems[2] = &vNested; Value vList; - state.mkList(vList, 5); - vList.bigList.elems[0] = &vOne; - vList.bigList.elems[1] = &vTwo; - vList.bigList.elems[2] = &vNested; - vList.bigList.size = 3; + vList.mkList(list); test(vList, "[ 1 2 { ... } ]", PrintOptions { .maxDepth = 1 }); test(vList, "[ 1 2 { nested = { ... }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 2 }); @@ -539,11 +539,11 @@ TEST_F(ValuePrintingTests, ansiColorsList) Value vTwo; vTwo.mkInt(2); + auto list = state.buildList(3); + list.elems[0] = &vOne; + list.elems[1] = &vTwo; Value vList; - state.mkList(vList, 5); - vList.bigList.elems[0] = &vOne; - vList.bigList.elems[1] = &vTwo; - vList.bigList.size = 3; + vList.mkList(list); test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_CYAN "2" ANSI_NORMAL " " ANSI_MAGENTA "«nullptr»" ANSI_NORMAL " ]", @@ -670,11 +670,11 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated) Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); + auto list = state.buildList(2); + list.elems[0] = &vEmpty; + list.elems[1] = &vEmpty; Value vList; - state.mkList(vList, 3); - vList.bigList.elems[0] = &vEmpty; - vList.bigList.elems[1] = &vEmpty; - vList.bigList.size = 2; + vList.mkList(list); test(vList, "[ { } " ANSI_MAGENTA "«repeated»" ANSI_NORMAL " ]", @@ -690,11 +690,11 @@ TEST_F(ValuePrintingTests, listRepeated) Value vEmpty; vEmpty.mkAttrs(emptyBuilder.finish()); + auto list = state.buildList(2); + list.elems[0] = &vEmpty; + list.elems[1] = &vEmpty; Value vList; - state.mkList(vList, 3); - vList.bigList.elems[0] = &vEmpty; - vList.bigList.elems[1] = &vEmpty; - vList.bigList.size = 2; + vList.mkList(list); test(vList, "[ { } «repeated» ]", PrintOptions { }); test(vList, @@ -750,11 +750,12 @@ TEST_F(ValuePrintingTests, ansiColorsListElided) Value vTwo; vTwo.mkInt(2); + { + auto list = state.buildList(2); + list.elems[0] = &vOne; + list.elems[1] = &vTwo; Value vList; - state.mkList(vList, 4); - vList.bigList.elems[0] = &vOne; - vList.bigList.elems[1] = &vTwo; - vList.bigList.size = 2; + vList.mkList(list); test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT "«1 item elided»" ANSI_NORMAL " ]", @@ -762,12 +763,18 @@ TEST_F(ValuePrintingTests, ansiColorsListElided) .ansiColors = true, .maxListItems = 1 }); + } Value vThree; vThree.mkInt(3); - vList.bigList.elems[2] = &vThree; - vList.bigList.size = 3; + { + auto list = state.buildList(3); + list.elems[0] = &vOne; + list.elems[1] = &vTwo; + list.elems[2] = &vThree; + Value vList; + vList.mkList(list); test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT "«2 items elided»" ANSI_NORMAL " ]", @@ -775,6 +782,7 @@ TEST_F(ValuePrintingTests, ansiColorsListElided) .ansiColors = true, .maxListItems = 1 }); + } } } // namespace nix