From bd383d1b6f91c4fe7ac21c52771e92027f649fa0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 4 Feb 2022 00:31:33 +0100 Subject: [PATCH] Make most calls to determinePos() lazy --- src/libcmd/installables.cc | 2 +- src/libexpr/eval-inline.hh | 26 +++++++++++++++++--------- src/libexpr/eval.cc | 14 ++++++++++---- src/libexpr/eval.hh | 10 +++++++++- src/libexpr/get-drvs.cc | 4 ++-- src/libexpr/value.hh | 2 +- src/nix-build/nix-build.cc | 2 +- src/nix-env/user-env.cc | 2 +- src/nix-instantiate/nix-instantiate.cc | 2 +- src/nix/repl.cc | 6 +++--- 10 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index ec4f8f6f9..38a177f80 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -497,7 +497,7 @@ Value * InstallableFlake::getFlakeOutputs(EvalState & state, const flake::Locked auto aOutputs = vFlake->attrs->get(state.symbols.create("outputs")); assert(aOutputs); - state.forceValue(*aOutputs->value, aOutputs->value->determinePos(noPos)); + state.forceValue(*aOutputs->value, [&]() { return aOutputs->value->determinePos(noPos); }); return aOutputs->value; } diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index 08acb0877..aef1f6351 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -15,12 +15,6 @@ LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s)) }); } -LocalNoInlineNoReturn(void throwTypeError(const char * s, const Value & v)) -{ - throw TypeError(s, showType(v)); -} - - LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const Value & v)) { throw TypeError({ @@ -31,6 +25,13 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const void EvalState::forceValue(Value & v, const Pos & pos) +{ + forceValue(v, [&]() { return pos; }); +} + + +template +void EvalState::forceValue(Value & v, Callable getPos) { if (v.isThunk()) { Env * env = v.thunk.env; @@ -47,15 +48,22 @@ void EvalState::forceValue(Value & v, const Pos & pos) else if (v.isApp()) callFunction(*v.app.left, *v.app.right, v, noPos); else if (v.isBlackhole()) - throwEvalError(pos, "infinite recursion encountered"); + throwEvalError(getPos(), "infinite recursion encountered"); } inline void EvalState::forceAttrs(Value & v, const Pos & pos) { - forceValue(v, pos); + forceAttrs(v, [&]() { return pos; }); +} + + +template +inline void EvalState::forceAttrs(Value & v, Callable getPos) +{ + forceValue(v, getPos); if (v.type() != nAttrs) - throwTypeError(pos, "value is %1% while a set was expected", v); + throwTypeError(getPos(), "value is %1% while a set was expected", v); } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index afde29d11..2d4eb57fc 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -219,7 +219,7 @@ string showType(const Value & v) } } -Pos Value::determinePos(const Pos &pos) const +Pos Value::determinePos(const Pos & pos) const { switch (internalType) { case tAttrs: return *attrs->pos; @@ -754,6 +754,11 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const }); } +LocalNoInlineNoReturn(void throwTypeError(const char * s, const Value & v)) +{ + throw TypeError(s, showType(v)); +} + LocalNoInlineNoReturn(void throwAssertionError(const Pos & pos, const char * s, const string & s1)) { throw AssertionError({ @@ -1138,7 +1143,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) Hence we need __overrides.) */ if (hasOverrides) { Value * vOverrides = (*v.attrs)[overrides->second.displ].value; - state.forceAttrs(*vOverrides, vOverrides->determinePos(noPos)); + state.forceAttrs(*vOverrides, [&]() { return vOverrides->determinePos(noPos); }); Bindings * newBnds = state.allocBindings(v.attrs->capacity() + vOverrides->attrs->size()); for (auto & i : *v.attrs) newBnds->push_back(i); @@ -1500,7 +1505,8 @@ void EvalState::incrFunctionCall(ExprLambda * fun) void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) { - Pos pos = fun.determinePos(noPos); + auto pos = fun.determinePos(noPos); + forceValue(fun, pos); if (fun.type() == nAttrs) { @@ -1797,7 +1803,7 @@ void EvalState::forceValueDeep(Value & v) recurse = [&](Value & v) { if (!seen.insert(&v).second) return; - forceValue(v, v.determinePos(noPos)); + forceValue(v, [&]() { return v.determinePos(noPos); }); if (v.type() == nAttrs) { for (auto & i : *v.attrs) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 190dd16dc..ce5a16e11 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -224,6 +224,9 @@ public: result. Otherwise, this is a no-op. */ inline void forceValue(Value & v, const Pos & pos); + template + inline void forceValue(Value & v, Callable getPos); + /* Force a value, then recursively force list elements and attributes. */ void forceValueDeep(Value & v); @@ -232,7 +235,12 @@ public: NixInt forceInt(Value & v, const Pos & pos); NixFloat forceFloat(Value & v, const Pos & pos); bool forceBool(Value & v, const Pos & pos); - inline void forceAttrs(Value & v, const Pos & pos); + + void forceAttrs(Value & v, const Pos & pos); + + template + inline void forceAttrs(Value & v, Callable getPos); + inline void forceList(Value & v, const Pos & pos); void forceFunction(Value & v, const Pos & pos); // either lambda or primop std::string_view forceString(Value & v, const Pos & pos = noPos); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 1b1cef1bf..5995a857b 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -172,7 +172,7 @@ StringSet DrvInfo::queryMetaNames() bool DrvInfo::checkMeta(Value & v) { - state->forceValue(v, v.determinePos(noPos)); + state->forceValue(v, [&]() { return v.determinePos(noPos); }); if (v.type() == nList) { for (auto elem : v.listItems()) if (!checkMeta(*elem)) return false; @@ -278,7 +278,7 @@ static bool getDerivation(EvalState & state, Value & v, bool ignoreAssertionFailures) { try { - state.forceValue(v, v.determinePos(noPos)); + state.forceValue(v, [&]() { return v.determinePos(noPos); }); if (!state.isDerivation(v)) return true; /* Remove spurious duplicates (e.g., a set like `rec { x = diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 1896c7563..bef5cd6bd 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -361,7 +361,7 @@ public: return internalType == tList1 ? 1 : internalType == tList2 ? 2 : bigList.size; } - Pos determinePos(const Pos &pos) const; + Pos determinePos(const Pos & pos) const; /* Check whether forcing this value requires a trivial amount of computation. In particular, function applications are diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index a7dce23f1..ae4746955 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -318,7 +318,7 @@ static void main_nix_build(int argc, char * * argv) for (auto & i : attrPaths) { Value & v(*findAlongAttrPath(*state, i, *autoArgs, vRoot).first); - state->forceValue(v, v.determinePos(noPos)); + state->forceValue(v, [&]() { return v.determinePos(noPos); }); getDerivations(*state, v, "", *autoArgs, drvs, false); } } diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 727338105..5842e6501 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -129,7 +129,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, /* Evaluate it. */ debug("evaluating user environment builder"); - state.forceValue(topLevel, topLevel.determinePos(noPos)); + state.forceValue(topLevel, [&]() { return topLevel.determinePos(noPos); }); PathSet context; Attr & aDrvPath(*topLevel.attrs->find(state.sDrvPath)); auto topLevelDrv = state.store->parseStorePath(state.coerceToPath(*aDrvPath.pos, *aDrvPath.value, context)); diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index 8836f810c..a08683be1 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -40,7 +40,7 @@ void processExpr(EvalState & state, const Strings & attrPaths, for (auto & i : attrPaths) { Value & v(*findAlongAttrPath(state, i, autoArgs, vRoot).first); - state.forceValue(v, v.determinePos(noPos)); + state.forceValue(v, [&]() { return v.determinePos(noPos); }); PathSet context; if (evalOnly) { diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 54daeb246..e9bebff17 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -673,7 +673,7 @@ void NixRepl::reloadFiles() void NixRepl::addAttrsToScope(Value & attrs) { - state->forceAttrs(attrs, attrs.determinePos(noPos)); + state->forceAttrs(attrs, [&]() { return attrs.determinePos(noPos); }); if (displ + attrs.attrs->size() >= envSize) throw Error("environment full; cannot add more variables"); @@ -712,7 +712,7 @@ void NixRepl::evalString(string s, Value & v) { Expr * e = parseString(s); e->eval(*state, *env, v); - state->forceValue(v, v.determinePos(noPos)); + state->forceValue(v, [&]() { return v.determinePos(noPos); }); } @@ -742,7 +742,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m str.flush(); checkInterrupt(); - state->forceValue(v, v.determinePos(noPos)); + state->forceValue(v, [&]() { return v.determinePos(noPos); }); switch (v.type()) {