Merge pull request NixOS#6204 from layus/coerce-string

Add context to better locate runtime coercions

doc/manual/src/release-notes/rl-next.md

@@ -18,3 +18,6 @@
 * Instead of "antiquotation", the more common term [string interpolation](../language/string-interpolation.md) is now used consistently.
   Historical release notes were not changed.
 
+* Error traces have been reworked to provide detailed explanations and more
+  accurate error locations. A short excerpt of the trace is now shown by
+  default when an error occurs.

src/libcmd/installables.cc

@@ -591,7 +591,7 @@ ref<eval_cache::EvalCache> openEvalCache(
             auto vFlake = state.allocValue();
             flake::callFlake(state, *lockedFlake, *vFlake);
 
-            state.forceAttrs(*vFlake, noPos);
+            state.forceAttrs(*vFlake, noPos, "while parsing cached flake data");
 
             auto aOutputs = vFlake->attrs->get(state.symbols.create("outputs"));
             assert(aOutputs);

src/libcmd/repl.cc

@@ -397,7 +397,7 @@ StringSet NixRepl::completePrefix(const std::string & prefix)
             Expr * e = parseString(expr);
             Value v;
             e->eval(*state, *env, v);
-            state->forceAttrs(v, noPos);
+            state->forceAttrs(v, noPos, "nevermind, it is ignored anyway");
 
             for (auto & i : *v.attrs) {
                 std::string_view name = state->symbols[i.name];
@@ -590,7 +590,7 @@ bool NixRepl::processLine(std::string line)
         const auto [path, line] = [&] () -> std::pair<Path, uint32_t> {
             if (v.type() == nPath || v.type() == nString) {
                 PathSet context;
-                auto path = state->coerceToPath(noPos, v, context);
+                auto path = state->coerceToPath(noPos, v, context, "while evaluating the filename to edit");
                 return {path, 0};
             } else if (v.isLambda()) {
                 auto pos = state->positions[v.lambda.fun->pos];
@@ -834,7 +834,7 @@ void NixRepl::loadFiles()
 
 void NixRepl::addAttrsToScope(Value & attrs)
 {
-    state->forceAttrs(attrs, [&]() { return attrs.determinePos(noPos); });
+    state->forceAttrs(attrs, [&]() { return attrs.determinePos(noPos); }, "while evaluating an attribute set to be merged in the global scope");
     if (displ + attrs.attrs->size() >= envSize)
         throw Error("environment full; cannot add more variables");
 
@@ -939,7 +939,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
             Bindings::iterator i = v.attrs->find(state->sDrvPath);
             PathSet context;
             if (i != v.attrs->end())
-                str << state->store->printStorePath(state->coerceToStorePath(i->pos, *i->value, context));
+                str << state->store->printStorePath(state->coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation"));
             else
                 str << "???";
             str << "»";

src/libexpr/attr-path.cc

@@ -118,7 +118,7 @@ std::pair<std::string, uint32_t> findPackageFilename(EvalState & state, Value &
 
     // FIXME: is it possible to extract the Pos object instead of doing this
     //        toString + parsing?
-    auto pos = state.forceString(*v2);
+    auto pos = state.forceString(*v2, noPos, "while evaluating the 'meta.position' attribute of a derivation");
 
     auto colon = pos.rfind(':');
     if (colon == std::string::npos)

src/libexpr/eval-cache.cc

@@ -385,7 +385,7 @@ Value & AttrCursor::getValue()
     if (!_value) {
         if (parent) {
             auto & vParent = parent->first->getValue();
-            root->state.forceAttrs(vParent, noPos);
+            root->state.forceAttrs(vParent, noPos, "while searching for an attribute");
             auto attr = vParent.attrs->get(parent->second);
             if (!attr)
                 throw Error("attribute '%s' is unexpectedly missing", getAttrPathStr());
@@ -571,14 +571,14 @@ std::string AttrCursor::getString()
                 debug("using cached string attribute '%s'", getAttrPathStr());
                 return s->first;
             } else
-                root->state.debugThrowLastTrace(TypeError("'%s' is not a string", getAttrPathStr()));
+                root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow<TypeError>();
         }
     }
 
     auto & v = forceValue();
 
     if (v.type() != nString && v.type() != nPath)
-        root->state.debugThrowLastTrace(TypeError("'%s' is not a string but %s", getAttrPathStr(), showType(v.type())));
+        root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow<TypeError>();
 
     return v.type() == nString ? v.string.s : v.path;
 }
@@ -602,7 +602,7 @@ string_t AttrCursor::getStringWithContext()
                     return *s;
                 }
             } else
-                root->state.debugThrowLastTrace(TypeError("'%s' is not a string", getAttrPathStr()));
+                root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow<TypeError>();
         }
     }
 
@@ -613,7 +613,7 @@ string_t AttrCursor::getStringWithContext()
     else if (v.type() == nPath)
         return {v.path, {}};
     else
-        root->state.debugThrowLastTrace(TypeError("'%s' is not a string but %s", getAttrPathStr(), showType(v.type())));
+        root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow<TypeError>();
 }
 
 bool AttrCursor::getBool()
@@ -626,14 +626,14 @@ bool AttrCursor::getBool()
                 debug("using cached Boolean attribute '%s'", getAttrPathStr());
                 return *b;
             } else
-                root->state.debugThrowLastTrace(TypeError("'%s' is not a Boolean", getAttrPathStr()));
+                root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow<TypeError>();
         }
     }
 
     auto & v = forceValue();
 
     if (v.type() != nBool)
-        root->state.debugThrowLastTrace(TypeError("'%s' is not a Boolean", getAttrPathStr()));
+        root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow<TypeError>();
 
     return v.boolean;
 }
@@ -685,7 +685,7 @@ std::vector<std::string> AttrCursor::getListOfStrings()
     std::vector<std::string> res;
 
     for (auto & elem : v.listItems())
-        res.push_back(std::string(root->state.forceStringNoCtx(*elem)));
+        res.push_back(std::string(root->state.forceStringNoCtx(*elem, noPos, "while evaluating an attribute for caching")));
 
     if (root->db)
         cachedValue = {root->db->setListOfStrings(getKey(), res), res};
@@ -703,14 +703,14 @@ std::vector<Symbol> AttrCursor::getAttrs()
                 debug("using cached attrset attribute '%s'", getAttrPathStr());
                 return *attrs;
             } else
-                root->state.debugThrowLastTrace(TypeError("'%s' is not an attribute set", getAttrPathStr()));
+                root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow<TypeError>();
         }
     }
 
     auto & v = forceValue();
 
     if (v.type() != nAttrs)
-        root->state.debugThrowLastTrace(TypeError("'%s' is not an attribute set", getAttrPathStr()));
+        root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow<TypeError>();
 
     std::vector<Symbol> attrs;
     for (auto & attr : *getValue().attrs)

src/libexpr/eval-inline.hh

@@ -103,33 +103,36 @@ void EvalState::forceValue(Value & v, Callable getPos)
     else if (v.isApp())
         callFunction(*v.app.left, *v.app.right, v, noPos);
     else if (v.isBlackhole())
-        throwEvalError(getPos(), "infinite recursion encountered");
+        error("infinite recursion encountered").atPos(getPos()).template debugThrow<EvalError>();
 }
 
 
 [[gnu::always_inline]]
-inline void EvalState::forceAttrs(Value & v, const PosIdx pos)
+inline void EvalState::forceAttrs(Value & v, const PosIdx pos, std::string_view errorCtx)
 {
-    forceAttrs(v, [&]() { return pos; });
+    forceAttrs(v, [&]() { return pos; }, errorCtx);
 }
 
 
 template <typename Callable>
 [[gnu::always_inline]]
-inline void EvalState::forceAttrs(Value & v, Callable getPos)
+inline void EvalState::forceAttrs(Value & v, Callable getPos, std::string_view errorCtx)
 {
-    forceValue(v, getPos);
-    if (v.type() != nAttrs)
-        throwTypeError(getPos(), "value is %1% while a set was expected", v);
+    forceValue(v, noPos);
+    if (v.type() != nAttrs) {
+        PosIdx pos = getPos();
+        error("value is %1% while a set was expected", showType(v)).withTrace(pos, errorCtx).debugThrow<TypeError>();
+    }
 }
 
 
 [[gnu::always_inline]]
-inline void EvalState::forceList(Value & v, const PosIdx pos)
+inline void EvalState::forceList(Value & v, const PosIdx pos, std::string_view errorCtx)
 {
-    forceValue(v, pos);
-    if (!v.isList())
-        throwTypeError(pos, "value is %1% while a list was expected", v);
+    forceValue(v, noPos);
+    if (!v.isList()) {
+        error("value is %1% while a list was expected", showType(v)).withTrace(pos, errorCtx).debugThrow<TypeError>();
+    }
 }