From bd9b1d97b42f307c8b595bb2de9b15bec1405b23 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 4 Apr 2014 22:19:33 +0200 Subject: [PATCH] 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 30a8793ee..e0451dfa7 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 1d53f4982..d1bd27f16 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 88ea68a5f..57b87ab58 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 9234e2970..d781d92ba 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 * es; - ExprConcatStrings(bool forceString, vector * es) - : forceString(forceString), es(es) { }; + ExprConcatStrings(const Pos & pos, bool forceString, vector * es) + : pos(pos), forceString(forceString), es(es) { }; COMMON_METHODS }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 4830e7ca1..d39d0c0a7 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 & es) +static Expr * stripIndentation(const Pos & pos, SymbolTable & symbols, vector & es) { if (es.empty()) return new ExprString(symbols.create("")); @@ -216,7 +216,7 @@ static Expr * stripIndentation(SymbolTable & symbols, vector & es) } /* If this is a single string, then don't do a concatenation. */ - return es2->size() == 1 && dynamic_cast((*es2)[0]) ? (*es2)[0] : new ExprConcatStrings(true, es2); + return es2->size() == 1 && dynamic_cast((*es2)[0]) ? (*es2)[0] : new ExprConcatStrings(pos, true, es2); } @@ -344,7 +344,7 @@ expr_op { vector * l = new vector; 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(1, $2)); } + | DOLLAR_CURLY expr '}' { $$ = new ExprConcatStrings(CUR_POS, true, new vector(1, $2)); } ; expr_list diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 00833403d..84ef967ff 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -42,7 +42,7 @@ std::pair 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 a95c4cba9..e71ad34ed 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");