* Better error messages (especially wrt types).

7 files changed, 96 insertions, 62 deletions

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 667961cd5..5315619d4 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -41,7 +41,7 @@ static Expr substArgs(Expr body, ATermList formals, Expr arg)
         if (!matchNoDefFormal(*i, name) && !matchDefFormal(*i, name, def))
             abort(); /* can't happen */
         if (subs[name] == 0) {
-            if (def == 0) throw Error(format("required function argument `%1%' missing")
+            if (def == 0) throw TypeError(format("the argument named `%1%' required by the function is missing")
                 % aterm2String(name));
             defsUsed.push_back(name);
             recAttrs = ATinsert(recAttrs, makeBind(name, def, makeNoPos()));
@@ -68,7 +68,7 @@ static Expr substArgs(Expr body, ATermList formals, Expr arg)
             matchNoDefFormal(*i, name) || matchDefFormal(*i, name, def);
             subs.remove(name);
         }
-        throw Error(format("unexpected function argument `%1%'")
+        throw TypeError(format("the function does not expect an argument named `%1%'")
             % aterm2String(subs.begin()->key));
     }
 
@@ -133,7 +133,8 @@ string evalString(EvalState & state, Expr e)
 {
     e = evalExpr(state, e);
     ATerm s;
-    if (!matchStr(e, s)) throw Error("string expected");
+    if (!matchStr(e, s))
+        throw TypeError(format("value is %1% while a string was expected") % showType(e));
     return aterm2String(s);
 }
 
@@ -142,7 +143,8 @@ Path evalPath(EvalState & state, Expr e)
 {
     e = evalExpr(state, e);
     ATerm s;
-    if (!matchPath(e, s)) throw Error("path expected");
+    if (!matchPath(e, s))
+        throw TypeError(format("value is %1% while a path was expected") % showType(e));
     return aterm2String(s);
 }
 
@@ -152,7 +154,7 @@ bool evalBool(EvalState & state, Expr e)
     e = evalExpr(state, e);
     if (e == eTrue) return true;
     else if (e == eFalse) return false;
-    else throw Error("boolean expected");
+    else throw TypeError(format("value is %1% while a boolean was expected") % showType(e));
 }
 
 
@@ -160,7 +162,8 @@ ATermList evalList(EvalState & state, Expr e)
 {
     e = evalExpr(state, e);
     ATermList list;
-    if (!matchList(e, list)) throw Error("list expected");
+    if (!matchList(e, list))
+        throw TypeError(format("value is %1% while a list was expected") % showType(e));
     return list;
 }
 
@@ -226,13 +229,13 @@ string coerceToStringWithContext(EvalState & state,
         Expr a = attrs.get(toATerm("type"));
         if (a && evalString(state, a) == "derivation") {
             a = attrs.get(toATerm("outPath"));
-            if (!a) throw Error("output path missing from derivation");
+            if (!a) throw TypeError("output path missing from derivation");
             context = ATinsert(context, e);
             return evalPath(state, a);
         }
     }
     
-    throw Error("cannot coerce value to string");
+    throw TypeError(format("cannot coerce %1% to a string") % showType(e));
 }
 
 
@@ -294,7 +297,7 @@ Expr evalExpr2(EvalState & state, Expr e)
     if (matchVar(e, name)) {
         ATerm primOp = state.primOps.get(name);
         if (!primOp)
-            throw Error(format("impossible: undefined variable `%1%'") % aterm2String(name));
+            throw EvalError(format("impossible: undefined variable `%1%'") % aterm2String(name));
         int arity;
         ATermBlob fun;
         if (!matchPrimOpDef(primOp, arity, fun)) abort();
@@ -355,7 +358,7 @@ Expr evalExpr2(EvalState & state, Expr e)
             }
         }
         
-        else throw Error("function or primop expected in function call");
+        else throw TypeError("the left-hand side of the function call is neither a function nor a primop (built-in operation)");
     }
 
     /* Attribute selection. */
@@ -363,7 +366,7 @@ Expr evalExpr2(EvalState & state, Expr e)
         ATerm pos;
         string s1 = aterm2String(name);
         Expr a = queryAttr(evalExpr(state, e1), s1, pos);
-        if (!a) throw Error(format("attribute `%1%' missing") % s1);
+        if (!a) throw EvalError(format("attribute `%1%' missing") % s1);
         try {
             return evalExpr(state, a);
         } catch (Error & e) {
@@ -451,26 +454,33 @@ Expr evalExpr2(EvalState & state, Expr e)
     }
 
     /* String or path concatenation. */
-    if (matchOpPlus(e, e1, e2)) {
+    ATermList es;
+    if (matchOpPlus(e, e1, e2) || matchConcatStrings(e, es)) {
         ATermVector args;
-        args.push_back(e1);
-        args.push_back(e2);
-        return concatStrings(state, args);
+        if (matchOpPlus(e, e1, e2)) {
+            args.push_back(e1);
+            args.push_back(e2);
+        } else
+            for (ATermIterator i(es); i; ++i) args.push_back(*i);
+        
+        try {
+            return concatStrings(state, args);
+        } catch (Error & e) {
+            e.addPrefix(format("in a string concatenation: "));
+            throw;
+        }
     }
 
     /* List concatenation. */
     if (matchOpConcat(e, e1, e2)) {
-        ATermList l1 = evalList(state, e1);
-        ATermList l2 = evalList(state, e2);
-        return makeList(ATconcat(l1, l2));
-    }
-
-    /* String concatenation. */
-    ATermList es;
-    if (matchConcatStrings(e, es)) {
-        ATermVector args;
-        for (ATermIterator i(es); i; ++i) args.push_back(*i);
-        return concatStrings(state, args);
+        try {
+            ATermList l1 = evalList(state, e1);
+            ATermList l2 = evalList(state, e2);
+            return makeList(ATconcat(l1, l2));
+        } catch (Error & e) {
+            e.addPrefix(format("in a list concatenation: "));
+            throw;
+        }
     }
 
     /* Barf. */
@@ -492,7 +502,7 @@ Expr evalExpr(EvalState & state, Expr e)
     Expr nf = state.normalForms.get(e);
     if (nf) {
         if (nf == makeBlackHole())
-            throw Error("infinite recursion encountered");
+            throw EvalError("infinite recursion encountered");
         state.nrCached++;
         return nf;
     }
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index c881969ad..a3815733e 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -41,10 +41,6 @@ struct EvalState
 };
 
 
-MakeError(EvalError, Error)
-MakeError(AssertionError, EvalError)
-
-
 /* Evaluate an expression to normal form. */
 Expr evalExpr(EvalState & state, Expr e);
 
diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc
index 8e439f08b..82e34f776 100644
--- a/src/libexpr/get-drvs.cc
+++ b/src/libexpr/get-drvs.cc
@@ -16,7 +16,7 @@ string DrvInfo::queryOutPath(EvalState & state) const
 {
     if (outPath == "") {
         Expr a = attrs->get(toATerm("outPath"));
-        if (!a) throw Error("output path missing");
+        if (!a) throw TypeError("output path missing");
         (string &) outPath = evalPath(state, a);
     }
     return outPath;
@@ -81,7 +81,7 @@ static bool getDerivation(EvalState & state, Expr e,
     
         a = attrs->get(toATerm("name"));
         /* !!! We really would like to have a decent back trace here. */
-        if (!a) throw Error("derivation name missing");
+        if (!a) throw TypeError("derivation name missing");
         drv.name = evalString(state, a);
 
         a = attrs->get(toATerm("system"));
@@ -123,7 +123,7 @@ static void getDerivations(EvalState & state, Expr e,
         for (ATermIterator i(formals); i; ++i) {
             Expr name, def;
             if (matchNoDefFormal(*i, name))
-                throw Error(format("expression evaluates to a function with no-default arguments (`%1%')")
+                throw TypeError(format("cannot auto-call a function that has an argument without a default value (`%1%')")
                     % aterm2String(name));
             else if (!matchDefFormal(*i, name, def))
                 abort(); /* can't happen */
@@ -224,7 +224,7 @@ static void getDerivations(EvalState & state, Expr e,
         return;
     }
 
-    throw Error("expression does not evaluate to a derivation (or a set or list of those)");
+    throw TypeError("expression does not evaluate to a derivation (or a set or list of those)");
 }
 
 
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index cce8edc39..938670f5b 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -50,7 +50,7 @@ void queryAllAttrs(Expr e, ATermMap & attrs, bool withPos)
 {
     ATermList bnds;
     if (!matchAttrs(e, bnds))
-        throw Error("attribute set expected");
+        throw TypeError("attribute set expected");
 
     for (ATermIterator i(bnds); i; ++i) {
         ATerm name;
@@ -73,7 +73,7 @@ Expr queryAttr(Expr e, const string & name, ATerm & pos)
 {
     ATermList bnds;
     if (!matchAttrs(e, bnds))
-        throw Error("attribute set expected");
+        throw TypeError("attribute set expected");
 
     for (ATermIterator i(bnds); i; ++i) {
         ATerm name2, pos2;
@@ -214,7 +214,7 @@ static void checkVarDefs2(set<Expr> & done, const ATermMap & defs, Expr e)
 
     if (matchVar(e, name)) {
         if (!defs.get(name))
-            throw Error(format("undefined variable `%1%'")
+            throw EvalError(format("undefined variable `%1%'")
                 % aterm2String(name));
     }
 
@@ -289,3 +289,24 @@ Expr makeBool(bool b)
 {
     return b ? eTrue : eFalse;
 }
+
+
+string showType(Expr e)
+{
+    ATerm t1, t2, t3;
+    ATermList l1;
+    int i1;
+    if (matchStr(e, t1)) return "a string";
+    if (matchPath(e, t1)) return "a path";
+    if (matchUri(e, t1)) return "a path";
+    if (matchNull(e)) return "null";
+    if (matchInt(e, i1)) return "an integer";
+    if (matchBool(e, t1)) return "a boolean";
+    if (matchFunction(e, l1, t1, t2)) return "a function";
+    if (matchFunction1(e, t1, t2, t3)) return "a function";
+    if (matchAttrs(e, l1)) return "an attribute set";
+    if (matchList(e, l1)) return "a list";
+    if (matchContext(e, l1, t1)) return "a context containing " + showType(t1);
+    return "an unknown type";
+}
+
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index e3c7ee607..4dc235c8e 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -9,6 +9,11 @@
 #include "util.hh"
 
 
+MakeError(EvalError, Error)
+MakeError(AssertionError, EvalError)
+MakeError(TypeError, EvalError)
+
+
 /* Nix expressions are represented as ATerms.  The maximal sharing
    property of the ATerm library allows us to implement caching of
    normals forms efficiently. */
@@ -82,5 +87,7 @@ void checkVarDefs(const ATermMap & def, Expr e);
 /* Create an expression representing a boolean. */
 Expr makeBool(bool b);
 
+string showType(Expr e);
+
 
 #endif /* !__NIXEXPR_H */
diff --git a/src/libexpr/parser.cc b/src/libexpr/parser.cc
index 241a43734..5c2c85cd7 100644
--- a/src/libexpr/parser.cc
+++ b/src/libexpr/parser.cc
@@ -103,7 +103,7 @@ static void checkAttrs(ATermMap & names, ATermList bnds)
         ATerm pos;
         if (!matchBind(*i, name, e, pos)) abort(); /* can't happen */
         if (names.get(name))
-            throw Error(format("duplicate attribute `%1%' at %2%")
+            throw EvalError(format("duplicate attribute `%1%' at %2%")
                 % aterm2String(name) % showPos(pos));
         names.set(name, name);
     }
@@ -123,7 +123,7 @@ static void checkAttrSets(ATerm e)
                 !matchDefFormal(*i, name, deflt))
                 abort();
             if (names.get(name))
-                throw Error(format("duplicate formal function argument `%1%' at %2%")
+                throw EvalError(format("duplicate formal function argument `%1%' at %2%")
                     % aterm2String(name) % showPos(pos));
             names.set(name, name);
         }
@@ -168,12 +168,12 @@ static Expr parse(EvalState & state,
     int res = yyparse(scanner, &data);
     yylex_destroy(scanner);
     
-    if (res) throw Error(data.error);
+    if (res) throw EvalError(data.error);
 
     try {
         checkVarDefs(state.primOps, data.result);
     } catch (Error & e) {
-        throw Error(format("%1%, in `%2%'") % e.msg() % path);
+        throw EvalError(format("%1%, in `%2%'") % e.msg() % path);
     }
     
     checkAttrSets(data.result);
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 4cd180fd6..0dd73a016 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -26,19 +26,19 @@ static Expr primImport(EvalState & state, const ATermVector & args)
            Nix expression created at the derivation's output path. */
         if (a && evalString(state, a) == "derivation") {
             a = queryAttr(arg, "drvPath");
-            if (!a) throw Error("bad derivation in import");
+            if (!a) throw EvalError("bad derivation in import");
             Path drvPath = evalPath(state, a);
 
             buildDerivations(singleton<PathSet>(drvPath));
  
             a = queryAttr(arg, "outPath");
-            if (!a) throw Error("bad derivation in import");
+            if (!a) throw EvalError("bad derivation in import");
             path = evalPath(state, a);
         }
     }
 
     if (path == "")
-        throw Error("path or derivation expected in import");
+        throw TypeError("`import' requires a path or derivation as its argument");
     
     return evalFile(state, path);
 }
@@ -133,11 +133,11 @@ static void processBinding(EvalState & state, Expr e, Derivation & drv,
         
         if (a && evalString(state, a) == "derivation") {
             a = queryAttr(e, "drvPath");
-            if (!a) throw Error("derivation name missing");
+            if (!a) throw EvalError("derivation name missing");
             Path drvPath = evalPath(state, a);
 
             a = queryAttr(e, "outPath");
-            if (!a) throw Error("output path missing");
+            if (!a) throw EvalError("output path missing");
             /* !!! supports only single output path */
             Path outPath = evalPath(state, a);
 
@@ -148,7 +148,7 @@ static void processBinding(EvalState & state, Expr e, Derivation & drv,
         else if (a && evalString(state, a) == "storePath") {
 
             a = queryAttr(e, "outPath");
-            if (!a) throw Error("output path missing");
+            if (!a) throw EvalError("output path missing");
             /* !!! supports only single output path */
             Path outPath = evalPath(state, a);
 
@@ -156,7 +156,7 @@ static void processBinding(EvalState & state, Expr e, Derivation & drv,
             ss.push_back(outPath);
         }
 
-        else throw Error("invalid derivation attribute");
+        else throw TypeError("attribute sets in derivations must either be derivations or store paths");
     }
 
     else if (matchPath(e, s)) {
@@ -171,7 +171,7 @@ static void processBinding(EvalState & state, Expr e, Derivation & drv,
 
         else {
             if (isDerivation(srcPath))
-                throw Error(format("file names are not allowed to end in `%1%'")
+                throw EvalError(format("file names are not allowed to end in `%1%'")
                     % drvExtension);
             Path dstPath;
             if (state.srcToStore[srcPath] != "")
@@ -200,14 +200,14 @@ static void processBinding(EvalState & state, Expr e, Derivation & drv,
         Strings ss2;
         processBinding(state, evalExpr(state, e1), drv, ss2);
         if (ss2.size() != 1)
-            throw Error("left-hand side of `~' operator cannot be a list");
+            throw TypeError("left-hand side of `~' operator cannot be a list");
         e2 = evalExpr(state, e2);
         if (!(matchStr(e2, s) || matchPath(e2, s)))
-            throw Error("right-hand side of `~' operator must be a path or string");
+            throw TypeError("right-hand side of `~' operator must be a path or string");
         ss.push_back(canonPath(ss2.front() + "/" + aterm2String(s)));
     }
     
-    else throw Error("invalid derivation attribute");
+    else throw TypeError(format("%1% is not allowed as a derivation argument") % showType(e));
 }
 
 
@@ -240,7 +240,7 @@ static Expr primDerivationStrict(EvalState & state, const ATermVector & args)
     /* Figure out the name already (for stack backtraces). */
     Expr eDrvName = attrs.get(toATerm("name"));
     if (!eDrvName)
-        throw Error("required attribute `name' missing");
+        throw EvalError("required attribute `name' missing");
     ATerm posDrvName;
     if (!matchAttrRHS(eDrvName, eDrvName, posDrvName)) abort();
     string drvName = evalString(state, eDrvName);
@@ -291,16 +291,16 @@ static Expr primDerivationStrict(EvalState & state, const ATermVector & args)
             else if (key == "outputHashMode") {
                 if (s == "recursive") outputHashRecursive = true; 
                 else if (s == "flat") outputHashRecursive = false;
-                else throw Error(format("invalid value `%1%' for `outputHashMode' attribute") % s);
+                else throw EvalError(format("invalid value `%1%' for `outputHashMode' attribute") % s);
             }
         }
     }
     
     /* Do we have all required attributes? */
     if (drv.builder == "")
-        throw Error("required attribute `builder' missing");
+        throw EvalError("required attribute `builder' missing");
     if (drv.platform == "")
-        throw Error("required attribute `system' missing");
+        throw EvalError("required attribute `system' missing");
 
     /* If an output hash was given, check it. */
     if (outputHash == "")
@@ -308,7 +308,7 @@ static Expr primDerivationStrict(EvalState & state, const ATermVector & args)
     else {
         HashType ht = parseHashType(outputHashAlgo);
         if (ht == htUnknown)
-            throw Error(format("unknown hash algorithm `%1%'") % outputHashAlgo);
+            throw EvalError(format("unknown hash algorithm `%1%'") % outputHashAlgo);
         Hash h;
         if (outputHash.size() == Hash(ht).hashSize * 2)
             /* hexadecimal representation */
@@ -326,7 +326,7 @@ static Expr primDerivationStrict(EvalState & state, const ATermVector & args)
        alphanumerics and some other characters appear. */
     checkStoreName(drvName);
     if (isDerivation(drvName))
-        throw Error(format("derivation names are not allowed to end in `%1%'")
+        throw EvalError(format("derivation names are not allowed to end in `%1%'")
             % drvExtension);
 
     /* !!! the name should not end in the derivation extension (.drv).
@@ -457,7 +457,7 @@ static Expr primIsNull(EvalState & state, const ATermVector & args)
 
 static Path findDependency(Path dir, string dep)
 {
-    if (dep[0] == '/') throw Error(
+    if (dep[0] == '/') throw EvalError(
         format("illegal absolute dependency `%1%'") % dep);
 
     Path p = canonPath(dir + "/" + dep);
@@ -515,7 +515,7 @@ static Expr primDependencyClosure(EvalState & state, const ATermVector & args)
 
     /* Get the start set. */
     Expr startSet = queryAttr(attrs, "startSet");
-    if (!startSet) throw Error("attribute `startSet' required");
+    if (!startSet) throw EvalError("attribute `startSet' required");
     ATermList startSet2 = evalList(state, startSet);
 
     Path pivot;
@@ -538,7 +538,7 @@ static Expr primDependencyClosure(EvalState & state, const ATermVector & args)
     }
 
     Expr scanner = queryAttr(attrs, "scanner");
-    if (!scanner) throw Error("attribute `scanner' required");
+    if (!scanner) throw EvalError("attribute `scanner' required");
     
     /* Construct the dependency closure by querying the dependency of
        each path in `workSet', adding the dependencies to