From 51c6d6a2e807245def86880b5e464020ea7a187f Mon Sep 17 00:00:00 2001 From: skye Date: Sat, 28 Feb 2026 17:46:05 -0500 Subject: [PATCH] libexpr: Remove various default constructions of `Value`s Progress towards #744 Change-Id: I138ecf7ab712ea570ecbf506c7b6f6be6a6a6964 --- lix/legacy/nix-instantiate.cc | 6 +----- lix/libcmd/common-eval-args.cc | 8 +++----- lix/libcmd/installables.cc | 22 +++++++++++----------- lix/libcmd/repl.cc | 10 +++------- lix/libexpr/attr-path.cc | 15 ++++++++------- lix/libexpr/attr-set.hh | 5 +++-- lix/libexpr/eval-expr.cc | 33 +++++++++++++++++---------------- lix/libexpr/eval.cc | 4 ++-- lix/libexpr/flake/flake.cc | 12 ++++-------- lix/libexpr/primops.cc | 17 ++++++++--------- 10 files changed, 60 insertions(+), 72 deletions(-) diff --git a/lix/legacy/nix-instantiate.cc b/lix/legacy/nix-instantiate.cc index 5eeb0c500..01f7d188d 100644 --- a/lix/legacy/nix-instantiate.cc +++ b/lix/legacy/nix-instantiate.cc @@ -42,11 +42,7 @@ void processExpr(EvalState & state, const Strings & attrPaths, NixStringContext context; if (evalOnly) { - Value vRes; - if (autoArgs.empty()) - vRes = v; - else - vRes = state.autoCallFunction(autoArgs, v, noPos); + Value vRes = autoArgs.empty() ? v : state.autoCallFunction(autoArgs, v, noPos); if (output == okRaw) std::cout << *state.coerceToString(noPos, vRes, context, "while generating the nix-instantiate output", StringCoercionMode::Strict); // We intentionally don't output a newline here. The default PS1 for Bash in NixOS starts with a newline diff --git a/lix/libcmd/common-eval-args.cc b/lix/libcmd/common-eval-args.cc index 143b45c7f..f6ac3ad48 100644 --- a/lix/libcmd/common-eval-args.cc +++ b/lix/libcmd/common-eval-args.cc @@ -183,11 +183,9 @@ Bindings * MixEvalArgs::getAutoArgs(Evaluator & state) { auto res = state.buildBindings(autoArgs.size()); for (auto & i : autoArgs) { - Value v; - if (i.second[0] == 'E') - v = state.evalLazily(state.parseExprFromString(i.second.substr(1), CanonPath::fromCwd())); - else - v = {NewValueAs::string, ((std::string_view) i.second).substr(1)}; + Value v = i.second[0] == 'E' + ? state.evalLazily(state.parseExprFromString(i.second.substr(1), CanonPath::fromCwd())) + : Value{NewValueAs::string, ((std::string_view) i.second).substr(1)}; res.insert(state.symbols.create(i.first), v); } return res.finish(); diff --git a/lix/libcmd/installables.cc b/lix/libcmd/installables.cc index fddc7f834..da0e53d72 100644 --- a/lix/libcmd/installables.cc +++ b/lix/libcmd/installables.cc @@ -446,18 +446,18 @@ Installables SourceExprCommand::parseInstallables( throw UsageError("'--file' and '--expr' are exclusive"); auto evaluator = getEvaluator(); - Value vFile; - if (file == "-") { - auto & e = evaluator->parseStdin(); - vFile = state.eval(e); - } - else if (file) - vFile = state.evalFile(state.aio.blockOn(lookupFileArg(*evaluator, *file)).unwrap()); - else { - auto & e = evaluator->parseExprFromString(*expr, CanonPath::fromCwd()); - vFile = state.eval(e); - } + Value vFile = [&](NeverAsync = {}) { + if (file == "-") { + auto & e = evaluator->parseStdin(); + return state.eval(e); + } else if (file) { + return state.evalFile(state.aio.blockOn(lookupFileArg(*evaluator, *file)).unwrap()); + } else { + auto & e = evaluator->parseExprFromString(*expr, CanonPath::fromCwd()); + return state.eval(e); + } + }(); for (auto & s : ss) { auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(s); diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 427ceba01..50ff1ddeb 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -1318,12 +1318,10 @@ void NixRepl::loadFlake(const std::string & flakeRefS) .kind = ReplLoadKind::Flake, }; - Value v; - try { loaded.remove(loadable); loaded.push_back(loadable); - v = flake::callFlake( + Value v = flake::callFlake( state, flake::lockFlake( state, @@ -1451,7 +1449,6 @@ Value NixRepl::getReplOverlaysEvalFunction() } auto evalReplInitFilesPath = CanonPath::root + "repl-overlays.nix"; - *replOverlaysEvalFunction = Value{}; auto code = #include "repl-overlays.nix.gen.hh" ; @@ -1461,16 +1458,15 @@ Value NixRepl::getReplOverlaysEvalFunction() evaluator.builtins.staticEnv ); - **replOverlaysEvalFunction = state.eval(expr); + *replOverlaysEvalFunction = state.eval(expr); return **replOverlaysEvalFunction; } Value NixRepl::replOverlays() { - Value replInits; auto replInitStorage = evaluator.mem.newList(evalSettings.replOverlays.get().size()); - replInits = {NewValueAs::list, replInitStorage}; + Value replInits = {NewValueAs::list, replInitStorage}; size_t i = 0; for (auto path : evalSettings.replOverlays.get()) { diff --git a/lix/libexpr/attr-path.cc b/lix/libexpr/attr-path.cc index 8f7c509c7..9051123d9 100644 --- a/lix/libexpr/attr-path.cc +++ b/lix/libexpr/attr-path.cc @@ -163,13 +163,14 @@ findAlongAttrPath(EvalState & state, const std::string & attrPath, Bindings & au std::pair findPackageFilename(EvalState & state, Value & v, std::string what) { - Value v2; - try { - auto dummyArgs = state.ctx.mem.allocBindings(0); - v2 = findAlongAttrPath(state, "meta.position", *dummyArgs, v).first; - } catch (Error &) { - throw NoPositionInfo("package '%s' has no source location information", what); - } + Value v2 = [&]() { + try { + auto dummyArgs = state.ctx.mem.allocBindings(0); + return findAlongAttrPath(state, "meta.position", *dummyArgs, v).first; + } catch (Error &) { + throw NoPositionInfo("package '%s' has no source location information", what); + } + }(); // FIXME: is it possible to extract the Pos object instead of doing this // toString + parsing? diff --git a/lix/libexpr/attr-set.hh b/lix/libexpr/attr-set.hh index 89740375b..5a9d971f9 100644 --- a/lix/libexpr/attr-set.hh +++ b/lix/libexpr/attr-set.hh @@ -72,8 +72,9 @@ public: const Attr * get(Symbol name) { - Attr key(name, {}); - iterator i = std::lower_bound(begin(), end(), key); + iterator i = std::lower_bound(begin(), end(), name, [](const Attr & value, const Symbol & compare) { + return value.name < compare; + }); if (i != end() && i->name == name) return &*i; return nullptr; } diff --git a/lix/libexpr/eval-expr.cc b/lix/libexpr/eval-expr.cc index e1bc72307..cd3a99f16 100644 --- a/lix/libexpr/eval-expr.cc +++ b/lix/libexpr/eval-expr.cc @@ -162,18 +162,18 @@ Value ExprSet::eval(EvalState & state, Env & env) * http://github.com/NixOS/nix/issues/7012. Any accesses to the output attrset will thus infrec. */ Value vBackup = v; - Value nameVal; + Symbol nameSym; { KJ_DEFER(v = vBackup); v = Value{NewValueAs::blackhole}; - nameVal = i.nameExpr->eval(state, *dynamicEnv); + Value nameVal = i.nameExpr->eval(state, *dynamicEnv); state.forceValue(nameVal, i.pos); if (nameVal.type() == nNull) { continue; } state.forceStringNoCtx(nameVal, i.pos, "while evaluating the name of a dynamic attribute"); + nameSym = state.ctx.symbols.create(nameVal.str()); } - auto nameSym = state.ctx.symbols.create(nameVal.str()); auto j = v.attrs()->get(nameSym); if (j) { state.ctx.errors @@ -617,19 +617,20 @@ Value ExprSelect::eval(EvalState & state, Env & env) // Position for the current selector in this select chain. PosIdx posCurrentSyntax; - Value baseSelectee; - try { - // Evaluate the original thing we're selecting on. - baseSelectee = e->eval(state, env); - } catch (Error & e) { - // clang-format off - e.addTrace(state.ctx.positions[getPos()], HintFmt( - "while evaluating an expression to select '%s' on it", - showAttrPath(state.ctx.symbols, attrPath) - )); - // clang-format on - throw; - } + Value baseSelectee = [&]() { + try { + // Evaluate the original thing we're selecting on. + return e->eval(state, env); + } catch (Error & e) { + // clang-format off + e.addTrace(state.ctx.positions[getPos()], HintFmt( + "while evaluating an expression to select '%s' on it", + showAttrPath(state.ctx.symbols, attrPath) + )); + // clang-format on + throw; + } + }(); try { // With the original selectee evaluated, we'll walk the selection path starting diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 6698b63fe..6a4f0b83d 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -733,13 +733,13 @@ void mapStaticEnvBindings(const SymbolTable & st, const StaticEnv & se, const En // add 'with' bindings. Bindings::iterator j = env.values[0].attrs()->begin(); while (j != env.values[0].attrs()->end()) { - vm[std::string(st[j->name])] = j->value; + vm.insert_or_assign(std::string(st[j->name]), j->value); ++j; } } else { // iterate through staticenv bindings and add them. for (auto & i : se.vars) - vm[std::string(st[i.first])] = env.values[i.second]; + vm.insert_or_assign(std::string(st[i.first]), env.values[i.second]); } } } diff --git a/lix/libexpr/flake/flake.cc b/lix/libexpr/flake/flake.cc index 19fc82c53..d4a896bba 100644 --- a/lix/libexpr/flake/flake.cc +++ b/lix/libexpr/flake/flake.cc @@ -942,10 +942,7 @@ LockedFlake lockFlake( Value callFlake(EvalState & state, const LockedFlake & lockedFlake) { - Value vLocks; - Value vRootSubdir; - - vLocks = {NewValueAs::string, lockedFlake.lockFile.to_string()}; + Value vLocks = {NewValueAs::string, lockedFlake.lockFile.to_string()}; Value vRootSrc = emitTreeAttrs( state.ctx, @@ -955,14 +952,13 @@ Value callFlake(EvalState & state, const LockedFlake & lockedFlake) lockedFlake.flake.forceDirty ); - vRootSubdir = {NewValueAs::string, lockedFlake.flake.lockedRef.subdir}; + Value vRootSubdir = {NewValueAs::string, lockedFlake.flake.lockedRef.subdir}; if (!state.ctx.caches.vCallFlake) { - state.ctx.caches.vCallFlake = allocRootValue({}); - *state.ctx.caches.vCallFlake = state.eval(state.ctx.parseExprFromString( + state.ctx.caches.vCallFlake = allocRootValue(state.eval(state.ctx.parseExprFromString( #include "call-flake.nix.gen.hh" , CanonPath::root - )); + ))); } Value vTmp1 = state.callFunction(*state.ctx.caches.vCallFlake, vLocks, noPos); diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index d3fd4a243..92f14c3bf 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -211,11 +211,11 @@ static void import(EvalState & state, Value & vPath, Value * vScope, Value & v) Value w{NewValueAs::attrs, attrs.finish()}; if (!state.ctx.caches.vImportedDrvToDerivation) { - state.ctx.caches.vImportedDrvToDerivation = allocRootValue({}); - *state.ctx.caches.vImportedDrvToDerivation = state.eval(state.ctx.parseExprFromString( + state.ctx.caches.vImportedDrvToDerivation = + allocRootValue(state.eval(state.ctx.parseExprFromString( #include "imported-drv-to-derivation.nix.gen.hh" - , CanonPath::root - )); + , CanonPath::root + ))); } state.forceFunction( @@ -1702,11 +1702,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; - if (isInDir(p, realPath)) - arg1 = {NewValueAs::string, path + "/" + std::string(p, realPath.size() + 1)}; - else - arg1 = {NewValueAs::string, p}; + Value arg1 = { + NewValueAs::string, + isInDir(p, realPath) ? path + "/" + std::string(p, realPath.size() + 1) : p + }; Value arg2 = {NewValueAs::string,