23/64: Show position info in string concatenation / addition errors

[Ludovic Courtès]

civodul pushed a commit to branch nix
in repository guix.

commit bd9b1d97b42f307c8b595bb2de9b15bec1405b23
Author: Eelco Dolstra <address@hidden>
Date:   Fri Apr 4 22:19:33 2014 +0200

    Show position info in string concatenation / addition errors
---
 src/libexpr/eval.cc     |   39 +++++++++++++++++++++++++++------------
 src/libexpr/eval.hh     |    4 ++--
 src/libexpr/get-drvs.cc |    6 +++---
 src/libexpr/nixexpr.hh  |    5 +++--
 src/libexpr/parser.y    |   12 ++++++------
 src/libexpr/primops.cc  |   36 ++++++++++++++++++------------------
 src/nix-env/user-env.cc |    6 ++++--
 7 files changed, 63 insertions(+), 45 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 30a8793..e0451df 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -275,6 +275,16 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, 
const string & s2))
     throw EvalError(format(s) % s2);
 }
 
+LocalNoInlineNoReturn(void throwEvalError(const char * s, const Pos & pos))
+{
+    throw EvalError(format(s) % pos);
+}
+
+LocalNoInlineNoReturn(void throwEvalError(const char * s, const string & s2, 
const Pos & pos))
+{
+    throw EvalError(format(s) % s2 % pos);
+}
+
 LocalNoInlineNoReturn(void throwEvalError(const char * s, const string & s2, 
const string & s3))
 {
     throw EvalError(format(s) % s2 % s3);
@@ -295,6 +305,11 @@ LocalNoInlineNoReturn(void throwTypeError(const char * s))
     throw TypeError(s);
 }
 
+LocalNoInlineNoReturn(void throwTypeError(const char * s, const Pos & pos))
+{
+    throw TypeError(format(s) % pos);
+}
+
 LocalNoInlineNoReturn(void throwTypeError(const char * s, const string & s1))
 {
     throw TypeError(format(s) % s1);
@@ -648,7 +663,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & 
v)
         foreach (AttrDefs::iterator, i, attrs)
             v.attrs->push_back(Attr(i->first, i->second.e->maybeThunk(state, 
env), &i->second.pos));
 
-    /* dynamic attrs apply *after* rec and __overrides */
+    /* Dynamic attrs apply *after* rec and __overrides. */
     foreach (DynamicAttrDefs::iterator, i, dynamicAttrs) {
         Value nameVal;
         if (i->nameExpr->es->size() == 1) {
@@ -1113,17 +1128,17 @@ void ExprConcatStrings::eval(EvalState & state, Env & 
env, Value & v)
 
         if (firstType == tInt) {
             if (vTmp.type != tInt)
-                throwEvalError("cannot add %1% to an integer", showType(vTmp));
+                throwEvalError("cannot add %1% to an integer, at %2%", 
showType(vTmp), pos);
             n += vTmp.integer;
         } else
-            s << state.coerceToString(vTmp, context, false, firstType == 
tString);
+            s << state.coerceToString(pos, vTmp, context, false, firstType == 
tString);
     }
 
     if (firstType == tInt)
         mkInt(v, n);
     else if (firstType == tPath) {
         if (!context.empty())
-            throwEvalError("a string that refers to a store path cannot be 
appended to a path, in `%1%'", s.str());
+            throwEvalError("a string that refers to a store path cannot be 
appended to a path, at %1%", pos);
         mkPath(v, s.str().c_str());
     } else
         mkString(v, s.str(), context);
@@ -1233,7 +1248,7 @@ bool EvalState::isDerivation(Value & v)
 }
 
 
-string EvalState::coerceToString(Value & v, PathSet & context,
+string EvalState::coerceToString(const Pos & pos, Value & v, PathSet & context,
     bool coerceMore, bool copyToStore)
 {
     forceValue(v);
@@ -1252,8 +1267,8 @@ string EvalState::coerceToString(Value & v, PathSet & 
context,
 
     if (v.type == tAttrs) {
         Bindings::iterator i = v.attrs->find(sOutPath);
-        if (i == v.attrs->end()) throwTypeError("cannot coerce a set to a 
string");
-        return coerceToString(*i->value, context, coerceMore, copyToStore);
+        if (i == v.attrs->end()) throwTypeError("cannot coerce a set to a 
string, at %1%", pos);
+        return coerceToString(pos, *i->value, context, coerceMore, 
copyToStore);
     }
 
     if (coerceMore) {
@@ -1268,7 +1283,7 @@ string EvalState::coerceToString(Value & v, PathSet & 
context,
         if (v.type == tList) {
             string result;
             for (unsigned int n = 0; n < v.list.length; ++n) {
-                result += coerceToString(*v.list.elems[n],
+                result += coerceToString(pos, *v.list.elems[n],
                     context, coerceMore, copyToStore);
                 if (n < v.list.length - 1
                     /* !!! not quite correct */
@@ -1279,7 +1294,7 @@ string EvalState::coerceToString(Value & v, PathSet & 
context,
         }
     }
 
-    throwTypeError("cannot coerce %1% to a string", v);
+    throwTypeError("cannot coerce %1% to a string, at %2%", v, pos);
 }
 
 
@@ -1305,11 +1320,11 @@ string EvalState::copyPathToStore(PathSet & context, 
const Path & path)
 }
 
 
-Path EvalState::coerceToPath(Value & v, PathSet & context)
+Path EvalState::coerceToPath(const Pos & pos, Value & v, PathSet & context)
 {
-    string path = coerceToString(v, context, false, false);
+    string path = coerceToString(pos, v, context, false, false);
     if (path == "" || path[0] != '/')
-        throwEvalError("string `%1%' doesn't represent an absolute path", 
path);
+        throwEvalError("string `%1%' doesn't represent an absolute path, at 
%1%", path, pos);
     return path;
 }
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 1d53f49..d1bd27f 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -177,7 +177,7 @@ public:
        string.  If `coerceMore' is set, also converts nulls, integers,
        booleans and lists to a string.  If `copyToStore' is set,
        referenced paths are copied to the Nix store as a side effect. */
-    string coerceToString(Value & v, PathSet & context,
+    string coerceToString(const Pos & pos, Value & v, PathSet & context,
         bool coerceMore = false, bool copyToStore = true);
 
     string copyPathToStore(PathSet & context, const Path & path);
@@ -185,7 +185,7 @@ public:
     /* Path coercion.  Converts strings, paths and derivations to a
        path.  The result is guaranteed to be a canonicalised, absolute
        path.  Nothing is copied to the store. */
-    Path coerceToPath(Value & v, PathSet & context);
+    Path coerceToPath(const Pos & pos, Value & v, PathSet & context);
 
 public:
 
diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc
index 88ea68a..57b87ab 100644
--- a/src/libexpr/get-drvs.cc
+++ b/src/libexpr/get-drvs.cc
@@ -13,7 +13,7 @@ string DrvInfo::queryDrvPath()
     if (drvPath == "" && attrs) {
         Bindings::iterator i = attrs->find(state->sDrvPath);
         PathSet context;
-        drvPath = i != attrs->end() ? state->coerceToPath(*i->value, context) 
: "";
+        drvPath = i != attrs->end() ? state->coerceToPath(*i->pos, *i->value, 
context) : "";
     }
     return drvPath;
 }
@@ -24,7 +24,7 @@ string DrvInfo::queryOutPath()
     if (outPath == "" && attrs) {
         Bindings::iterator i = attrs->find(state->sOutPath);
         PathSet context;
-        outPath = i != attrs->end() ? state->coerceToPath(*i->value, context) 
: "";
+        outPath = i != attrs->end() ? state->coerceToPath(*i->pos, *i->value, 
context) : "";
     }
     return outPath;
 }
@@ -50,7 +50,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs()
                 Bindings::iterator outPath = 
out->value->attrs->find(state->sOutPath);
                 if (outPath == out->value->attrs->end()) continue; // FIXME: 
throw error?
                 PathSet context;
-                outputs[name] = state->coerceToPath(*outPath->value, context);
+                outputs[name] = state->coerceToPath(*outPath->pos, 
*outPath->value, context);
             }
         } else
             outputs["out"] = queryOutPath();
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index 9234e29..d781d92 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -320,10 +320,11 @@ MakeBinOp(OpConcatLists, "++")
 
 struct ExprConcatStrings : Expr
 {
+    Pos pos;
     bool forceString;
     vector<Expr *> * es;
-    ExprConcatStrings(bool forceString, vector<Expr *> * es)
-        : forceString(forceString), es(es) { };
+    ExprConcatStrings(const Pos & pos, bool forceString, vector<Expr *> * es)
+        : pos(pos), forceString(forceString), es(es) { };
     COMMON_METHODS
 };
 
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index 4830e7c..d39d0c0 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -130,7 +130,7 @@ static void addFormal(const Pos & pos, Formals * formals, 
const Formal & formal)
 }
 
 
-static Expr * stripIndentation(SymbolTable & symbols, vector<Expr *> & es)
+static Expr * stripIndentation(const Pos & pos, SymbolTable & symbols, 
vector<Expr *> & es)
 {
     if (es.empty()) return new ExprString(symbols.create(""));
 
@@ -216,7 +216,7 @@ static Expr * stripIndentation(SymbolTable & symbols, 
vector<Expr *> & es)
     }
 
     /* If this is a single string, then don't do a concatenation. */
-    return es2->size() == 1 && dynamic_cast<ExprString *>((*es2)[0]) ? 
(*es2)[0] : new ExprConcatStrings(true, es2);
+    return es2->size() == 1 && dynamic_cast<ExprString *>((*es2)[0]) ? 
(*es2)[0] : new ExprConcatStrings(pos, true, es2);
 }
 
 
@@ -344,7 +344,7 @@ expr_op
     { vector<Expr *> * l = new vector<Expr *>;
       l->push_back($1);
       l->push_back($3);
-      $$ = new ExprConcatStrings(false, l);
+      $$ = new ExprConcatStrings(CUR_POS, false, l);
     }
   | expr_op '-' expr_op { $$ = new ExprApp(CUR_POS, new ExprApp(new 
ExprBuiltin(data->symbols.create("sub")), $1), $3); }
   | expr_op '*' expr_op { $$ = new ExprApp(CUR_POS, new ExprApp(new 
ExprBuiltin(data->symbols.create("mul")), $1), $3); }
@@ -381,7 +381,7 @@ expr_simple
   | INT { $$ = new ExprInt($1); }
   | '"' string_parts '"' { $$ = $2; }
   | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE {
-      $$ = stripIndentation(data->symbols, *$2);
+      $$ = stripIndentation(CUR_POS, data->symbols, *$2);
   }
   | PATH { $$ = new ExprPath(absPath($1, data->basePath)); }
   | SPATH {
@@ -413,7 +413,7 @@ expr_simple
 
 string_parts
   : STR
-  | string_parts_interpolated { $$ = new ExprConcatStrings(true, $1); }
+  | string_parts_interpolated { $$ = new ExprConcatStrings(CUR_POS, true, $1); 
}
   | { $$ = new ExprString(data->symbols.create("")); }
   ;
 
@@ -509,7 +509,7 @@ attr
 
 string_attr
   : '"' string_parts '"' { $$ = $2; }
-  | DOLLAR_CURLY expr '}' { $$ = new ExprConcatStrings(true, new 
vector<Expr*>(1, $2)); }
+  | DOLLAR_CURLY expr '}' { $$ = new ExprConcatStrings(CUR_POS, true, new 
vector<Expr*>(1, $2)); }
   ;
 
 expr_list
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 0083340..84ef967 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -42,7 +42,7 @@ std::pair<string, string> decodeContext(const string & s)
 static void prim_import(EvalState & state, const Pos & pos, Value * * args, 
Value & v)
 {
     PathSet context;
-    Path path = state.coerceToPath(*args[0], context);
+    Path path = state.coerceToPath(pos, *args[0], context);
 
     foreach (PathSet::iterator, i, context) {
         Path ctx = decodeContext(*i).first;
@@ -255,14 +255,14 @@ static void prim_abort(EvalState & state, const Pos & 
pos, Value * * args, Value
 {
     PathSet context;
     throw Abort(format("evaluation aborted with the following error message: 
`%1%'") %
-        state.coerceToString(*args[0], context));
+        state.coerceToString(pos, *args[0], context));
 }
 
 
 static void prim_throw(EvalState & state, const Pos & pos, Value * * args, 
Value & v)
 {
     PathSet context;
-    throw ThrownError(format("%1%") % state.coerceToString(*args[0], context));
+    throw ThrownError(format("%1%") % state.coerceToString(pos, *args[0], 
context));
 }
 
 
@@ -273,7 +273,7 @@ static void prim_addErrorContext(EvalState & state, const 
Pos & pos, Value * * a
         v = *args[1];
     } catch (Error & e) {
         PathSet context;
-        e.addPrefix(format("%1%\n") % state.coerceToString(*args[0], context));
+        e.addPrefix(format("%1%\n") % state.coerceToString(pos, *args[0], 
context));
         throw;
     }
 }
@@ -383,7 +383,7 @@ static void prim_derivationStrict(EvalState & state, const 
Pos & pos, Value * *
             if (key == "args") {
                 state.forceList(*i->value, pos);
                 for (unsigned int n = 0; n < i->value->list.length; ++n) {
-                    string s = state.coerceToString(*i->value->list.elems[n], 
context, true);
+                    string s = state.coerceToString(posDrvName, 
*i->value->list.elems[n], context, true);
                     drv.args.push_back(s);
                 }
             }
@@ -391,7 +391,7 @@ static void prim_derivationStrict(EvalState & state, const 
Pos & pos, Value * *
             /* All other attributes are passed to the builder through
                the environment. */
             else {
-                string s = state.coerceToString(*i->value, context, true);
+                string s = state.coerceToString(posDrvName, *i->value, 
context, true);
                 drv.env[key] = s;
                 if (key == "builder") drv.builder = s;
                 else if (i->name == state.sSystem) drv.platform = s;
@@ -558,7 +558,7 @@ static void prim_derivationStrict(EvalState & state, const 
Pos & pos, Value * *
 static void prim_toPath(EvalState & state, const Pos & pos, Value * * args, 
Value & v)
 {
     PathSet context;
-    Path path = state.coerceToPath(*args[0], context);
+    Path path = state.coerceToPath(pos, *args[0], context);
     mkString(v, canonPath(path), context);
 }
 
@@ -574,7 +574,7 @@ static void prim_toPath(EvalState & state, const Pos & pos, 
Value * * args, Valu
 static void prim_storePath(EvalState & state, const Pos & pos, Value * * args, 
Value & v)
 {
     PathSet context;
-    Path path = state.coerceToPath(*args[0], context);
+    Path path = state.coerceToPath(pos, *args[0], context);
     /* Resolve symlinks in ‘path’, unless ‘path’ itself is a symlink
        directly in the store.  The latter condition is necessary so
        e.g. nix-push does the right thing. */
@@ -592,7 +592,7 @@ static void prim_storePath(EvalState & state, const Pos & 
pos, Value * * args, V
 static void prim_pathExists(EvalState & state, const Pos & pos, Value * * 
args, Value & v)
 {
     PathSet context;
-    Path path = state.coerceToPath(*args[0], context);
+    Path path = state.coerceToPath(pos, *args[0], context);
     if (!context.empty())
         throw EvalError(format("string `%1%' cannot refer to other paths, at 
%2%") % path % pos);
     mkBool(v, pathExists(path));
@@ -604,7 +604,7 @@ static void prim_pathExists(EvalState & state, const Pos & 
pos, Value * * args,
 static void prim_baseNameOf(EvalState & state, const Pos & pos, Value * * 
args, Value & v)
 {
     PathSet context;
-    mkString(v, baseNameOf(state.coerceToString(*args[0], context)), context);
+    mkString(v, baseNameOf(state.coerceToString(pos, *args[0], context)), 
context);
 }
 
 
@@ -614,7 +614,7 @@ static void prim_baseNameOf(EvalState & state, const Pos & 
pos, Value * * args,
 static void prim_dirOf(EvalState & state, const Pos & pos, Value * * args, 
Value & v)
 {
     PathSet context;
-    Path dir = dirOf(state.coerceToPath(*args[0], context));
+    Path dir = dirOf(state.coerceToPath(pos, *args[0], context));
     if (args[0]->type == tPath) mkPath(v, dir.c_str()); else mkString(v, dir, 
context);
 }
 
@@ -623,7 +623,7 @@ static void prim_dirOf(EvalState & state, const Pos & pos, 
Value * * args, Value
 static void prim_readFile(EvalState & state, const Pos & pos, Value * * args, 
Value & v)
 {
     PathSet context;
-    Path path = state.coerceToPath(*args[0], context);
+    Path path = state.coerceToPath(pos, *args[0], context);
     if (!context.empty())
         throw EvalError(format("string `%1%' cannot refer to other paths, at 
%2%") % path % pos);
     mkString(v, readFile(path).c_str());
@@ -731,7 +731,7 @@ struct FilterFromExpr : PathFilter
 static void prim_filterSource(EvalState & state, const Pos & pos, Value * * 
args, Value & v)
 {
     PathSet context;
-    Path path = state.coerceToPath(*args[1], context);
+    Path path = state.coerceToPath(pos, *args[1], context);
     if (!context.empty())
         throw EvalError(format("string `%1%' cannot refer to other paths, at 
%2%") % path % pos);
 
@@ -1105,7 +1105,7 @@ static void prim_lessThan(EvalState & state, const Pos & 
pos, Value * * args, Va
 static void prim_toString(EvalState & state, const Pos & pos, Value * * args, 
Value & v)
 {
     PathSet context;
-    string s = state.coerceToString(*args[0], context, true, false);
+    string s = state.coerceToString(pos, *args[0], context, true, false);
     mkString(v, s, context);
 }
 
@@ -1119,7 +1119,7 @@ static void prim_substring(EvalState & state, const Pos & 
pos, Value * * args, V
     int start = state.forceInt(*args[0], pos);
     int len = state.forceInt(*args[1], pos);
     PathSet context;
-    string s = state.coerceToString(*args[2], context);
+    string s = state.coerceToString(pos, *args[2], context);
 
     if (start < 0) throw EvalError(format("negative start position in 
`substring', at %1%") % pos);
 
@@ -1130,7 +1130,7 @@ static void prim_substring(EvalState & state, const Pos & 
pos, Value * * args, V
 static void prim_stringLength(EvalState & state, const Pos & pos, Value * * 
args, Value & v)
 {
     PathSet context;
-    string s = state.coerceToString(*args[0], context);
+    string s = state.coerceToString(pos, *args[0], context);
     mkInt(v, s.size());
 }
 
@@ -1138,7 +1138,7 @@ static void prim_stringLength(EvalState & state, const 
Pos & pos, Value * * args
 static void prim_unsafeDiscardStringContext(EvalState & state, const Pos & 
pos, Value * * args, Value & v)
 {
     PathSet context;
-    string s = state.coerceToString(*args[0], context);
+    string s = state.coerceToString(pos, *args[0], context);
     mkString(v, s, PathSet());
 }
 
@@ -1152,7 +1152,7 @@ static void prim_unsafeDiscardStringContext(EvalState & 
state, const Pos & pos,
 static void prim_unsafeDiscardOutputDependency(EvalState & state, const Pos & 
pos, Value * * args, Value & v)
 {
     PathSet context;
-    string s = state.coerceToString(*args[0], context);
+    string s = state.coerceToString(pos, *args[0], context);
 
     PathSet context2;
     foreach (PathSet::iterator, i, context) {
diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc
index a95c4cb..e71ad34 100644
--- a/src/nix-env/user-env.cc
+++ b/src/nix-env/user-env.cc
@@ -121,8 +121,10 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
     debug("evaluating user environment builder");
     state.forceValue(topLevel);
     PathSet context;
-    Path topLevelDrv = 
state.coerceToPath(*topLevel.attrs->find(state.sDrvPath)->value, context);
-    Path topLevelOut = 
state.coerceToPath(*topLevel.attrs->find(state.sOutPath)->value, context);
+    Attr & aDrvPath(*topLevel.attrs->find(state.sDrvPath));
+    Path topLevelDrv = state.coerceToPath(aDrvPath.pos ? *(aDrvPath.pos) : 
noPos, *(aDrvPath.value), context);
+    Attr & aOutPath(*topLevel.attrs->find(state.sOutPath));
+    Path topLevelOut = state.coerceToPath(aOutPath.pos ? *(aOutPath.pos) : 
noPos, *(aOutPath.value), context);
 
     /* Realise the resulting store expression. */
     debug("building user environment");