From 49b0bb020663b7283549754a826b1346c93a8bbb Mon Sep 17 00:00:00 2001 From: Kevin Amado Date: Sat, 27 Nov 2021 12:40:24 -0500 Subject: [PATCH] forceValue: make pos mandatory - Make passing the position to `forceValue` mandatory, this way we remember people that the position is important for better error messages - Add pos to all `forceValue` calls --- src/libcmd/installables.cc | 11 ++++++----- src/libexpr/attr-path.cc | 2 +- src/libexpr/eval-cache.cc | 2 +- src/libexpr/eval-inline.hh | 4 ++-- src/libexpr/eval.cc | 17 +++++++++-------- src/libexpr/eval.hh | 2 +- src/libexpr/get-drvs.cc | 4 ++-- src/nix-build/nix-build.cc | 2 +- src/nix-env/user-env.cc | 2 +- src/nix-instantiate/nix-instantiate.cc | 2 +- src/nix/eval.cc | 2 +- src/nix/repl.cc | 4 ++-- 12 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 0c2db0399..268861ce8 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -198,8 +198,9 @@ void SourceExprCommand::completeInstallable(std::string_view prefix) prefix_ = ""; } - Value &v1(*findAlongAttrPath(*state, prefix_, *autoArgs, root).first); - state->forceValue(v1); + auto [v, pos] = findAlongAttrPath(*state, prefix_, *autoArgs, root); + Value &v1(*v); + state->forceValue(v1, pos); Value v2; state->autoCallFunction(*autoArgs, v1, v2); @@ -446,7 +447,7 @@ struct InstallableAttrPath : InstallableValue std::pair toValue(EvalState & state) override { auto [vRes, pos] = findAlongAttrPath(state, attrPath, *cmd.getAutoArgs(state), **v); - state.forceValue(*vRes); + state.forceValue(*vRes, pos); return {vRes, pos}; } @@ -496,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); + state.forceValue(*aOutputs->value, aOutputs->value->determinePos(noPos)); return aOutputs->value; } @@ -608,7 +609,7 @@ std::pair InstallableFlake::toValue(EvalState & state) for (auto & attrPath : getActualAttrPaths()) { try { auto [v, pos] = findAlongAttrPath(state, attrPath, *emptyArgs, *vOutputs); - state.forceValue(*v); + state.forceValue(*v, pos); return {v, pos}; } catch (AttrPathNotFound & e) { } diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index c50c6d92b..8ec24269f 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -58,7 +58,7 @@ std::pair findAlongAttrPath(EvalState & state, const string & attr Value * vNew = state.allocValue(); state.autoCallFunction(autoArgs, *v, *vNew); v = vNew; - state.forceValue(*v); + state.forceValue(*v, v->determinePos(vIn.determinePos(noPos))); /* It should evaluate to either a set or an expression, according to what is specified in the attrPath. */ diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index d7e21783d..a92acafa8 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -381,7 +381,7 @@ Value & AttrCursor::forceValue() auto & v = getValue(); try { - root->state.forceValue(v); + root->state.forceValue(v, v.determinePos(noPos)); } catch (EvalError &) { debug("setting '%s' to failed", getAttrPathStr()); if (root->db) diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index 655408cd3..cd58315ba 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -53,7 +53,7 @@ void EvalState::forceValue(Value & v, const Pos & pos) inline void EvalState::forceAttrs(Value & v) { - forceValue(v); + forceValue(v, v.determinePos(noPos)); if (v.type() != nAttrs) throwTypeError("value is %1% while a set was expected", v); } @@ -69,7 +69,7 @@ inline void EvalState::forceAttrs(Value & v, const Pos & pos) inline void EvalState::forceList(Value & v) { - forceValue(v); + forceValue(v, v.determinePos(noPos)); if (!v.isList()) throwTypeError("value is %1% while a list was expected", v); } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 94ffab175..ee3d86296 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1280,7 +1280,7 @@ void ExprOpHasAttr::eval(EvalState & state, Env & env, Value & v) e->eval(state, env, vTmp); for (auto & i : attrPath) { - state.forceValue(*vAttrs); + state.forceValue(*vAttrs, vAttrs->determinePos(noPos)); Bindings::iterator j; Symbol name = getName(i, state, env); if (vAttrs->type() != nAttrs || @@ -1494,14 +1494,15 @@ void EvalState::incrFunctionCall(ExprLambda * fun) void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) { - forceValue(fun); + Pos pos = fun.determinePos(noPos); + forceValue(fun, pos); if (fun.type() == nAttrs) { auto found = fun.attrs->find(sFunctor); if (found != fun.attrs->end()) { Value * v = allocValue(); - callFunction(*found->value, fun, *v, noPos); - forceValue(*v); + callFunction(*found->value, fun, *v, pos); + forceValue(*v, pos); return autoCallFunction(args, *v, res); } } @@ -1787,7 +1788,7 @@ void EvalState::forceValueDeep(Value & v) recurse = [&](Value & v) { if (!seen.insert(&v).second) return; - forceValue(v); + forceValue(v, v.determinePos(noPos)); if (v.type() == nAttrs) { for (auto & i : *v.attrs) @@ -1924,7 +1925,7 @@ bool EvalState::isDerivation(Value & v) if (v.type() != nAttrs) return false; Bindings::iterator i = v.attrs->find(sType); if (i == v.attrs->end()) return false; - forceValue(*i->value); + forceValue(*i->value, *i->pos); if (i->value->type() != nString) return false; return strcmp(i->value->string.s, "derivation") == 0; } @@ -2036,8 +2037,8 @@ Path EvalState::coerceToPath(const Pos & pos, Value & v, PathSet & context) bool EvalState::eqValues(Value & v1, Value & v2) { - forceValue(v1); - forceValue(v2); + forceValue(v1, v1.determinePos(noPos)); + forceValue(v2, v2.determinePos(noPos)); /* !!! Hack to support some old broken code that relies on pointer equality tests between sets. (Specifically, builderDefs calls diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 82ce9d1b3..f0b94f440 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -221,7 +221,7 @@ public: of the evaluation of the thunk. If `v' is a delayed function application, call the function and overwrite `v' with the result. Otherwise, this is a no-op. */ - inline void forceValue(Value & v, const Pos & pos = noPos); + inline void forceValue(Value & v, const Pos & pos); /* Force a value, then recursively force list elements and attributes. */ diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 25fd9b949..fe1d11505 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); + state->forceValue(v, 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); + state.forceValue(v, v.determinePos(noPos)); if (!state.isDerivation(v)) return true; /* Remove spurious duplicates (e.g., a set like `rec { x = diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 471a03ac7..66aa145a4 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); + state->forceValue(v, 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 68c1c16b2..727338105 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); + state.forceValue(topLevel, 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 19a954ddd..8836f810c 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); + state.forceValue(v, v.determinePos(noPos)); PathSet context; if (evalOnly) { diff --git a/src/nix/eval.cc b/src/nix/eval.cc index c7517cf79..686244be0 100644 --- a/src/nix/eval.cc +++ b/src/nix/eval.cc @@ -81,7 +81,7 @@ struct CmdEval : MixJSON, InstallableCommand recurse = [&](Value & v, const Pos & pos, const Path & path) { - state->forceValue(v); + state->forceValue(v, pos); if (v.type() == nString) // FIXME: disallow strings with contexts? writeFile(path, v.string.s); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index be9f96836..5c2bf4bfd 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -712,7 +712,7 @@ void NixRepl::evalString(string s, Value & v) { Expr * e = parseString(s); e->eval(*state, *env, v); - state->forceValue(v); + state->forceValue(v, 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); + state->forceValue(v, v.determinePos(noPos)); switch (v.type()) {