libexpr: misc improvements for proper error position

When working on some more complex Nix code, there are sometimes rather
unhelpful or misleading error messages, especially if coerce-errors are
thrown.

This patch is a first steps towards improving that. I'm happy to file
more changes after that, but I'd like to gather some feedback first.

To summarize, this patch does the following things:

* Attrsets (a.k.a. `Bindings` in `libexpr`) now have a `Pos`. This is
  helpful e.g. to identify which attribute-set in `listToAttrs` is
  invalid.

* The `Value`-struct has a new method named `determinePos` which tries
  to guess the position of a value and falls back to a default if that's
  not possible.

  This can be used to provide better messages if a coercion fails.

* The new `determinePos`-API is used by `builtins.concatMap` now. With
  that change, Nix shows the exact position in the error where a wrong
  value was returned by the lambda.

  To make sure it's still obvious that `concatMap` is the problem,
  another stack-frame was added.

* The changes described above can be added to every other `primop`, but
  first I'd like to get some feedback about the overall approach.

src/libexpr/attr-set.hh

@@ -35,6 +35,7 @@ class Bindings
 {
 public:
     typedef uint32_t size_t;
+    Pos *pos;
 
 private:
     size_t size_, capacity_;

src/libexpr/eval.cc

@@ -201,6 +201,15 @@ string showType(const Value & v)
     }
 }
 
+Pos Value::determinePos(const Pos &pos) const
+{
+    switch (internalType) {
+        case tAttrs: return *attrs->pos;
+        case tLambda: return lambda.fun->pos;
+        case tApp: return app.left->determinePos(pos);
+        default: return pos;
+    }
+}
 
 bool Value::isTrivial() const
 {
@@ -1060,6 +1069,8 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
         v.attrs->push_back(Attr(nameSym, i.valueExpr->maybeThunk(state, *dynamicEnv), &i.pos));
         v.attrs->sort(); // FIXME: inefficient
     }
+
+    v.attrs->pos = &pos;
 }
 
 

src/libexpr/nixexpr.hh

@@ -180,6 +180,7 @@ struct ExprOpHasAttr : Expr
 struct ExprAttrs : Expr
 {
     bool recursive;
+    Pos pos;
     struct AttrDef {
         bool inherited;
         Expr * e;
@@ -199,7 +200,8 @@ struct ExprAttrs : Expr
     };
     typedef std::vector<DynamicAttrDef> DynamicAttrDefs;
     DynamicAttrDefs dynamicAttrs;
-    ExprAttrs() : recursive(false) { };
+    ExprAttrs(const Pos &pos) : recursive(false), pos(pos) { };
+    ExprAttrs() : recursive(false), pos(noPos) { };
     COMMON_METHODS
 };
 

src/libexpr/parser.y

@@ -478,7 +478,7 @@ binds
           $$->attrs[i.symbol] = ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, i.symbol), makeCurPos(@6, data));
       }
     }
-  | { $$ = new ExprAttrs; }
+  | { $$ = new ExprAttrs(makeCurPos(@0, data)); }
   ;
 
 attrs

src/libexpr/primops.cc

@@ -2149,21 +2149,27 @@ static void prim_listToAttrs(EvalState & state, const Pos & pos, Value * * args,
         state.forceAttrs(v2, pos);
 
         Bindings::iterator j = v2.attrs->find(state.sName);
-        if (j == v2.attrs->end())
-            throw TypeError({
-                .msg = hintfmt("'name' attribute missing in a call to 'listToAttrs'"),
-                .errPos = pos
+        if (j == v2.attrs->end()) {
+            auto e = TypeError({
+                .msg = hintfmt("'name' attribute missing for 'listToAttrs'"),
+                .errPos = *v2.attrs->pos
             });
-        string name = state.forceStringNoCtx(*j->value, pos);
+            e.addTrace(pos, hintfmt("while invoking '%s'", "listToAttrs"));
+            throw e;
+        }
+        string name = state.forceStringNoCtx(*j->value, *j->pos);
 
         Symbol sym = state.symbols.create(name);
         if (seen.insert(sym).second) {
             Bindings::iterator j2 = v2.attrs->find(state.symbols.create(state.sValue));
-            if (j2 == v2.attrs->end())
-                throw TypeError({
-                    .msg = hintfmt("'value' attribute missing in a call to 'listToAttrs'"),
-                    .errPos = pos
+            if (j2 == v2.attrs->end()) {
+                auto e = TypeError({
+                    .msg = hintfmt("'value' attribute missing for 'listToAttrs'"),
+                    .errPos = *v2.attrs->pos
                 });
+                e.addTrace(pos, hintfmt("while invoking '%s'", "listToAttrs"));
+                throw e;
+            }
             v.attrs->push_back(Attr(sym, j2->value, j2->pos));
         }
     }
@@ -2804,7 +2810,12 @@ static void prim_concatMap(EvalState & state, const Pos & pos, Value * * args, V
     for (unsigned int n = 0; n < nrLists; ++n) {
         Value * vElem = args[1]->listElems()[n];
         state.callFunction(*args[0], *vElem, lists[n], pos);
-        state.forceList(lists[n], pos);
+        try {
+            state.forceList(lists[n], lists[n].determinePos(args[0]->determinePos(pos)));
+        } catch (TypeError &e) {
+            e.addTrace(pos, hintfmt("while invoking '%s'", "concatMap"));
+            throw e;
+        }
         len += lists[n].listSize();
     }
 

src/libexpr/value.hh

@@ -341,6 +341,8 @@ public:
         return internalType == tList1 ? 1 : internalType == tList2 ? 2 : bigList.size;
     }
 
+    Pos determinePos(const Pos &pos) const;
+
     /* Check whether forcing this value requires a trivial amount of
        computation. In particular, function applications are
        non-trivial. */