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

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<Value *, Pos> 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<Value *, Pos> 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<Value *, Pos> 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()) {