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.

4 files changed, 18 insertions, 21 deletions

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 02f99feb31..3d77b94e77 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 c847e3baf8..03a14695f1 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 7db0a15fa3..e555a5dff0 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 c63043c4d1..6a282e9054 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;
 }