From 6d9a6d2cc3a20c9047436d174c9f91a2223da220 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 17:39:16 +0100 Subject: [PATCH 1/7] Ensure that attrsets are sorted Previously you had to remember to call value->attrs->sort() after populating value->attrs. Now there is a BindingsBuilder helper that wraps Bindings and ensures that sort() is called before you can use it. --- src/libexpr/attr-set.cc | 25 +++++- src/libexpr/attr-set.hh | 34 ++++++++ src/libexpr/common-eval-args.cc | 9 +- src/libexpr/eval.cc | 50 ++++++----- src/libexpr/eval.hh | 7 +- src/libexpr/get-drvs.cc | 13 ++- src/libexpr/primops.cc | 120 ++++++++++++-------------- src/libexpr/primops/context.cc | 21 +++-- src/libexpr/primops/fetchMercurial.cc | 14 +-- src/libexpr/primops/fetchTree.cc | 27 +++--- src/libexpr/primops/fromTOML.cc | 11 +-- src/libexpr/value.hh | 8 ++ src/nix-build/nix-build.cc | 11 +-- src/nix-env/nix-env.cc | 28 +++--- src/nix-env/user-env.cc | 57 ++++++------ src/nix/bundle.cc | 14 +-- src/nix/main.cc | 9 +- 17 files changed, 260 insertions(+), 198 deletions(-) diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index b6091c955..d74243f45 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -41,16 +41,39 @@ Value * EvalState::allocAttr(Value & vAttrs, const Symbol & name) } -Value * EvalState::allocAttr(Value & vAttrs, const std::string & name) +Value * EvalState::allocAttr(Value & vAttrs, std::string_view name) { return allocAttr(vAttrs, symbols.create(name)); } +Value & BindingsBuilder::alloc(const Symbol & name, ptr pos) +{ + auto value = state.allocValue(); + bindings->push_back(Attr(name, value, pos)); + return *value; +} + + +Value & BindingsBuilder::alloc(std::string_view name, ptr pos) +{ + return alloc(state.symbols.create(name), pos); +} + + void Bindings::sort() { std::sort(begin(), end()); } +Value & Value::mkAttrs(BindingsBuilder & bindings) +{ + clearValue(); + internalType = tAttrs; + attrs = bindings.finish(); + return *this; +} + + } diff --git a/src/libexpr/attr-set.hh b/src/libexpr/attr-set.hh index 7d6ffc9f3..903289b69 100644 --- a/src/libexpr/attr-set.hh +++ b/src/libexpr/attr-set.hh @@ -113,5 +113,39 @@ public: friend class EvalState; }; +/* A wrapper around Bindings that ensures that its always in sorted + order at the end. The only way to consume a BindingsBuilder is to + call finish(), which sorts the bindings. */ +class BindingsBuilder +{ + EvalState & state; + Bindings * bindings; + +public: + + BindingsBuilder(EvalState & state, Bindings * bindings) + : state(state), bindings(bindings) + { } + + void insert(Symbol name, Value * value, ptr pos = ptr(&noPos)) + { + insert(Attr(name, value, pos)); + } + + void insert(const Attr & attr) + { + bindings->push_back(attr); + } + + Value & alloc(const Symbol & name, ptr pos = ptr(&noPos)); + + Value & alloc(std::string_view name, ptr pos = ptr(&noPos)); + + Bindings * finish() + { + bindings->sort(); + return bindings; + } +}; } diff --git a/src/libexpr/common-eval-args.cc b/src/libexpr/common-eval-args.cc index fb0932c00..ecae2ca1f 100644 --- a/src/libexpr/common-eval-args.cc +++ b/src/libexpr/common-eval-args.cc @@ -73,17 +73,16 @@ MixEvalArgs::MixEvalArgs() Bindings * MixEvalArgs::getAutoArgs(EvalState & state) { - Bindings * res = state.allocBindings(autoArgs.size()); + auto res = state.buildBindings(autoArgs.size()); for (auto & i : autoArgs) { - Value * v = state.allocValue(); + auto v = state.allocValue(); if (i.second[0] == 'E') state.mkThunk_(*v, state.parseExprFromString(string(i.second, 1), absPath("."))); else mkString(*v, string(i.second, 1)); - res->push_back(Attr(state.symbols.create(i.first), v)); + res.insert(state.symbols.create(i.first), v); } - res->sort(); - return res; + return res.finish(); } Path lookupFileArg(EvalState & state, string s) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index a95726f5f..0710ce270 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -772,18 +772,30 @@ void mkString(Value & v, const char * s) } +void Value::mkString(std::string_view s) +{ + mkString(dupStringWithLen(s.data(), s.size())); +} + + Value & mkString(Value & v, std::string_view s, const PathSet & context) { - v.mkString(dupStringWithLen(s.data(), s.size())); + v.mkString(s, context); + return v; +} + + +void Value::mkString(std::string_view s, const PathSet & context) +{ + mkString(s); if (!context.empty()) { size_t n = 0; - v.string.context = (const char * *) + string.context = (const char * *) allocBytes((context.size() + 1) * sizeof(char *)); for (auto & i : context) - v.string.context[n++] = dupString(i.c_str()); - v.string.context[n] = 0; + string.context[n++] = dupString(i.c_str()); + string.context[n] = 0; } - return v; } @@ -882,11 +894,11 @@ void EvalState::mkThunk_(Value & v, Expr * expr) void EvalState::mkPos(Value & v, ptr pos) { if (pos->file.set()) { - mkAttrs(v, 3); - mkString(*allocAttr(v, sFile), pos->file); - mkInt(*allocAttr(v, sLine), pos->line); - mkInt(*allocAttr(v, sColumn), pos->column); - v.attrs->sort(); + auto attrs = buildBindings(3); + attrs.alloc(sFile).mkString(pos->file); + attrs.alloc(sLine).mkInt(pos->line); + attrs.alloc(sColumn).mkInt(pos->column); + v.mkAttrs(attrs); } else mkNull(v); } @@ -1341,7 +1353,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & /* Nope, so show the first unexpected argument to the user. */ for (auto & i : *args[0]->attrs) - if (lambda.formals->argNames.find(i.name) == lambda.formals->argNames.end()) + if (!lambda.formals->argNames.count(i.name)) throwTypeError(pos, "%1% called with unexpected argument '%2%'", lambda, i.name); abort(); // can't happen } @@ -1484,22 +1496,20 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) return; } - Value * actualArgs = allocValue(); - mkAttrs(*actualArgs, std::max(static_cast(fun.lambda.fun->formals->formals.size()), args.size())); + auto attrs = buildBindings(std::max(static_cast(fun.lambda.fun->formals->formals.size()), args.size())); if (fun.lambda.fun->formals->ellipsis) { // If the formals have an ellipsis (eg the function accepts extra args) pass // all available automatic arguments (which includes arguments specified on // the command line via --arg/--argstr) - for (auto& v : args) { - actualArgs->attrs->push_back(v); - } + for (auto & v : args) + attrs.insert(v); } else { // Otherwise, only pass the arguments that the function accepts for (auto & i : fun.lambda.fun->formals->formals) { Bindings::iterator j = args.find(i.name); if (j != args.end()) { - actualArgs->attrs->push_back(*j); + attrs.insert(*j); } else if (!i.def) { throwMissingArgumentError(i.pos, R"(cannot evaluate a function that has an argument without a value ('%1%') @@ -1512,9 +1522,7 @@ https://nixos.org/manual/nix/stable/#ss-functions.)", i.name); } } - actualArgs->attrs->sort(); - - callFunction(fun, *actualArgs, res, noPos); + callFunction(fun, allocValue()->mkAttrs(attrs), res, noPos); } @@ -1719,7 +1727,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) auto path = canonPath(s.str()); mkPath(v, path.c_str()); } else - mkString(v, s.str(), context); + v.mkString(s.str(), context); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index d7ef7b88a..44ccc6593 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -339,10 +339,15 @@ public: Env & allocEnv(size_t size); Value * allocAttr(Value & vAttrs, const Symbol & name); - Value * allocAttr(Value & vAttrs, const std::string & name); + Value * allocAttr(Value & vAttrs, std::string_view name); Bindings * allocBindings(size_t capacity); + BindingsBuilder buildBindings(size_t capacity) + { + return BindingsBuilder(*this, allocBindings(capacity)); + } + void mkList(Value & v, size_t length); void mkAttrs(Value & v, size_t capacity); void mkThunk_(Value & v, Expr * expr); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index ed4c47fbb..25fd9b949 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -254,15 +254,14 @@ bool DrvInfo::queryMetaBool(const string & name, bool def) void DrvInfo::setMeta(const string & name, Value * v) { getMeta(); - Bindings * old = meta; - meta = state->allocBindings(1 + (old ? old->size() : 0)); + auto attrs = state->buildBindings(1 + (meta ? meta->size() : 0)); Symbol sym = state->symbols.create(name); - if (old) - for (auto i : *old) + if (meta) + for (auto i : *meta) if (i.name != sym) - meta->push_back(i); - if (v) meta->push_back(Attr(sym, v)); - meta->sort(); + attrs.insert(i); + if (v) attrs.insert(sym, v); + meta = attrs.finish(); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index ab99962f7..daf5fc927 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -125,13 +125,15 @@ static Path realisePath(EvalState & state, const Pos & pos, Value & v, const Rea the actual path. The 'drv' and 'drvPath' outputs must correspond. */ -static void mkOutputString(EvalState & state, Value & v, - const StorePath & drvPath, const BasicDerivation & drv, - std::pair o) +static void mkOutputString( + EvalState & state, + BindingsBuilder & attrs, + const StorePath & drvPath, + const BasicDerivation & drv, + const std::pair & o) { auto optOutputPath = o.second.path(*state.store, drv.name, o.first); - mkString( - *state.allocAttr(v, state.symbols.create(o.first)), + attrs.alloc(o.first).mkString( optOutputPath ? state.store->printStorePath(*optOutputPath) /* Downstream we would substitute this for an actual path once @@ -172,23 +174,19 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS if (auto optStorePath = isValidDerivationInStore()) { auto storePath = *optStorePath; Derivation drv = state.store->readDerivation(storePath); - Value & w = *state.allocValue(); - state.mkAttrs(w, 3 + drv.outputs.size()); - Value * v2 = state.allocAttr(w, state.sDrvPath); - mkString(*v2, path, {"=" + path}); - v2 = state.allocAttr(w, state.sName); - mkString(*v2, drv.env["name"]); - Value * outputsVal = - state.allocAttr(w, state.symbols.create("outputs")); - state.mkList(*outputsVal, drv.outputs.size()); - unsigned int outputs_index = 0; + auto attrs = state.buildBindings(3 + drv.outputs.size()); + attrs.alloc(state.sDrvPath).mkString(path, {"=" + path}); + attrs.alloc(state.sName).mkString(drv.env["name"]); + auto & outputsVal = attrs.alloc(state.sOutputs); + state.mkList(outputsVal, drv.outputs.size()); - for (const auto & o : drv.outputs) { - mkOutputString(state, w, storePath, drv, o); - outputsVal->listElems()[outputs_index] = state.allocValue(); - mkString(*(outputsVal->listElems()[outputs_index++]), o.first); + for (const auto & [i, o] : enumerate(drv.outputs)) { + mkOutputString(state, attrs, storePath, drv, o); + (outputsVal.listElems()[i] = state.allocValue())->mkString(o.first); } - w.attrs->sort(); + + auto w = state.allocValue(); + w->mkAttrs(attrs); if (!state.vImportedDrvToDerivation) { state.vImportedDrvToDerivation = allocRootValue(state.allocValue()); @@ -198,7 +196,7 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS } state.forceFunction(**state.vImportedDrvToDerivation, pos); - mkApp(v, **state.vImportedDrvToDerivation, w); + mkApp(v, **state.vImportedDrvToDerivation, *w); state.forceAttrs(v, pos); } @@ -802,16 +800,16 @@ static RegisterPrimOp primop_floor({ * else => {success=false; value=false;} */ static void prim_tryEval(EvalState & state, const Pos & pos, Value * * args, Value & v) { - state.mkAttrs(v, 2); + auto attrs = state.buildBindings(2); try { state.forceValue(*args[0], pos); - v.attrs->push_back(Attr(state.sValue, args[0])); - mkBool(*state.allocAttr(v, state.symbols.create("success")), true); + attrs.insert(state.sValue, args[0]); + mkBool(attrs.alloc("success"), true); } catch (AssertionError & e) { - mkBool(*state.allocAttr(v, state.sValue), false); - mkBool(*state.allocAttr(v, state.symbols.create("success")), false); + mkBool(attrs.alloc(state.sValue), false); + mkBool(attrs.alloc("success"), false); } - v.attrs->sort(); + v.mkAttrs(attrs); } static RegisterPrimOp primop_tryEval({ @@ -839,7 +837,7 @@ static RegisterPrimOp primop_tryEval({ static void prim_getEnv(EvalState & state, const Pos & pos, Value * * args, Value & v) { string name = state.forceStringNoCtx(*args[0], pos); - mkString(v, evalSettings.restrictEval || evalSettings.pureEval ? "" : getEnv(name).value_or("")); + v.mkString(evalSettings.restrictEval || evalSettings.pureEval ? "" : getEnv(name).value_or("")); } static RegisterPrimOp primop_getEnv({ @@ -1265,11 +1263,11 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * drvHashes.lock()->insert_or_assign(drvPath, h); } - state.mkAttrs(v, 1 + drv.outputs.size()); - mkString(*state.allocAttr(v, state.sDrvPath), drvPathS, {"=" + drvPathS}); + auto attrs = state.buildBindings(1 + drv.outputs.size()); + attrs.alloc(state.sDrvPath).mkString(drvPathS, {"=" + drvPathS}); for (auto & i : drv.outputs) - mkOutputString(state, v, drvPath, drv, i); - v.attrs->sort(); + mkOutputString(state, attrs, drvPath, drv, i); + v.mkAttrs(attrs); } static RegisterPrimOp primop_derivationStrict(RegisterPrimOp::Info { @@ -1579,20 +1577,20 @@ static void prim_readDir(EvalState & state, const Pos & pos, Value * * args, Val } DirEntries entries = readDirectory(path); - state.mkAttrs(v, entries.size()); + + auto attrs = state.buildBindings(entries.size()); for (auto & ent : entries) { - Value * ent_val = state.allocAttr(v, state.symbols.create(ent.name)); if (ent.type == DT_UNKNOWN) ent.type = getFileType(path + "/" + ent.name); - ent_val->mkString( + attrs.alloc(ent.name).mkString( ent.type == DT_REG ? "regular" : ent.type == DT_DIR ? "directory" : ent.type == DT_LNK ? "symlink" : "unknown"); } - v.attrs->sort(); + v.mkAttrs(attrs); } static RegisterPrimOp primop_readDir({ @@ -2308,7 +2306,7 @@ static void prim_listToAttrs(EvalState & state, const Pos & pos, Value * * args, { state.forceList(*args[0], pos); - state.mkAttrs(v, args[0]->listSize()); + auto attrs = state.buildBindings(args[0]->listSize()); std::set seen; @@ -2334,11 +2332,11 @@ static void prim_listToAttrs(EvalState & state, const Pos & pos, Value * * args, v2->attrs, pos ); - v.attrs->push_back(Attr(sym, j2->value, j2->pos)); + attrs.insert(sym, j2->value, j2->pos); } } - v.attrs->sort(); + v.mkAttrs(attrs); } static RegisterPrimOp primop_listToAttrs({ @@ -2445,14 +2443,11 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args return; } - state.mkAttrs(v, args[0]->lambda.fun->formals->formals.size()); - for (auto & i : args[0]->lambda.fun->formals->formals) { + auto attrs = state.buildBindings(args[0]->lambda.fun->formals->formals.size()); + for (auto & i : args[0]->lambda.fun->formals->formals) // !!! should optimise booleans (allocate only once) - Value * value = state.allocValue(); - v.attrs->push_back(Attr(i.name, value, ptr(&i.pos))); - mkBool(*value, i.def); - } - v.attrs->sort(); + mkBool(attrs.alloc(i.name, ptr(&i.pos)), i.def); + v.mkAttrs(attrs); } static RegisterPrimOp primop_functionArgs({ @@ -3003,21 +2998,21 @@ static void prim_partition(EvalState & state, const Pos & pos, Value * * args, V wrong.push_back(vElem); } - state.mkAttrs(v, 2); + auto attrs = state.buildBindings(2); - Value * vRight = state.allocAttr(v, state.sRight); + auto & vRight = attrs.alloc(state.sRight); auto rsize = right.size(); - state.mkList(*vRight, rsize); + state.mkList(vRight, rsize); if (rsize) - memcpy(vRight->listElems(), right.data(), sizeof(Value *) * rsize); + memcpy(vRight.listElems(), right.data(), sizeof(Value *) * rsize); - Value * vWrong = state.allocAttr(v, state.sWrong); + auto & vWrong = attrs.alloc(state.sWrong); auto wsize = wrong.size(); - state.mkList(*vWrong, wsize); + state.mkList(vWrong, wsize); if (wsize) - memcpy(vWrong->listElems(), wrong.data(), sizeof(Value *) * wsize); + memcpy(vWrong.listElems(), wrong.data(), sizeof(Value *) * wsize); - v.attrs->sort(); + v.mkAttrs(attrs); } static RegisterPrimOp primop_partition({ @@ -3734,10 +3729,10 @@ static void prim_parseDrvName(EvalState & state, const Pos & pos, Value * * args { string name = state.forceStringNoCtx(*args[0], pos); DrvName parsed(name); - state.mkAttrs(v, 2); - mkString(*state.allocAttr(v, state.sName), parsed.name); - mkString(*state.allocAttr(v, state.symbols.create("version")), parsed.version); - v.attrs->sort(); + auto attrs = state.buildBindings(2); + attrs.alloc(state.sName).mkString(parsed.name); + attrs.alloc("version").mkString(parsed.version); + v.mkAttrs(attrs); } static RegisterPrimOp primop_parseDrvName({ @@ -3883,11 +3878,10 @@ void EvalState::createBaseEnv() mkList(v, searchPath.size()); int n = 0; for (auto & i : searchPath) { - auto v2 = v.listElems()[n++] = allocValue(); - mkAttrs(*v2, 2); - mkString(*allocAttr(*v2, symbols.create("path")), i.second); - mkString(*allocAttr(*v2, symbols.create("prefix")), i.first); - v2->attrs->sort(); + auto attrs = buildBindings(2); + attrs.alloc("path").mkString(i.second); + attrs.alloc("prefix").mkString(i.first); + (v.listElems()[n++] = allocValue())->mkAttrs(attrs); } addConstant("__nixPath", v); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 20545afd0..ab08303d8 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -103,27 +103,26 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, } } - state.mkAttrs(v, contextInfos.size()); + auto attrs = state.buildBindings(contextInfos.size()); auto sPath = state.symbols.create("path"); auto sAllOutputs = state.symbols.create("allOutputs"); for (const auto & info : contextInfos) { - auto & infoVal = *state.allocAttr(v, state.symbols.create(info.first)); - state.mkAttrs(infoVal, 3); + auto infoAttrs = state.buildBindings(3); if (info.second.path) - mkBool(*state.allocAttr(infoVal, sPath), true); + mkBool(infoAttrs.alloc(sPath), true); if (info.second.allOutputs) - mkBool(*state.allocAttr(infoVal, sAllOutputs), true); + mkBool(infoAttrs.alloc(sAllOutputs), true); if (!info.second.outputs.empty()) { - auto & outputsVal = *state.allocAttr(infoVal, state.sOutputs); + auto & outputsVal = infoAttrs.alloc(state.sOutputs); state.mkList(outputsVal, info.second.outputs.size()); - size_t i = 0; - for (const auto & output : info.second.outputs) - mkString(*(outputsVal.listElems()[i++] = state.allocValue()), output); + for (const auto & [i, output] : enumerate(info.second.outputs)) + (outputsVal.listElems()[i] = state.allocValue())->mkString(output); } - infoVal.attrs->sort(); + attrs.alloc(info.first).mkAttrs(infoAttrs); } - v.attrs->sort(); + + v.mkAttrs(attrs); } static RegisterPrimOp primop_getContext("__getContext", 1, prim_getContext); diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index c23480853..42214c207 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -70,19 +70,19 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar // FIXME: use name auto [tree, input2] = input.fetch(state.store); - state.mkAttrs(v, 8); + auto attrs2 = state.buildBindings(8); auto storePath = state.store->printStorePath(tree.storePath); - mkString(*state.allocAttr(v, state.sOutPath), storePath, PathSet({storePath})); + attrs2.alloc(state.sOutPath).mkString(storePath, {storePath}); if (input2.getRef()) - mkString(*state.allocAttr(v, state.symbols.create("branch")), *input2.getRef()); + attrs2.alloc("branch").mkString(*input2.getRef()); // Backward compatibility: set 'rev' to // 0000000000000000000000000000000000000000 for a dirty tree. auto rev2 = input2.getRev().value_or(Hash(htSHA1)); - mkString(*state.allocAttr(v, state.symbols.create("rev")), rev2.gitRev()); - mkString(*state.allocAttr(v, state.symbols.create("shortRev")), std::string(rev2.gitRev(), 0, 12)); + attrs2.alloc("rev").mkString(rev2.gitRev()); + attrs2.alloc("shortRev").mkString(rev2.gitRev().substr(0, 12)); if (auto revCount = input2.getRevCount()) - mkInt(*state.allocAttr(v, state.symbols.create("revCount")), *revCount); - v.attrs->sort(); + attrs2.alloc("revCount").mkInt(*revCount); + v.mkAttrs(attrs2); state.allowPath(tree.storePath); } diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 079513873..a97bfcae7 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -21,49 +21,48 @@ void emitTreeAttrs( { assert(input.isImmutable()); - state.mkAttrs(v, 8); + auto attrs = state.buildBindings(8); auto storePath = state.store->printStorePath(tree.storePath); - mkString(*state.allocAttr(v, state.sOutPath), storePath, PathSet({storePath})); + attrs.alloc(state.sOutPath).mkString(storePath, {storePath}); // FIXME: support arbitrary input attributes. auto narHash = input.getNarHash(); assert(narHash); - mkString(*state.allocAttr(v, state.symbols.create("narHash")), - narHash->to_string(SRI, true)); + attrs.alloc("narHash").mkString(narHash->to_string(SRI, true)); if (input.getType() == "git") - mkBool(*state.allocAttr(v, state.symbols.create("submodules")), + attrs.alloc("submodules").mkBool( fetchers::maybeGetBoolAttr(input.attrs, "submodules").value_or(false)); if (!forceDirty) { if (auto rev = input.getRev()) { - mkString(*state.allocAttr(v, state.symbols.create("rev")), rev->gitRev()); - mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev->gitShortRev()); + attrs.alloc("rev").mkString(rev->gitRev()); + attrs.alloc("shortRev").mkString(rev->gitShortRev()); } else if (emptyRevFallback) { // Backwards compat for `builtins.fetchGit`: dirty repos return an empty sha1 as rev auto emptyHash = Hash(htSHA1); - mkString(*state.allocAttr(v, state.symbols.create("rev")), emptyHash.gitRev()); - mkString(*state.allocAttr(v, state.symbols.create("shortRev")), emptyHash.gitShortRev()); + attrs.alloc("rev").mkString(emptyHash.gitRev()); + attrs.alloc("shortRev").mkString(emptyHash.gitShortRev()); } if (auto revCount = input.getRevCount()) - mkInt(*state.allocAttr(v, state.symbols.create("revCount")), *revCount); + attrs.alloc("revCount").mkInt(*revCount); else if (emptyRevFallback) - mkInt(*state.allocAttr(v, state.symbols.create("revCount")), 0); + attrs.alloc("revCount").mkInt(0); } if (auto lastModified = input.getLastModified()) { - mkInt(*state.allocAttr(v, state.symbols.create("lastModified")), *lastModified); - mkString(*state.allocAttr(v, state.symbols.create("lastModifiedDate")), + attrs.alloc("lastModified").mkInt(*lastModified); + attrs.alloc("lastModifiedDate").mkString( fmt("%s", std::put_time(std::gmtime(&*lastModified), "%Y%m%d%H%M%S"))); } - v.attrs->sort(); + v.mkAttrs(attrs); } std::string fixURI(std::string uri, EvalState & state, const std::string & defaultScheme = "file") diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index 30466fc5b..a328b6736 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -24,15 +24,12 @@ static void prim_fromTOML(EvalState & state, const Pos & pos, Value * * args, Va size_t size = 0; for (auto & i : table) { (void) i; size++; } - state.mkAttrs(v, size); + auto attrs = state.buildBindings(size); - for(auto & elem: table) { + for(auto & elem : table) + visit(attrs.alloc(elem.first), elem.second); - auto & v2 = *state.allocAttr(v, state.symbols.create(elem.first)); - visit(v2, elem.second); - } - - v.attrs->sort(); + v.mkAttrs(attrs); } break;; case toml::value_t::array: diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 6b4f3c0ae..a8fea02dc 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -10,6 +10,8 @@ namespace nix { +class BindingsBuilder; + typedef enum { tInt = 1, @@ -235,6 +237,10 @@ public: string.context = context; } + void mkString(std::string_view s); + + void mkString(std::string_view s, const PathSet & context); + inline void mkPath(const char * s) { clearValue(); @@ -255,6 +261,8 @@ public: attrs = a; } + Value & mkAttrs(BindingsBuilder & bindings); + inline void mkList(size_t size) { clearValue(); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index d3d6ce1ce..b73eb0795 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -258,13 +258,10 @@ static void main_nix_build(int argc, char * * argv) auto autoArgs = myArgs.getAutoArgs(*state); if (runEnv) { - auto newArgs = state->allocBindings(autoArgs->size() + 1); - auto tru = state->allocValue(); - mkBool(*tru, true); - newArgs->push_back(Attr(state->symbols.create("inNixShell"), tru)); - for (auto & i : *autoArgs) newArgs->push_back(i); - newArgs->sort(); - autoArgs = newArgs; + auto newArgs = state->buildBindings(autoArgs->size() + 1); + newArgs.alloc("inNixShell").mkBool(true); + for (auto & i : *autoArgs) newArgs.insert(i); + autoArgs = newArgs.finish(); } if (packages) { diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index b9e7be1c6..171f1c440 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -98,8 +98,11 @@ static bool isNixExpr(const Path & path, struct stat & st) } +static constexpr size_t maxAttrs = 1024; + + static void getAllExprs(EvalState & state, - const Path & path, StringSet & attrs, Value & v) + const Path & path, StringSet & seen, BindingsBuilder & attrs) { StringSet namesSorted; for (auto & i : readDirectory(path)) namesSorted.insert(i.name); @@ -124,22 +127,21 @@ static void getAllExprs(EvalState & state, string attrName = i; if (hasSuffix(attrName, ".nix")) attrName = string(attrName, 0, attrName.size() - 4); - if (!attrs.insert(attrName).second) { + if (!seen.insert(attrName).second) { printError("warning: name collision in input Nix expressions, skipping '%1%'", path2); continue; } /* Load the expression on demand. */ - Value & vFun = state.getBuiltin("import"); - Value & vArg(*state.allocValue()); - mkString(vArg, path2); - if (v.attrs->size() == v.attrs->capacity()) + auto vArg = state.allocValue(); + vArg->mkString(path2); + if (seen.size() == maxAttrs) throw Error("too many Nix expressions in directory '%1%'", path); - mkApp(*state.allocAttr(v, state.symbols.create(attrName)), vFun, vArg); + attrs.alloc(attrName).mkApp(&state.getBuiltin("import"), vArg); } else if (S_ISDIR(st.st_mode)) /* `path2' is a directory (with no default.nix in it); recurse into it. */ - getAllExprs(state, path2, attrs, v); + getAllExprs(state, path2, seen, attrs); } } @@ -161,11 +163,11 @@ static void loadSourceExpr(EvalState & state, const Path & path, Value & v) ~/.nix-defexpr directory that includes some system-wide directory). */ else if (S_ISDIR(st.st_mode)) { - state.mkAttrs(v, 1024); - state.mkList(*state.allocAttr(v, state.symbols.create("_combineChannels")), 0); - StringSet attrs; - getAllExprs(state, path, attrs, v); - v.attrs->sort(); + auto attrs = state.buildBindings(maxAttrs); + attrs.alloc("_combineChannels").mkList(0); + StringSet seen; + getAllExprs(state, path, seen, attrs); + v.mkAttrs(attrs); } else throw Error("path '%s' is not a directory or a Nix expression", path); diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 1fd4bcbd3..68c1c16b2 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -50,7 +50,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, StorePathSet references; Value manifest; state.mkList(manifest, elems.size()); - unsigned int n = 0; + size_t n = 0; for (auto & i : elems) { /* Create a pseudo-derivation containing the name, system, output paths, and optionally the derivation path, as well @@ -59,28 +59,25 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, DrvInfo::Outputs outputs = i.queryOutputs(true); StringSet metaNames = i.queryMetaNames(); - Value & v(*state.allocValue()); - manifest.listElems()[n++] = &v; - state.mkAttrs(v, 7 + outputs.size()); + auto attrs = state.buildBindings(7 + outputs.size()); - mkString(*state.allocAttr(v, state.sType), "derivation"); - mkString(*state.allocAttr(v, state.sName), i.queryName()); + attrs.alloc(state.sType).mkString("derivation"); + attrs.alloc(state.sName).mkString(i.queryName()); auto system = i.querySystem(); if (!system.empty()) - mkString(*state.allocAttr(v, state.sSystem), system); - mkString(*state.allocAttr(v, state.sOutPath), i.queryOutPath()); + attrs.alloc(state.sSystem).mkString(system); + attrs.alloc(state.sOutPath).mkString(i.queryOutPath()); if (drvPath != "") - mkString(*state.allocAttr(v, state.sDrvPath), i.queryDrvPath()); + attrs.alloc(state.sDrvPath).mkString(i.queryDrvPath()); // Copy each output meant for installation. - Value & vOutputs = *state.allocAttr(v, state.sOutputs); + auto & vOutputs = attrs.alloc(state.sOutputs); state.mkList(vOutputs, outputs.size()); - unsigned int m = 0; - for (auto & j : outputs) { - mkString(*(vOutputs.listElems()[m++] = state.allocValue()), j.first); - Value & vOutputs = *state.allocAttr(v, state.symbols.create(j.first)); - state.mkAttrs(vOutputs, 2); - mkString(*state.allocAttr(vOutputs, state.sOutPath), j.second); + for (const auto & [m, j] : enumerate(outputs)) { + (vOutputs.listElems()[m] = state.allocValue())->mkString(j.first); + auto outputAttrs = state.buildBindings(2); + outputAttrs.alloc(state.sOutPath).mkString(j.second); + attrs.alloc(j.first).mkAttrs(outputAttrs); /* This is only necessary when installing store paths, e.g., `nix-env -i /nix/store/abcd...-foo'. */ @@ -91,15 +88,16 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, } // Copy the meta attributes. - Value & vMeta = *state.allocAttr(v, state.sMeta); - state.mkAttrs(vMeta, metaNames.size()); + auto meta = state.buildBindings(metaNames.size()); for (auto & j : metaNames) { Value * v = i.queryMeta(j); if (!v) continue; - vMeta.attrs->push_back(Attr(state.symbols.create(j), v)); + meta.insert(state.symbols.create(j), v); } - vMeta.attrs->sort(); - v.attrs->sort(); + + attrs.alloc(state.sMeta).mkAttrs(meta); + + (manifest.listElems()[n++] = state.allocValue())->mkAttrs(attrs); if (drvPath != "") references.insert(state.store->parseStorePath(drvPath)); } @@ -118,13 +116,16 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, /* Construct a Nix expression that calls the user environment builder with the manifest as argument. */ - Value args, topLevel; - state.mkAttrs(args, 3); - mkString(*state.allocAttr(args, state.symbols.create("manifest")), - state.store->printStorePath(manifestFile), {state.store->printStorePath(manifestFile)}); - args.attrs->push_back(Attr(state.symbols.create("derivations"), &manifest)); - args.attrs->sort(); - mkApp(topLevel, envBuilder, args); + auto attrs = state.buildBindings(3); + attrs.alloc("manifest").mkString( + state.store->printStorePath(manifestFile), + {state.store->printStorePath(manifestFile)}); + attrs.insert(state.symbols.create("derivations"), &manifest); + Value args; + args.mkAttrs(attrs); + + Value topLevel; + topLevel.mkApp(&envBuilder, &args); /* Evaluate it. */ debug("evaluating user environment builder"); diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index aca024bca..adb5b3e73 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -78,20 +78,20 @@ struct CmdBundle : InstallableCommand Strings{bundlerName == "" ? "defaultBundler" : bundlerName}, Strings({"bundlers."}), lockFlags); - Value * arg = evalState->allocValue(); - evalState->mkAttrs(*arg, 2); + auto attrs = evalState->buildBindings(2); PathSet context; for (auto & i : app.context) context.insert("=" + store->printStorePath(i.path)); - mkString(*evalState->allocAttr(*arg, evalState->symbols.create("program")), app.program, context); + attrs.alloc("program").mkString(app.program, context); - mkString(*evalState->allocAttr(*arg, evalState->symbols.create("system")), settings.thisSystem.get()); - - arg->attrs->sort(); + attrs.alloc("system").mkString(settings.thisSystem.get()); auto vRes = evalState->allocValue(); - evalState->callFunction(*bundler.toValue(*evalState).first, *arg, *vRes, noPos); + evalState->callFunction( + *bundler.toValue(*evalState).first, + evalState->allocValue()->mkAttrs(attrs), + *vRes, noPos); if (!evalState->isDerivation(*vRes)) throw Error("the bundler '%s' does not produce a derivation", bundler.what()); diff --git a/src/nix/main.cc b/src/nix/main.cc index 759118fd5..fe7469be4 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -187,14 +187,11 @@ static void showHelp(std::vector subcommand, MultiCommand & topleve , "/"), *vUtils); - auto vArgs = state.allocValue(); - state.mkAttrs(*vArgs, 16); - auto vJson = state.allocAttr(*vArgs, state.symbols.create("command")); - mkString(*vJson, toplevel.toJSON().dump()); - vArgs->attrs->sort(); + auto attrs = state.buildBindings(16); + attrs.alloc("command").mkString(toplevel.toJSON().dump()); auto vRes = state.allocValue(); - state.callFunction(*vGenerateManpage, *vArgs, *vRes, noPos); + state.callFunction(*vGenerateManpage, state.allocValue()->mkAttrs(attrs), *vRes, noPos); auto attr = vRes->attrs->get(state.symbols.create(mdName + ".md")); if (!attr) From cc08364315437bfd870363220373c48f64862e83 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 18:24:42 +0100 Subject: [PATCH 2/7] Remove non-method mkString() --- src/libexpr/common-eval-args.cc | 2 +- src/libexpr/eval.cc | 13 ------- src/libexpr/eval.hh | 2 - src/libexpr/flake/flake.cc | 4 +- src/libexpr/json-to-value.cc | 5 ++- src/libexpr/primops.cc | 66 +++++++++++++++----------------- src/libexpr/primops/context.cc | 7 ++-- src/libexpr/primops/fetchTree.cc | 2 +- src/libexpr/primops/fromTOML.cc | 2 +- src/libexpr/value.hh | 3 -- src/nix-env/nix-env.cc | 4 +- 11 files changed, 44 insertions(+), 66 deletions(-) diff --git a/src/libexpr/common-eval-args.cc b/src/libexpr/common-eval-args.cc index ecae2ca1f..fffca4ac5 100644 --- a/src/libexpr/common-eval-args.cc +++ b/src/libexpr/common-eval-args.cc @@ -79,7 +79,7 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state) if (i.second[0] == 'E') state.mkThunk_(*v, state.parseExprFromString(string(i.second, 1), absPath("."))); else - mkString(*v, string(i.second, 1)); + v->mkString(((std::string_view) i.second).substr(1)); res.insert(state.symbols.create(i.first), v); } return res.finish(); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 0710ce270..2ac872745 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -766,25 +766,12 @@ LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, con } -void mkString(Value & v, const char * s) -{ - v.mkString(dupString(s)); -} - - void Value::mkString(std::string_view s) { mkString(dupStringWithLen(s.data(), s.size())); } -Value & mkString(Value & v, std::string_view s, const PathSet & context) -{ - v.mkString(s, context); - return v; -} - - void Value::mkString(std::string_view s, const PathSet & context) { mkString(s); diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 44ccc6593..c7f74d7b7 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -44,8 +44,6 @@ struct Env }; -Value & mkString(Value & v, std::string_view s, const PathSet & context = PathSet()); - void copyContext(const Value & v, PathSet & context); diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index c549c5971..15e8d4b06 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -682,7 +682,7 @@ void callFlake(EvalState & state, auto vTmp1 = state.allocValue(); auto vTmp2 = state.allocValue(); - mkString(*vLocks, lockedFlake.lockFile.to_string()); + vLocks->mkString(lockedFlake.lockFile.to_string()); emitTreeAttrs( state, @@ -692,7 +692,7 @@ void callFlake(EvalState & state, false, lockedFlake.flake.forceDirty); - mkString(*vRootSubdir, lockedFlake.flake.lockedRef.subdir); + vRootSubdir->mkString(lockedFlake.flake.lockedRef.subdir); if (!state.vCallFlake) { state.vCallFlake = allocRootValue(state.allocValue()); diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 9ca5ac86d..3825ca9a9 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -113,8 +113,11 @@ public: bool string(string_t & val) { - return handle_value(mkString, val.c_str()); + rs->value(state).mkString(val); + rs->add(); + return true; } + #if NLOHMANN_JSON_VERSION_MAJOR >= 3 && NLOHMANN_JSON_VERSION_MINOR >= 8 bool binary(binary_t&) { diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index daf5fc927..cb18fe494 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1285,7 +1285,7 @@ static RegisterPrimOp primop_derivationStrict(RegisterPrimOp::Info { ‘out’. */ static void prim_placeholder(EvalState & state, const Pos & pos, Value * * args, Value & v) { - mkString(v, hashPlaceholder(state.forceStringNoCtx(*args[0], pos))); + v.mkString(hashPlaceholder(state.forceStringNoCtx(*args[0], pos))); } static RegisterPrimOp primop_placeholder({ @@ -1310,7 +1310,7 @@ static void prim_toPath(EvalState & state, const Pos & pos, Value * * args, Valu { PathSet context; Path path = state.coerceToPath(pos, *args[0], context); - mkString(v, canonPath(path), context); + v.mkString(canonPath(path), context); } static RegisterPrimOp primop_toPath({ @@ -1354,7 +1354,7 @@ static void prim_storePath(EvalState & state, const Pos & pos, Value * * args, V if (!settings.readOnlyMode) state.store->ensurePath(path2); context.insert(state.store->printStorePath(path2)); - mkString(v, path, context); + v.mkString(path, context); } static RegisterPrimOp primop_storePath({ @@ -1420,7 +1420,7 @@ static RegisterPrimOp primop_pathExists({ static void prim_baseNameOf(EvalState & state, const Pos & pos, Value * * args, Value & v) { PathSet context; - mkString(v, baseNameOf(state.coerceToString(pos, *args[0], context, false, false)), context); + v.mkString(baseNameOf(state.coerceToString(pos, *args[0], context, false, false)), context); } static RegisterPrimOp primop_baseNameOf({ @@ -1441,7 +1441,7 @@ static void prim_dirOf(EvalState & state, const Pos & pos, Value * * args, Value { PathSet context; Path dir = dirOf(state.coerceToString(pos, *args[0], context, false, false)); - if (args[0]->type() == nPath) mkPath(v, dir.c_str()); else mkString(v, dir, context); + if (args[0]->type() == nPath) mkPath(v, dir.c_str()); else v.mkString(dir, context); } static RegisterPrimOp primop_dirOf({ @@ -1470,7 +1470,7 @@ static void prim_readFile(EvalState & state, const Pos & pos, Value * * args, Va string s = readFile(path); if (s.find((char) 0) != string::npos) throw Error("the contents of the file '%1%' cannot be represented as a Nix string", path); - mkString(v, s.c_str()); + v.mkString(s); } static RegisterPrimOp primop_readFile({ @@ -1549,7 +1549,7 @@ static void prim_hashFile(EvalState & state, const Pos & pos, Value * * args, Va throw EvalError("cannot read '%s' since path '%s' is not valid, at %s", path, e.path, pos); } - mkString(v, hashFile(*ht, path).to_string(Base16, false)); + v.mkString(hashFile(*ht, path).to_string(Base16, false)); } static RegisterPrimOp primop_hashFile({ @@ -1626,7 +1626,7 @@ static void prim_toXML(EvalState & state, const Pos & pos, Value * * args, Value std::ostringstream out; PathSet context; printValueAsXML(state, true, false, *args[0], out, context, pos); - mkString(v, out.str(), context); + v.mkString(out.str(), context); } static RegisterPrimOp primop_toXML({ @@ -1734,7 +1734,7 @@ static void prim_toJSON(EvalState & state, const Pos & pos, Value * * args, Valu std::ostringstream out; PathSet context; printValueAsJSON(state, true, *args[0], pos, out, context); - mkString(v, out.str(), context); + v.mkString(out.str(), context); } static RegisterPrimOp primop_toJSON({ @@ -1808,7 +1808,7 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu result, since `storePath' itself has references to the paths used in args[1]. */ - mkString(v, storePath, {storePath}); + v.mkString(storePath, {storePath}); } static RegisterPrimOp primop_toFile({ @@ -1925,10 +1925,10 @@ static void addPath( /* Call the filter function. The first argument is the path, the second is a string indicating the type of the file. */ Value arg1; - mkString(arg1, path); + arg1.mkString(path); Value arg2; - mkString(arg2, + arg2.mkString( S_ISREG(st.st_mode) ? "regular" : S_ISDIR(st.st_mode) ? "directory" : S_ISLNK(st.st_mode) ? "symlink" : @@ -1955,7 +1955,7 @@ static void addPath( } else dstPath = state.store->printStorePath(*expectedStorePath); - mkString(v, dstPath, {dstPath}); + v.mkString(dstPath, {dstPath}); state.allowPath(dstPath); @@ -3303,7 +3303,7 @@ static void prim_toString(EvalState & state, const Pos & pos, Value * * args, Va { PathSet context; string s = state.coerceToString(pos, *args[0], context, true, false); - mkString(v, s, context); + v.mkString(s, context); } static RegisterPrimOp primop_toString({ @@ -3347,7 +3347,7 @@ static void prim_substring(EvalState & state, const Pos & pos, Value * * args, V .errPos = pos }); - mkString(v, (unsigned int) start >= s.size() ? "" : string(s, start, len), context); + v.mkString((unsigned int) start >= s.size() ? "" : string(s, start, len), context); } static RegisterPrimOp primop_substring({ @@ -3401,7 +3401,7 @@ static void prim_hashString(EvalState & state, const Pos & pos, Value * * args, PathSet context; // discarded string s = state.forceString(*args[1], context, pos); - mkString(v, hashString(*ht, s).to_string(Base16, false)); + v.mkString(hashString(*ht, s).to_string(Base16, false)); } static RegisterPrimOp primop_hashString({ @@ -3451,7 +3451,7 @@ void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) if (!match[i+1].matched) mkNull(*(v.listElems()[i] = state.allocValue())); else - mkString(*(v.listElems()[i] = state.allocValue()), match[i + 1].str().c_str()); + (v.listElems()[i] = state.allocValue())->mkString(match[i + 1].str()); } } catch (std::regex_error &e) { @@ -3526,7 +3526,6 @@ static void prim_split(EvalState & state, const Pos & pos, Value * * args, Value const size_t len = std::distance(begin, end); state.mkList(v, 2 * len + 1); size_t idx = 0; - Value * elem; if (len == 0) { v.listElems()[idx++] = args[1]; @@ -3538,12 +3537,11 @@ static void prim_split(EvalState & state, const Pos & pos, Value * * args, Value std::smatch match = *i; // Add a string for non-matched characters. - elem = v.listElems()[idx++] = state.allocValue(); - mkString(*elem, match.prefix().str().c_str()); + (v.listElems()[idx++] = state.allocValue())->mkString(match.prefix().str()); // Add a list for matched substrings. const size_t slen = match.size() - 1; - elem = v.listElems()[idx++] = state.allocValue(); + auto elem = v.listElems()[idx++] = state.allocValue(); // Start at 1, beacause the first match is the whole string. state.mkList(*elem, slen); @@ -3551,15 +3549,14 @@ static void prim_split(EvalState & state, const Pos & pos, Value * * args, Value if (!match[si + 1].matched) mkNull(*(elem->listElems()[si] = state.allocValue())); else - mkString(*(elem->listElems()[si] = state.allocValue()), match[si + 1].str().c_str()); + (elem->listElems()[si] = state.allocValue())->mkString(match[si + 1].str()); } // Add a string for non-matched suffix characters. - if (idx == 2 * len) { - elem = v.listElems()[idx++] = state.allocValue(); - mkString(*elem, match.suffix().str().c_str()); - } + if (idx == 2 * len) + (v.listElems()[idx++] = state.allocValue())->mkString(match.suffix().str()); } + assert(idx == 2 * len + 1); } catch (std::regex_error &e) { @@ -3631,7 +3628,7 @@ static void prim_concatStringsSep(EvalState & state, const Pos & pos, Value * * res += state.coerceToString(pos, *elem, context); } - mkString(v, res, context); + v.mkString(res, context); } static RegisterPrimOp primop_concatStringsSep({ @@ -3700,7 +3697,7 @@ static void prim_replaceStrings(EvalState & state, const Pos & pos, Value * * ar } } - mkString(v, res, context); + v.mkString(res, context); } static RegisterPrimOp primop_replaceStrings({ @@ -3781,11 +3778,8 @@ static void prim_splitVersion(EvalState & state, const Pos & pos, Value * * args components.emplace_back(std::move(component)); } state.mkList(v, components.size()); - unsigned int n = 0; - for (auto & component : components) { - auto listElem = v.listElems()[n++] = state.allocValue(); - mkString(*listElem, std::move(component)); - } + for (const auto & [n, component] : enumerate(components)) + (v.listElems()[n] = state.allocValue())->mkString(std::move(component)); } static RegisterPrimOp primop_splitVersion({ @@ -3851,14 +3845,14 @@ void EvalState::createBaseEnv() mkInt(v, time(0)); addConstant("__currentTime", v); - mkString(v, settings.thisSystem.get()); + v.mkString(settings.thisSystem.get()); addConstant("__currentSystem", v); } - mkString(v, nixVersion); + v.mkString(nixVersion); addConstant("__nixVersion", v); - mkString(v, store->storeDir); + v.mkString(store->storeDir); addConstant("__storeDir", v); /* Language version. This should be increased every time a new diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index ab08303d8..321c3c301 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -7,8 +7,7 @@ namespace nix { static void prim_unsafeDiscardStringContext(EvalState & state, const Pos & pos, Value * * args, Value & v) { PathSet context; - string s = state.coerceToString(pos, *args[0], context); - mkString(v, s, PathSet()); + v.mkString(state.coerceToString(pos, *args[0], context)); } static RegisterPrimOp primop_unsafeDiscardStringContext("__unsafeDiscardStringContext", 1, prim_unsafeDiscardStringContext); @@ -39,7 +38,7 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const Pos & po for (auto & p : context) context2.insert(p.at(0) == '=' ? string(p, 1) : p); - mkString(v, s, context2); + v.mkString(s, context2); } static RegisterPrimOp primop_unsafeDiscardOutputDependency("__unsafeDiscardOutputDependency", 1, prim_unsafeDiscardOutputDependency); @@ -186,7 +185,7 @@ static void prim_appendContext(EvalState & state, const Pos & pos, Value * * arg } } - mkString(v, orig, context); + v.mkString(orig, context); } static RegisterPrimOp primop_appendContext("__appendContext", 2, prim_appendContext); diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index a97bfcae7..6647bd35c 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -247,7 +247,7 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, state.allowPath(storePath); auto path = state.store->printStorePath(storePath); - mkString(v, path, PathSet({path})); + v.mkString(path, PathSet({path})); } static void prim_fetchurl(EvalState & state, const Pos & pos, Value * * args, Value & v) diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index a328b6736..bae0189ab 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -52,7 +52,7 @@ static void prim_fromTOML(EvalState & state, const Pos & pos, Value * * args, Va mkFloat(v, toml::get(t)); break;; case toml::value_t::string: - mkString(v, toml::get(t)); + v.mkString(toml::get(t)); break;; case toml::value_t::local_datetime: case toml::value_t::offset_datetime: diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index a8fea02dc..94a17136f 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -424,9 +424,6 @@ static inline void mkString(Value & v, const Symbol & s) } -void mkString(Value & v, const char * s); - - void mkPath(Value & v, const char * s); diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 171f1c440..7393d91f9 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -678,8 +678,8 @@ static void opUpgrade(Globals & globals, Strings opFlags, Strings opArgs) static void setMetaFlag(EvalState & state, DrvInfo & drv, const string & name, const string & value) { - Value * v = state.allocValue(); - mkString(*v, value.c_str()); + auto v = state.allocValue(); + v->mkString(value); drv.setMeta(name, v); } From 263a8d293c67488065296737cdadee3d0e122bf5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 18:40:39 +0100 Subject: [PATCH 3/7] Remove non-method mk functions --- src/libexpr/eval.cc | 26 +++---- src/libexpr/json-to-value.cc | 27 ++++---- src/libexpr/nixexpr.hh | 4 +- src/libexpr/primops.cc | 118 ++++++++++++++++---------------- src/libexpr/primops/context.cc | 6 +- src/libexpr/primops/fromTOML.cc | 8 +-- src/libexpr/value.hh | 25 ------- src/nix-env/nix-env.cc | 2 +- tests/plugins/plugintest.cc | 4 +- 9 files changed, 99 insertions(+), 121 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 2ac872745..591aa6583 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -613,7 +613,7 @@ Value * EvalState::addPrimOp(const string & name, auto vPrimOp = allocValue(); vPrimOp->mkPrimOp(new PrimOp { .fun = primOp, .arity = 1, .name = sym }); Value v; - mkApp(v, *vPrimOp, *vPrimOp); + v.mkApp(vPrimOp, vPrimOp); return addConstant(name, v); } @@ -635,7 +635,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp) auto vPrimOp = allocValue(); vPrimOp->mkPrimOp(new PrimOp(std::move(primOp))); Value v; - mkApp(v, *vPrimOp, *vPrimOp); + v.mkApp(vPrimOp, vPrimOp); return addConstant(primOp.name, v); } @@ -887,7 +887,7 @@ void EvalState::mkPos(Value & v, ptr pos) attrs.alloc(sColumn).mkInt(pos->column); v.mkAttrs(attrs); } else - mkNull(v); + v.mkNull(); } @@ -1258,14 +1258,14 @@ void ExprOpHasAttr::eval(EvalState & state, Env & env, Value & v) if (vAttrs->type() != nAttrs || (j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) { - mkBool(v, false); + v.mkBool(false); return; } else { vAttrs = j->value; } } - mkBool(v, true); + v.mkBool(true); } @@ -1544,7 +1544,7 @@ void ExprAssert::eval(EvalState & state, Env & env, Value & v) void ExprOpNot::eval(EvalState & state, Env & env, Value & v) { - mkBool(v, !state.evalBool(env, e)); + v.mkBool(!state.evalBool(env, e)); } @@ -1552,7 +1552,7 @@ void ExprOpEq::eval(EvalState & state, Env & env, Value & v) { Value v1; e1->eval(state, env, v1); Value v2; e2->eval(state, env, v2); - mkBool(v, state.eqValues(v1, v2)); + v.mkBool(state.eqValues(v1, v2)); } @@ -1560,25 +1560,25 @@ void ExprOpNEq::eval(EvalState & state, Env & env, Value & v) { Value v1; e1->eval(state, env, v1); Value v2; e2->eval(state, env, v2); - mkBool(v, !state.eqValues(v1, v2)); + v.mkBool(!state.eqValues(v1, v2)); } void ExprOpAnd::eval(EvalState & state, Env & env, Value & v) { - mkBool(v, state.evalBool(env, e1, pos) && state.evalBool(env, e2, pos)); + v.mkBool(state.evalBool(env, e1, pos) && state.evalBool(env, e2, pos)); } void ExprOpOr::eval(EvalState & state, Env & env, Value & v) { - mkBool(v, state.evalBool(env, e1, pos) || state.evalBool(env, e2, pos)); + v.mkBool(state.evalBool(env, e1, pos) || state.evalBool(env, e2, pos)); } void ExprOpImpl::eval(EvalState & state, Env & env, Value & v) { - mkBool(v, !state.evalBool(env, e1, pos) || state.evalBool(env, e2, pos)); + v.mkBool(!state.evalBool(env, e1, pos) || state.evalBool(env, e2, pos)); } @@ -1705,9 +1705,9 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) } if (firstType == nInt) - mkInt(v, n); + v.mkInt(n); else if (firstType == nFloat) - mkFloat(v, nf); + v.mkFloat(nf); else if (firstType == nPath) { if (!context.empty()) throwEvalError(pos, "a string that refers to a store path cannot be appended to a path"); diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 3825ca9a9..4dc41729a 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -76,39 +76,42 @@ class JSONSax : nlohmann::json_sax { EvalState & state; std::unique_ptr rs; - template inline bool handle_value(T f, Args... args) - { - f(rs->value(state), args...); - rs->add(); - return true; - } - public: JSONSax(EvalState & state, Value & v) : state(state), rs(new JSONState(&v)) {}; bool null() { - return handle_value(mkNull); + rs->value(state).mkNull(); + rs->add(); + return true; } bool boolean(bool val) { - return handle_value(mkBool, val); + rs->value(state).mkBool(val); + rs->add(); + return true; } bool number_integer(number_integer_t val) { - return handle_value(mkInt, val); + rs->value(state).mkInt(val); + rs->add(); + return true; } bool number_unsigned(number_unsigned_t val) { - return handle_value(mkInt, val); + rs->value(state).mkInt(val); + rs->add(); + return true; } bool number_float(number_float_t val, const string_t & s) { - return handle_value(mkFloat, val); + rs->value(state).mkFloat(val); + rs->add(); + return true; } bool string(string_t & val) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index b328b3941..aa5a3f332 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -94,7 +94,7 @@ struct ExprInt : Expr { NixInt n; Value v; - ExprInt(NixInt n) : n(n) { mkInt(v, n); }; + ExprInt(NixInt n) : n(n) { v.mkInt(n); }; COMMON_METHODS Value * maybeThunk(EvalState & state, Env & env); }; @@ -103,7 +103,7 @@ struct ExprFloat : Expr { NixFloat nf; Value v; - ExprFloat(NixFloat nf) : nf(nf) { mkFloat(v, nf); }; + ExprFloat(NixFloat nf) : nf(nf) { v.mkFloat(nf); }; COMMON_METHODS Value * maybeThunk(EvalState & state, Env & env); }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index cb18fe494..57be5ac23 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -196,7 +196,7 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS } state.forceFunction(**state.vImportedDrvToDerivation, pos); - mkApp(v, **state.vImportedDrvToDerivation, *w); + v.mkApp(*state.vImportedDrvToDerivation, w); state.forceAttrs(v, pos); } @@ -430,7 +430,7 @@ static RegisterPrimOp primop_typeOf({ static void prim_isNull(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nNull); + v.mkBool(args[0]->type() == nNull); } static RegisterPrimOp primop_isNull({ @@ -450,7 +450,7 @@ static RegisterPrimOp primop_isNull({ static void prim_isFunction(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nFunction); + v.mkBool(args[0]->type() == nFunction); } static RegisterPrimOp primop_isFunction({ @@ -466,7 +466,7 @@ static RegisterPrimOp primop_isFunction({ static void prim_isInt(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nInt); + v.mkBool(args[0]->type() == nInt); } static RegisterPrimOp primop_isInt({ @@ -482,7 +482,7 @@ static RegisterPrimOp primop_isInt({ static void prim_isFloat(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nFloat); + v.mkBool(args[0]->type() == nFloat); } static RegisterPrimOp primop_isFloat({ @@ -498,7 +498,7 @@ static RegisterPrimOp primop_isFloat({ static void prim_isString(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nString); + v.mkBool(args[0]->type() == nString); } static RegisterPrimOp primop_isString({ @@ -514,7 +514,7 @@ static RegisterPrimOp primop_isString({ static void prim_isBool(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nBool); + v.mkBool(args[0]->type() == nBool); } static RegisterPrimOp primop_isBool({ @@ -530,7 +530,7 @@ static RegisterPrimOp primop_isBool({ static void prim_isPath(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nPath); + v.mkBool(args[0]->type() == nPath); } static RegisterPrimOp primop_isPath({ @@ -685,7 +685,7 @@ static void prim_genericClosure(EvalState & state, const Pos & pos, Value * * ar /* Call the `operator' function with `e' as argument. */ Value call; - mkApp(call, *op->value, *e); + call.mkApp(op->value, e); state.forceList(call, pos); /* Add the values returned by the operator to the work set. */ @@ -761,7 +761,7 @@ static RegisterPrimOp primop_addErrorContext(RegisterPrimOp::Info { static void prim_ceil(EvalState & state, const Pos & pos, Value * * args, Value & v) { auto value = state.forceFloat(*args[0], args[0]->determinePos(pos)); - mkInt(v, ceil(value)); + v.mkInt(ceil(value)); } static RegisterPrimOp primop_ceil({ @@ -780,7 +780,7 @@ static RegisterPrimOp primop_ceil({ static void prim_floor(EvalState & state, const Pos & pos, Value * * args, Value & v) { auto value = state.forceFloat(*args[0], args[0]->determinePos(pos)); - mkInt(v, floor(value)); + v.mkInt(floor(value)); } static RegisterPrimOp primop_floor({ @@ -804,10 +804,10 @@ static void prim_tryEval(EvalState & state, const Pos & pos, Value * * args, Val try { state.forceValue(*args[0], pos); attrs.insert(state.sValue, args[0]); - mkBool(attrs.alloc("success"), true); + attrs.alloc("success").mkBool(true); } catch (AssertionError & e) { - mkBool(attrs.alloc(state.sValue), false); - mkBool(attrs.alloc("success"), false); + attrs.alloc(state.sValue).mkBool(false); + attrs.alloc("success").mkBool(false); } v.mkAttrs(attrs); } @@ -1395,13 +1395,13 @@ static void prim_pathExists(EvalState & state, const Pos & pos, Value * * args, } try { - mkBool(v, pathExists(state.checkSourcePath(path))); + v.mkBool(pathExists(state.checkSourcePath(path))); } catch (SysError & e) { /* Don't give away info from errors while canonicalising ‘path’ in restricted mode. */ - mkBool(v, false); + v.mkBool(false); } catch (RestrictedPathError & e) { - mkBool(v, false); + v.mkBool(false); } } @@ -2213,7 +2213,7 @@ static void prim_unsafeGetAttrPos(EvalState & state, const Pos & pos, Value * * state.forceAttrs(*args[1], pos); Bindings::iterator i = args[1]->attrs->find(state.symbols.create(attr)); if (i == args[1]->attrs->end()) - mkNull(v); + v.mkNull(); else state.mkPos(v, i->pos); } @@ -2229,7 +2229,7 @@ static void prim_hasAttr(EvalState & state, const Pos & pos, Value * * args, Val { string attr = state.forceStringNoCtx(*args[0], pos); state.forceAttrs(*args[1], pos); - mkBool(v, args[1]->attrs->find(state.symbols.create(attr)) != args[1]->attrs->end()); + v.mkBool(args[1]->attrs->find(state.symbols.create(attr)) != args[1]->attrs->end()); } static RegisterPrimOp primop_hasAttr({ @@ -2247,7 +2247,7 @@ static RegisterPrimOp primop_hasAttr({ static void prim_isAttrs(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nAttrs); + v.mkBool(args[0]->type() == nAttrs); } static RegisterPrimOp primop_isAttrs({ @@ -2446,7 +2446,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args auto attrs = state.buildBindings(args[0]->lambda.fun->formals->formals.size()); for (auto & i : args[0]->lambda.fun->formals->formals) // !!! should optimise booleans (allocate only once) - mkBool(attrs.alloc(i.name, ptr(&i.pos)), i.def); + attrs.alloc(i.name, ptr(&i.pos)).mkBool(i.def); v.mkAttrs(attrs); } @@ -2478,8 +2478,8 @@ static void prim_mapAttrs(EvalState & state, const Pos & pos, Value * * args, Va Value * vName = state.allocValue(); Value * vFun2 = state.allocValue(); mkString(*vName, i.name); - mkApp(*vFun2, *args[0], *vName); - mkApp(*state.allocAttr(v, i.name), *vFun2, *i.value); + vFun2->mkApp(args[0], vName); + state.allocAttr(v, i.name)->mkApp(vFun2, i.value); } } @@ -2542,10 +2542,10 @@ static void prim_zipAttrsWith(EvalState & state, const Pos & pos, Value * * args for (auto & attr : *v.attrs) { Value * name = state.allocValue(); mkString(*name, attr.name); - Value * call1 = state.allocValue(); - mkApp(*call1, *args[0], *name); - Value * call2 = state.allocValue(); - mkApp(*call2, *call1, *attr.value); + auto call1 = state.allocValue(); + call1->mkApp(args[0], name); + auto call2 = state.allocValue(); + call2->mkApp(call1, attr.value); attr.value = call2; } } @@ -2592,7 +2592,7 @@ static RegisterPrimOp primop_zipAttrsWith({ static void prim_isList(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); - mkBool(v, args[0]->type() == nList); + v.mkBool(args[0]->type() == nList); } static RegisterPrimOp primop_isList({ @@ -2690,8 +2690,8 @@ static void prim_map(EvalState & state, const Pos & pos, Value * * args, Value & state.mkList(v, args[1]->listSize()); for (unsigned int n = 0; n < v.listSize(); ++n) - mkApp(*(v.listElems()[n] = state.allocValue()), - *args[0], *args[1]->listElems()[n]); + (v.listElems()[n] = state.allocValue())->mkApp( + args[0], args[1]->listElems()[n]); } static RegisterPrimOp primop_map({ @@ -2760,7 +2760,7 @@ static void prim_elem(EvalState & state, const Pos & pos, Value * * args, Value res = true; break; } - mkBool(v, res); + v.mkBool(res); } static RegisterPrimOp primop_elem({ @@ -2793,7 +2793,7 @@ static RegisterPrimOp primop_concatLists({ static void prim_length(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceList(*args[0], pos); - mkInt(v, args[0]->listSize()); + v.mkInt(args[0]->listSize()); } static RegisterPrimOp primop_length({ @@ -2850,12 +2850,12 @@ static void anyOrAll(bool any, EvalState & state, const Pos & pos, Value * * arg state.callFunction(*args[0], *elem, vTmp, pos); bool res = state.forceBool(vTmp, pos); if (res == any) { - mkBool(v, any); + v.mkBool(any); return; } } - mkBool(v, !any); + v.mkBool(!any); } @@ -2902,9 +2902,9 @@ static void prim_genList(EvalState & state, const Pos & pos, Value * * args, Val state.mkList(v, len); for (unsigned int n = 0; n < (unsigned int) len; ++n) { - Value * arg = state.allocValue(); - mkInt(*arg, n); - mkApp(*(v.listElems()[n] = state.allocValue()), *args[0], *arg); + auto arg = state.allocValue(); + arg->mkInt(n); + (v.listElems()[n] = state.allocValue())->mkApp(args[0], arg); } } @@ -3140,9 +3140,9 @@ static void prim_add(EvalState & state, const Pos & pos, Value * * args, Value & state.forceValue(*args[0], pos); state.forceValue(*args[1], pos); if (args[0]->type() == nFloat || args[1]->type() == nFloat) - mkFloat(v, state.forceFloat(*args[0], pos) + state.forceFloat(*args[1], pos)); + v.mkFloat(state.forceFloat(*args[0], pos) + state.forceFloat(*args[1], pos)); else - mkInt(v, state.forceInt(*args[0], pos) + state.forceInt(*args[1], pos)); + v.mkInt(state.forceInt(*args[0], pos) + state.forceInt(*args[1], pos)); } static RegisterPrimOp primop_add({ @@ -3159,9 +3159,9 @@ static void prim_sub(EvalState & state, const Pos & pos, Value * * args, Value & state.forceValue(*args[0], pos); state.forceValue(*args[1], pos); if (args[0]->type() == nFloat || args[1]->type() == nFloat) - mkFloat(v, state.forceFloat(*args[0], pos) - state.forceFloat(*args[1], pos)); + v.mkFloat(state.forceFloat(*args[0], pos) - state.forceFloat(*args[1], pos)); else - mkInt(v, state.forceInt(*args[0], pos) - state.forceInt(*args[1], pos)); + v.mkInt(state.forceInt(*args[0], pos) - state.forceInt(*args[1], pos)); } static RegisterPrimOp primop_sub({ @@ -3178,9 +3178,9 @@ static void prim_mul(EvalState & state, const Pos & pos, Value * * args, Value & state.forceValue(*args[0], pos); state.forceValue(*args[1], pos); if (args[0]->type() == nFloat || args[1]->type() == nFloat) - mkFloat(v, state.forceFloat(*args[0], pos) * state.forceFloat(*args[1], pos)); + v.mkFloat(state.forceFloat(*args[0], pos) * state.forceFloat(*args[1], pos)); else - mkInt(v, state.forceInt(*args[0], pos) * state.forceInt(*args[1], pos)); + v.mkInt(state.forceInt(*args[0], pos) * state.forceInt(*args[1], pos)); } static RegisterPrimOp primop_mul({ @@ -3205,7 +3205,7 @@ static void prim_div(EvalState & state, const Pos & pos, Value * * args, Value & }); if (args[0]->type() == nFloat || args[1]->type() == nFloat) { - mkFloat(v, state.forceFloat(*args[0], pos) / state.forceFloat(*args[1], pos)); + v.mkFloat(state.forceFloat(*args[0], pos) / state.forceFloat(*args[1], pos)); } else { NixInt i1 = state.forceInt(*args[0], pos); NixInt i2 = state.forceInt(*args[1], pos); @@ -3216,7 +3216,7 @@ static void prim_div(EvalState & state, const Pos & pos, Value * * args, Value & .errPos = pos }); - mkInt(v, i1 / i2); + v.mkInt(i1 / i2); } } @@ -3231,7 +3231,7 @@ static RegisterPrimOp primop_div({ static void prim_bitAnd(EvalState & state, const Pos & pos, Value * * args, Value & v) { - mkInt(v, state.forceInt(*args[0], pos) & state.forceInt(*args[1], pos)); + v.mkInt(state.forceInt(*args[0], pos) & state.forceInt(*args[1], pos)); } static RegisterPrimOp primop_bitAnd({ @@ -3245,7 +3245,7 @@ static RegisterPrimOp primop_bitAnd({ static void prim_bitOr(EvalState & state, const Pos & pos, Value * * args, Value & v) { - mkInt(v, state.forceInt(*args[0], pos) | state.forceInt(*args[1], pos)); + v.mkInt(state.forceInt(*args[0], pos) | state.forceInt(*args[1], pos)); } static RegisterPrimOp primop_bitOr({ @@ -3259,7 +3259,7 @@ static RegisterPrimOp primop_bitOr({ static void prim_bitXor(EvalState & state, const Pos & pos, Value * * args, Value & v) { - mkInt(v, state.forceInt(*args[0], pos) ^ state.forceInt(*args[1], pos)); + v.mkInt(state.forceInt(*args[0], pos) ^ state.forceInt(*args[1], pos)); } static RegisterPrimOp primop_bitXor({ @@ -3276,7 +3276,7 @@ static void prim_lessThan(EvalState & state, const Pos & pos, Value * * args, Va state.forceValue(*args[0], pos); state.forceValue(*args[1], pos); CompareValues comp{state}; - mkBool(v, comp(args[0], args[1])); + v.mkBool(comp(args[0], args[1])); } static RegisterPrimOp primop_lessThan({ @@ -3374,7 +3374,7 @@ static void prim_stringLength(EvalState & state, const Pos & pos, Value * * args { PathSet context; string s = state.coerceToString(pos, *args[0], context); - mkInt(v, s.size()); + v.mkInt(s.size()); } static RegisterPrimOp primop_stringLength({ @@ -3440,7 +3440,7 @@ void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) std::smatch match; if (!std::regex_match(str, match, regex->second)) { - mkNull(v); + v.mkNull(); return; } @@ -3449,7 +3449,7 @@ void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) state.mkList(v, len); for (size_t i = 0; i < len; ++i) { if (!match[i+1].matched) - mkNull(*(v.listElems()[i] = state.allocValue())); + (v.listElems()[i] = state.allocValue())->mkNull(); else (v.listElems()[i] = state.allocValue())->mkString(match[i + 1].str()); } @@ -3547,7 +3547,7 @@ static void prim_split(EvalState & state, const Pos & pos, Value * * args, Value state.mkList(*elem, slen); for (size_t si = 0; si < slen; ++si) { if (!match[si + 1].matched) - mkNull(*(elem->listElems()[si] = state.allocValue())); + (elem->listElems()[si] = state.allocValue())->mkNull(); else (elem->listElems()[si] = state.allocValue())->mkString(match[si + 1].str()); } @@ -3750,7 +3750,7 @@ static void prim_compareVersions(EvalState & state, const Pos & pos, Value * * a { string version1 = state.forceStringNoCtx(*args[0], pos); string version2 = state.forceStringNoCtx(*args[1], pos); - mkInt(v, compareVersions(version1, version2)); + v.mkInt(compareVersions(version1, version2)); } static RegisterPrimOp primop_compareVersions({ @@ -3832,17 +3832,17 @@ void EvalState::createBaseEnv() mkAttrs(v, 128); addConstant("builtins", v); - mkBool(v, true); + v.mkBool(true); addConstant("true", v); - mkBool(v, false); + v.mkBool(false); addConstant("false", v); - mkNull(v); + v.mkNull(); addConstant("null", v); if (!evalSettings.pureEval) { - mkInt(v, time(0)); + v.mkInt(time(0)); addConstant("__currentTime", v); v.mkString(settings.thisSystem.get()); @@ -3859,7 +3859,7 @@ void EvalState::createBaseEnv() language feature gets added. It's not necessary to increase it when primops get added, because you can just use `builtins ? primOp' to check. */ - mkInt(v, 6); + v.mkInt(6); addConstant("__langVersion", v); // Miscellaneous diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 321c3c301..a239c06da 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -17,7 +17,7 @@ static void prim_hasContext(EvalState & state, const Pos & pos, Value * * args, { PathSet context; state.forceString(*args[0], context, pos); - mkBool(v, !context.empty()); + v.mkBool(!context.empty()); } static RegisterPrimOp primop_hasContext("__hasContext", 1, prim_hasContext); @@ -109,9 +109,9 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, for (const auto & info : contextInfos) { auto infoAttrs = state.buildBindings(3); if (info.second.path) - mkBool(infoAttrs.alloc(sPath), true); + infoAttrs.alloc(sPath).mkBool(true); if (info.second.allOutputs) - mkBool(infoAttrs.alloc(sAllOutputs), true); + infoAttrs.alloc(sAllOutputs).mkBool(true); if (!info.second.outputs.empty()) { auto & outputsVal = infoAttrs.alloc(state.sOutputs); state.mkList(outputsVal, info.second.outputs.size()); diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index bae0189ab..80c7e0b82 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -43,13 +43,13 @@ static void prim_fromTOML(EvalState & state, const Pos & pos, Value * * args, Va } break;; case toml::value_t::boolean: - mkBool(v, toml::get(t)); + v.mkBool(toml::get(t)); break;; case toml::value_t::integer: - mkInt(v, toml::get(t)); + v.mkInt(toml::get(t)); break;; case toml::value_t::floating: - mkFloat(v, toml::get(t)); + v.mkFloat(toml::get(t)); break;; case toml::value_t::string: v.mkString(toml::get(t)); @@ -62,7 +62,7 @@ static void prim_fromTOML(EvalState & state, const Pos & pos, Value * * args, Va throw std::runtime_error("Dates and times are not supported"); break;; case toml::value_t::empty: - mkNull(v); + v.mkNull(); break;; } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 94a17136f..3f9b65197 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -393,31 +393,6 @@ public: // TODO: Remove these static functions, replace call sites with v.mk* instead -static inline void mkInt(Value & v, NixInt n) -{ - v.mkInt(n); -} - -static inline void mkFloat(Value & v, NixFloat n) -{ - v.mkFloat(n); -} - -static inline void mkBool(Value & v, bool b) -{ - v.mkBool(b); -} - -static inline void mkNull(Value & v) -{ - v.mkNull(); -} - -static inline void mkApp(Value & v, Value & left, Value & right) -{ - v.mkApp(&left, &right); -} - static inline void mkString(Value & v, const Symbol & s) { v.mkString(((const string &) s).c_str()); diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 7393d91f9..f64075567 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -408,7 +408,7 @@ static void queryInstSources(EvalState & state, Expr * eFun = state.parseExprFromString(i, absPath(".")); Value vFun, vTmp; state.eval(eFun, vFun); - mkApp(vTmp, vFun, vArg); + vTmp.mkApp(&vFun, &vArg); getDerivations(state, vTmp, "", *instSource.autoArgs, elems, true); } diff --git a/tests/plugins/plugintest.cc b/tests/plugins/plugintest.cc index c085d3329..cd7c9f8b1 100644 --- a/tests/plugins/plugintest.cc +++ b/tests/plugins/plugintest.cc @@ -16,9 +16,9 @@ static GlobalConfig::Register rs(&mySettings); static void prim_anotherNull (EvalState & state, const Pos & pos, Value ** args, Value & v) { if (mySettings.settingSet) - mkNull(v); + v.mkNull(); else - mkBool(v, false); + v.mkBool(false); } static RegisterPrimOp rp("anotherNull", 0, prim_anotherNull); From ed93aec3c317e5dc3748b28e451c8eb6df377060 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 18:45:16 +0100 Subject: [PATCH 4/7] Remove non-method mkPath() --- src/libexpr/eval.cc | 7 +++---- src/libexpr/primops.cc | 4 ++-- src/libexpr/value.hh | 5 ++--- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 591aa6583..cd7d19747 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -786,9 +786,9 @@ void Value::mkString(std::string_view s, const PathSet & context) } -void mkPath(Value & v, const char * s) +void Value::mkPath(std::string_view s) { - v.mkPath(dupString(s)); + mkPath(dupStringWithLen(s.data(), s.size())); } @@ -1711,8 +1711,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) else if (firstType == nPath) { if (!context.empty()) throwEvalError(pos, "a string that refers to a store path cannot be appended to a path"); - auto path = canonPath(s.str()); - mkPath(v, path.c_str()); + v.mkPath(canonPath(s.str())); } else v.mkString(s.str(), context); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 57be5ac23..650200ae4 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1441,7 +1441,7 @@ static void prim_dirOf(EvalState & state, const Pos & pos, Value * * args, Value { PathSet context; Path dir = dirOf(state.coerceToString(pos, *args[0], context, false, false)); - if (args[0]->type() == nPath) mkPath(v, dir.c_str()); else v.mkString(dir, context); + if (args[0]->type() == nPath) v.mkPath(dir); else v.mkString(dir, context); } static RegisterPrimOp primop_dirOf({ @@ -1522,7 +1522,7 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va string path = state.forceStringNoCtx(*args[1], pos); - mkPath(v, state.checkSourcePath(state.findFile(searchPath, path, pos)).c_str()); + v.mkPath(state.checkSourcePath(state.findFile(searchPath, path, pos))); } static RegisterPrimOp primop_findFile(RegisterPrimOp::Info { diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 3f9b65197..ba647a10d 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -248,6 +248,8 @@ public: path = s; } + void mkPath(std::string_view s); + inline void mkNull() { clearValue(); @@ -399,9 +401,6 @@ static inline void mkString(Value & v, const Symbol & s) } -void mkPath(Value & v, const char * s); - - #if HAVE_BOEHMGC typedef std::vector > ValueVector; typedef std::map, traceable_allocator > > ValueMap; From ca5baf239277daecef489a415fe157094bd7e799 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 19:09:40 +0100 Subject: [PATCH 5/7] Turn mkString(Symbol) into a method --- src/libexpr/nixexpr.hh | 2 +- src/libexpr/primops.cc | 10 +++++----- src/libexpr/value.hh | 13 +++++-------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index aa5a3f332..0a60057e5 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -112,7 +112,7 @@ struct ExprString : Expr { Symbol s; Value v; - ExprString(const Symbol & s) : s(s) { mkString(v, s); }; + ExprString(const Symbol & s) : s(s) { v.mkString(s); }; COMMON_METHODS Value * maybeThunk(EvalState & state, Env & env); }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 650200ae4..879dfa0a3 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -412,7 +412,7 @@ static void prim_typeOf(EvalState & state, const Pos & pos, Value * * args, Valu case nFloat: t = "float"; break; case nThunk: abort(); } - mkString(v, state.symbols.create(t)); + v.mkString(state.symbols.create(t)); } static RegisterPrimOp primop_typeOf({ @@ -2129,7 +2129,7 @@ static void prim_attrNames(EvalState & state, const Pos & pos, Value * * args, V size_t n = 0; for (auto & i : *args[0]->attrs) - mkString(*(v.listElems()[n++] = state.allocValue()), i.name); + (v.listElems()[n++] = state.allocValue())->mkString(i.name); std::sort(v.listElems(), v.listElems() + n, [](Value * v1, Value * v2) { return strcmp(v1->string.s, v2->string.s) < 0; }); @@ -2477,7 +2477,7 @@ static void prim_mapAttrs(EvalState & state, const Pos & pos, Value * * args, Va for (auto & i : *args[1]->attrs) { Value * vName = state.allocValue(); Value * vFun2 = state.allocValue(); - mkString(*vName, i.name); + vName->mkString(i.name); vFun2->mkApp(args[0], vName); state.allocAttr(v, i.name)->mkApp(vFun2, i.value); } @@ -2540,8 +2540,8 @@ static void prim_zipAttrsWith(EvalState & state, const Pos & pos, Value * * args } for (auto & attr : *v.attrs) { - Value * name = state.allocValue(); - mkString(*name, attr.name); + auto name = state.allocValue(); + name->mkString(attr.name); auto call1 = state.allocValue(); call1->mkApp(args[0], name); auto call2 = state.allocValue(); diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index ba647a10d..7d007ebdc 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -241,6 +241,11 @@ public: void mkString(std::string_view s, const PathSet & context); + inline void mkString(const Symbol & s) + { + mkString(((const std::string &) s).c_str()); + } + inline void mkPath(const char * s) { clearValue(); @@ -393,14 +398,6 @@ public: }; - -// TODO: Remove these static functions, replace call sites with v.mk* instead -static inline void mkString(Value & v, const Symbol & s) -{ - v.mkString(((const string &) s).c_str()); -} - - #if HAVE_BOEHMGC typedef std::vector > ValueVector; typedef std::map, traceable_allocator > > ValueMap; From 17daec0b832205df781f6443292667c9aa617047 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 19:23:11 +0100 Subject: [PATCH 6/7] Move empty attrset optimisation --- src/libexpr/attr-set.cc | 8 +++----- src/libexpr/eval.cc | 3 +-- src/libexpr/eval.hh | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index d74243f45..fc971957d 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -12,6 +12,8 @@ namespace nix { structure. */ Bindings * EvalState::allocBindings(size_t capacity) { + if (capacity == 0) + return &emptyBindings; if (capacity > std::numeric_limits::max()) throw Error("attribute set of size %d is too big", capacity); return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity); @@ -20,10 +22,6 @@ Bindings * EvalState::allocBindings(size_t capacity) void EvalState::mkAttrs(Value & v, size_t capacity) { - if (capacity == 0) { - v = vEmptySet; - return; - } v.mkAttrs(allocBindings(capacity)); nrAttrsets++; nrAttrsInAttrsets += capacity; @@ -63,7 +61,7 @@ Value & BindingsBuilder::alloc(std::string_view name, ptr pos) void Bindings::sort() { - std::sort(begin(), end()); + if (size_) std::sort(begin(), end()); } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index cd7d19747..112690189 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -413,6 +413,7 @@ EvalState::EvalState( , sSelf(symbols.create("self")) , sEpsilon(symbols.create("")) , repair(NoRepair) + , emptyBindings(0) , store(store) , buildStore(buildStore ? buildStore : store) , regexCache(makeRegexCache()) @@ -454,8 +455,6 @@ EvalState::EvalState( } } - vEmptySet.mkAttrs(allocBindings(0)); - createBaseEnv(); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index c7f74d7b7..c0d88201a 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -91,7 +91,7 @@ public: mode. */ std::optional allowedPaths; - Value vEmptySet; + Bindings emptyBindings; /* Store used to materialise .drv files. */ const ref store; From 2b4c94482364c236b03f7f8b61aca7076ab4079d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 20:29:17 +0100 Subject: [PATCH 7/7] Remove EvalState::mkAttrs() --- src/libexpr/attr-set.cc | 13 +++-------- src/libexpr/attr-set.hh | 10 +++++++-- src/libexpr/eval.cc | 20 +++++++++-------- src/libexpr/eval.hh | 3 ++- src/libexpr/json-to-value.cc | 6 +++--- src/libexpr/primops.cc | 42 +++++++++++++++++++++--------------- 6 files changed, 52 insertions(+), 42 deletions(-) diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index fc971957d..52ac47e9b 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -7,6 +7,7 @@ namespace nix { + /* Allocate a new array of attributes for an attribute set with a specific capacity. The space is implicitly reserved after the Bindings structure. */ @@ -16,15 +17,9 @@ Bindings * EvalState::allocBindings(size_t capacity) return &emptyBindings; if (capacity > std::numeric_limits::max()) throw Error("attribute set of size %d is too big", capacity); - return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity); -} - - -void EvalState::mkAttrs(Value & v, size_t capacity) -{ - v.mkAttrs(allocBindings(capacity)); nrAttrsets++; nrAttrsInAttrsets += capacity; + return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity); } @@ -67,9 +62,7 @@ void Bindings::sort() Value & Value::mkAttrs(BindingsBuilder & bindings) { - clearValue(); - internalType = tAttrs; - attrs = bindings.finish(); + mkAttrs(bindings.finish()); return *this; } diff --git a/src/libexpr/attr-set.hh b/src/libexpr/attr-set.hh index 903289b69..82c348287 100644 --- a/src/libexpr/attr-set.hh +++ b/src/libexpr/attr-set.hh @@ -118,13 +118,14 @@ public: call finish(), which sorts the bindings. */ class BindingsBuilder { - EvalState & state; Bindings * bindings; public: + EvalState & state; + BindingsBuilder(EvalState & state, Bindings * bindings) - : state(state), bindings(bindings) + : bindings(bindings), state(state) { } void insert(Symbol name, Value * value, ptr pos = ptr(&noPos)) @@ -146,6 +147,11 @@ public: bindings->sort(); return bindings; } + + Bindings * alreadySorted() + { + return bindings; + } }; } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 112690189..81aa86641 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -145,7 +145,7 @@ void printValue(std::ostream & str, std::set & active, const Valu str << v.fpoint; break; default: - throw Error("invalid value"); + abort(); } active.erase(&v); @@ -1065,8 +1065,8 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v) void ExprAttrs::eval(EvalState & state, Env & env, Value & v) { - state.mkAttrs(v, attrs.size() + dynamicAttrs.size()); - Env *dynamicEnv = &env; + v.mkAttrs(state.buildBindings(attrs.size() + dynamicAttrs.size()).finish()); + auto dynamicEnv = &env; if (recursive) { /* Create a new environment that contains the attributes in @@ -1592,7 +1592,7 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) if (v1.attrs->size() == 0) { v = v2; return; } if (v2.attrs->size() == 0) { v = v1; return; } - state.mkAttrs(v, v1.attrs->size() + v2.attrs->size()); + auto attrs = state.buildBindings(v1.attrs->size() + v2.attrs->size()); /* Merge the sets, preferring values from the second set. Make sure to keep the resulting vector in sorted order. */ @@ -1601,17 +1601,19 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) while (i != v1.attrs->end() && j != v2.attrs->end()) { if (i->name == j->name) { - v.attrs->push_back(*j); + attrs.insert(*j); ++i; ++j; } else if (i->name < j->name) - v.attrs->push_back(*i++); + attrs.insert(*i++); else - v.attrs->push_back(*j++); + attrs.insert(*j++); } - while (i != v1.attrs->end()) v.attrs->push_back(*i++); - while (j != v2.attrs->end()) v.attrs->push_back(*j++); + while (i != v1.attrs->end()) attrs.insert(*i++); + while (j != v2.attrs->end()) attrs.insert(*j++); + + v.mkAttrs(attrs.alreadySorted()); state.nrOpUpdateValuesCopied += v.attrs->size(); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index c0d88201a..89814785e 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -347,7 +347,6 @@ public: } void mkList(Value & v, size_t length); - void mkAttrs(Value & v, size_t capacity); void mkThunk_(Value & v, Expr * expr); void mkPos(Value & v, ptr pos); @@ -400,6 +399,8 @@ private: friend struct ExprSelect; friend void prim_getAttr(EvalState & state, const Pos & pos, Value * * args, Value & v); friend void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v); + + friend struct Value; }; diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 4dc41729a..88716250c 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -37,10 +37,10 @@ class JSONSax : nlohmann::json_sax { ValueMap attrs; std::unique_ptr resolve(EvalState & state) override { - Value & v = parent->value(state); - state.mkAttrs(v, attrs.size()); + auto attrs2 = state.buildBindings(attrs.size()); for (auto & i : attrs) - v.attrs->push_back(Attr(i.first, i.second)); + attrs2.insert(i.first, i.second); + parent->value(state).mkAttrs(attrs2.alreadySorted()); return std::move(parent); } void add() override { v = nullptr; } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 879dfa0a3..d7382af6b 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2274,11 +2274,12 @@ static void prim_removeAttrs(EvalState & state, const Pos & pos, Value * * args, /* Copy all attributes not in that set. Note that we don't need to sort v.attrs because it's a subset of an already sorted vector. */ - state.mkAttrs(v, args[0]->attrs->size()); + auto attrs = state.buildBindings(args[0]->attrs->size()); for (auto & i : *args[0]->attrs) { if (!names.count(i.name)) - v.attrs->push_back(i); + attrs.insert(i); } + v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_removeAttrs({ @@ -2369,13 +2370,15 @@ static void prim_intersectAttrs(EvalState & state, const Pos & pos, Value * * ar state.forceAttrs(*args[0], pos); state.forceAttrs(*args[1], pos); - state.mkAttrs(v, std::min(args[0]->attrs->size(), args[1]->attrs->size())); + auto attrs = state.buildBindings(std::min(args[0]->attrs->size(), args[1]->attrs->size())); for (auto & i : *args[0]->attrs) { Bindings::iterator j = args[1]->attrs->find(i.name); if (j != args[1]->attrs->end()) - v.attrs->push_back(*j); + attrs.insert(*j); } + + v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_intersectAttrs({ @@ -2429,7 +2432,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args { state.forceValue(*args[0], pos); if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) { - state.mkAttrs(v, 0); + v.mkAttrs(&state.emptyBindings); return; } if (!args[0]->isLambda()) @@ -2439,7 +2442,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args }); if (!args[0]->lambda.fun->hasFormals()) { - state.mkAttrs(v, 0); + v.mkAttrs(&state.emptyBindings); return; } @@ -2472,15 +2475,17 @@ static void prim_mapAttrs(EvalState & state, const Pos & pos, Value * * args, Va { state.forceAttrs(*args[1], pos); - state.mkAttrs(v, args[1]->attrs->size()); + auto attrs = state.buildBindings(args[1]->attrs->size()); for (auto & i : *args[1]->attrs) { Value * vName = state.allocValue(); Value * vFun2 = state.allocValue(); vName->mkString(i.name); vFun2->mkApp(args[0], vName); - state.allocAttr(v, i.name)->mkApp(vFun2, i.value); + attrs.alloc(i.name).mkApp(vFun2, i.value); } + + v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_mapAttrs({ @@ -2526,12 +2531,13 @@ static void prim_zipAttrsWith(EvalState & state, const Pos & pos, Value * * args } } - state.mkAttrs(v, attrsSeen.size()); + auto attrs = state.buildBindings(attrsSeen.size()); for (auto & [sym, elem] : attrsSeen) { - Value * list = state.allocAttr(v, sym); - state.mkList(*list, elem.first); - elem.second = list->listElems(); + 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]; @@ -3054,14 +3060,16 @@ static void prim_groupBy(EvalState & state, const Pos & pos, Value * * args, Val vector->second.push_back(vElem); } - state.mkAttrs(v, attrs.size()); + auto attrs2 = state.buildBindings(attrs.size()); for (auto & i : attrs) { - Value * list = state.allocAttr(v, i.first); + auto & list = attrs2.alloc(i.first); auto size = i.second.size(); - state.mkList(*list, size); - memcpy(list->listElems(), i.second.data(), sizeof(Value *) * size); + state.mkList(list, size); + memcpy(list.listElems(), i.second.data(), sizeof(Value *) * size); } + + v.mkAttrs(attrs2.alreadySorted()); } static RegisterPrimOp primop_groupBy({ @@ -3829,7 +3837,7 @@ void EvalState::createBaseEnv() Value v; /* `builtins' must be first! */ - mkAttrs(v, 128); + v.mkAttrs(buildBindings(128).finish()); addConstant("builtins", v); v.mkBool(true);