From 4f3725b167cc0080c570a814e28c6181ef5c7f52 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 19 Jul 2006 15:36:15 +0000 Subject: [PATCH] * Better error messages (especially wrt types). --- src/libexpr/eval.cc | 64 ++++++++++++++++++++++++----------------- src/libexpr/eval.hh | 4 --- src/libexpr/get-drvs.cc | 8 +++--- src/libexpr/nixexpr.cc | 27 +++++++++++++++-- src/libexpr/nixexpr.hh | 7 +++++ src/libexpr/parser.cc | 8 +++--- src/libexpr/primops.cc | 40 +++++++++++++------------- 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 & 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(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