From 6b47de580ffe6101863a1054d9d47f9cbe691214 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 8 Oct 2013 14:40:51 +0200 Subject: [PATCH] Show the exact position of undefined variables In particular, undefined variable errors in a "with" previously didn't show *any* position information, so this should help a lot in those cases. --- src/libexpr/eval.cc | 2 +- src/libexpr/nixexpr.cc | 2 +- src/libexpr/nixexpr.hh | 3 ++- src/libexpr/parser.y | 32 ++++++++++++++------------------ 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 02f99feb3..3d77b94e7 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -315,7 +315,7 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) return j->value; } if (!env->prevWith) - throwEvalError("undefined variable `%1%'", var.name); + throw EvalError(format("undefined variable `%1%' at %2%") % var.name % var.pos); for (unsigned int l = env->prevWith; l; --l, env = env->up) ; } } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index c847e3baf..03a14695f 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -198,7 +198,7 @@ void ExprVar::bindVars(const StaticEnv & env) /* Otherwise, the variable must be obtained from the nearest enclosing `with'. If there is no `with', then we can issue an "undefined variable" error now. */ - if (withLevel == -1) throw EvalError(format("undefined variable `%1%'") % name); + if (withLevel == -1) throw ParseError(format("undefined variable `%1%' at %2%") % name % pos); fromWith = true; this->level = withLevel; diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 7db0a15fa..e555a5dff 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -109,6 +109,7 @@ struct ExprPath : Expr struct ExprVar : Expr { + Pos pos; Symbol name; /* Whether the variable comes from an environment (e.g. a rec, let @@ -124,7 +125,7 @@ struct ExprVar : Expr unsigned int level; unsigned int displ; - ExprVar(const Symbol & name) : name(name) { }; + ExprVar(const Pos & pos, const Symbol & name) : pos(pos), name(name) { }; COMMON_METHODS Value * maybeThunk(EvalState & state, Env & env); }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index c63043c4d..6a282e905 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -310,13 +310,13 @@ expr_if expr_op : '!' expr_op %prec NOT { $$ = new ExprOpNot($2); } - | '-' expr_op %prec NEGATE { $$ = new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__sub")), new ExprInt(0)), $2); } +| '-' expr_op %prec NEGATE { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__sub")), new ExprInt(0)), $2); } | expr_op EQ expr_op { $$ = new ExprOpEq($1, $3); } | expr_op NEQ expr_op { $$ = new ExprOpNEq($1, $3); } - | expr_op '<' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__lessThan")), $1), $3); } - | expr_op LEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__lessThan")), $3), $1)); } - | expr_op '>' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__lessThan")), $3), $1); } - | expr_op GEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__lessThan")), $1), $3)); } + | expr_op '<' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $1), $3); } + | expr_op LEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $3), $1)); } + | expr_op '>' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $3), $1); } + | expr_op GEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $1), $3)); } | expr_op AND expr_op { $$ = new ExprOpAnd($1, $3); } | expr_op OR expr_op { $$ = new ExprOpOr($1, $3); } | expr_op IMPL expr_op { $$ = new ExprOpImpl($1, $3); } @@ -328,9 +328,9 @@ expr_op l->push_back($3); $$ = new ExprConcatStrings(false, l); } - | expr_op '-' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__sub")), $1), $3); } - | expr_op '*' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__mul")), $1), $3); } - | expr_op '/' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(data->symbols.create("__div")), $1), $3); } + | expr_op '-' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__sub")), $1), $3); } + | expr_op '*' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__mul")), $1), $3); } + | expr_op '/' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__div")), $1), $3); } | expr_op CONCAT expr_op { $$ = new ExprOpConcatLists($1, $3); } | expr_app ; @@ -349,12 +349,12 @@ expr_select | /* Backwards compatibility: because Nixpkgs has a rarely used function named ‘or’, allow stuff like ‘map or [...]’. */ expr_simple OR_KW - { $$ = new ExprApp($1, new ExprVar(data->symbols.create("or"))); } + { $$ = new ExprApp($1, new ExprVar(CUR_POS, data->symbols.create("or"))); } | expr_simple { $$ = $1; } ; expr_simple - : ID { $$ = new ExprVar(data->symbols.create($1)); } + : ID { $$ = new ExprVar(CUR_POS, data->symbols.create($1)); } | INT { $$ = new ExprInt($1); } | '"' string_parts '"' { /* For efficiency, and to simplify parse trees a bit. */ @@ -372,10 +372,10 @@ expr_simple /* The file wasn't found in the search path. However, we can't throw an error here, because the expression might never be evaluated. So return an expression that lazily calls - ‘abort’. */ + ‘throw’. */ $$ = path2 == "" ? (Expr * ) new ExprApp( - new ExprVar(data->symbols.create("throw")), + new ExprVar(noPos, data->symbols.create("throw")), new ExprString(data->symbols.create( (format("file `%1%' was not found in the Nix search path (add it using $NIX_PATH or -I)") % path).str()))) : (Expr * ) new ExprPath(path2); @@ -413,7 +413,7 @@ binds if ($$->attrs.find(*i) != $$->attrs.end()) dupAttr(*i, makeCurPos(@3, data), $$->attrs[*i].pos); Pos pos = makeCurPos(@3, data); - $$->attrs[*i] = ExprAttrs::AttrDef(new ExprVar(*i), pos, true); + $$->attrs[*i] = ExprAttrs::AttrDef(new ExprVar(CUR_POS, *i), pos, true); } } | binds INHERIT '(' expr ')' attrs ';' @@ -495,11 +495,7 @@ Expr * EvalState::parse(const char * text, if (res) throw ParseError(data.error); - try { - data.result->bindVars(staticEnv); - } catch (Error & e) { - throw ParseError(format("%1%, in `%2%'") % e.msg() % path); - } + data.result->bindVars(staticEnv); return data.result; }