From e93bf69b448d4f4ce6c3fe7b7acfa904afe058c0 Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Tue, 25 Oct 2022 01:46:10 +0200 Subject: [PATCH] Rework error throwing, and test it --- src/libexpr/eval-cache.cc | 16 +++--- src/libexpr/eval-inline.hh | 6 +- src/libexpr/eval.cc | 87 ++++++++++++---------------- src/libexpr/eval.hh | 63 ++++++++++---------- src/libexpr/primops.cc | 16 ++---- src/libexpr/tests/error_traces.cc | 96 +++++++++++++++++++++++++++++++ src/libexpr/tests/primops.cc | 6 ++ src/libutil/error.hh | 2 + 8 files changed, 190 insertions(+), 102 deletions(-) create mode 100644 src/libexpr/tests/error_traces.cc diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index df5348148..9a8a7fe62 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -571,14 +571,14 @@ std::string AttrCursor::getString() debug("using cached string attribute '%s'", getAttrPathStr()); return s->first; } else - root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); } } auto & v = forceValue(); if (v.type() != nString && v.type() != nPath) - root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); return v.type() == nString ? v.string.s : v.path; } @@ -602,7 +602,7 @@ string_t AttrCursor::getStringWithContext() return *s; } } else - root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); } } @@ -613,7 +613,7 @@ string_t AttrCursor::getStringWithContext() else if (v.type() == nPath) return {v.path, {}}; else - root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); } bool AttrCursor::getBool() @@ -626,14 +626,14 @@ bool AttrCursor::getBool() debug("using cached Boolean attribute '%s'", getAttrPathStr()); return *b; } else - root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); } } auto & v = forceValue(); if (v.type() != nBool) - root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); return v.boolean; } @@ -703,14 +703,14 @@ std::vector AttrCursor::getAttrs() debug("using cached attrset attribute '%s'", getAttrPathStr()); return *attrs; } else - root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); } } auto & v = forceValue(); if (v.type() != nAttrs) - root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); std::vector attrs; for (auto & attr : *getValue().attrs) diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index 279f5b8be..f0da688db 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -103,7 +103,7 @@ void EvalState::forceValue(Value & v, Callable getPos) else if (v.isApp()) callFunction(*v.app.left, *v.app.right, v, noPos); else if (v.isBlackhole()) - error("infinite recursion encountered").atPos(getPos()).debugThrow(); + error("infinite recursion encountered").atPos(getPos()).template debugThrow(); } @@ -121,7 +121,7 @@ inline void EvalState::forceAttrs(Value & v, Callable getPos, std::string_view e forceValue(v, noPos); if (v.type() != nAttrs) { PosIdx pos = getPos(); - this->error("value is %1% while a set was expected", showType(v)).withTrace(pos, errorCtx).debugThrow(); + error("value is %1% while a set was expected", showType(v)).withTrace(pos, errorCtx).debugThrow(); } } @@ -131,7 +131,7 @@ inline void EvalState::forceList(Value & v, const PosIdx pos, std::string_view e { forceValue(v, noPos); if (!v.isList()) { - this->error("value is %1% while a list was expected", showType(v)).withTrace(pos, errorCtx).debugThrow(); + error("value is %1% while a list was expected", showType(v)).withTrace(pos, errorCtx).debugThrow(); } } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3b8c0b9c1..0ff8bba4d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -418,41 +418,30 @@ static Strings parseNixPath(const std::string & s) return res; } -template -ErrorBuilder::ErrorBuilder(EvalState & s): - state(s), - info(ErrorInfo { .msg = hintformat(""), .errPos = std::nullopt, }) -{} - -template -ErrorBuilder & ErrorBuilder::atPos(PosIdx pos) +ErrorBuilder & ErrorBuilder::atPos(PosIdx pos) { info.errPos = state.positions[pos]; return *this; } -template -ErrorBuilder & ErrorBuilder::withTrace(PosIdx pos, const std::string_view text) +ErrorBuilder & ErrorBuilder::withTrace(PosIdx pos, const std::string_view text) { - info.traces.push_back(Trace{ .pos = state.positions[pos], .hint = hintformat(std::string(text)), .frame = false }); + info.traces.push_front(Trace{ .pos = state.positions[pos], .hint = hintformat(std::string(text)), .frame = false }); return *this; } -template -ErrorBuilder & ErrorBuilder::withFrameTrace(PosIdx pos, const std::string_view text) +ErrorBuilder & ErrorBuilder::withFrameTrace(PosIdx pos, const std::string_view text) { - info.traces.push_back(Trace{ .pos = state.positions[pos], .hint = hintformat(std::string(text)), .frame = true }); + info.traces.push_front(Trace{ .pos = state.positions[pos], .hint = hintformat(std::string(text)), .frame = true }); return *this; } -template -ErrorBuilder & ErrorBuilder::suggestions(Suggestions & s) { +ErrorBuilder & ErrorBuilder::withSuggestions(Suggestions & s) { info.suggestions = s; return *this; } -template -ErrorBuilder & ErrorBuilder::withFrame(const Env & env, const Expr & expr) { +ErrorBuilder & ErrorBuilder::withFrame(const Env & env, const Expr & expr) { // NOTE: This is abusing side-effects. // TODO: check compatibility with nested debugger calls. state.debugTraces.push_front(DebugTrace { @@ -972,7 +961,7 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) return j->value; } if (!env->prevWith) - error("undefined variable '%1%'", symbols[var.name]).atPos(var.pos).withFrame(*env, var).debugThrow(); + error("undefined variable '%1%'", symbols[var.name]).atPos(var.pos).withFrame(*env, var).debugThrow(); for (size_t l = env->prevWith; l; --l, env = env->up) ; } } @@ -1122,7 +1111,7 @@ void EvalState::cacheFile( // computation. if (mustBeTrivial && !(dynamic_cast(e))) - error("file '%s' must be an attribute set", path).debugThrow(); + error("file '%s' must be an attribute set", path).debugThrow(); eval(e, v); } catch (Error & e) { addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath); @@ -1146,7 +1135,7 @@ inline bool EvalState::evalBool(Env & env, Expr * e, const PosIdx pos, std::stri Value v; e->eval(*this, env, v); if (v.type() != nBool) - error("value is %1% while a Boolean was expected", showType(v)).withFrame(env, *e).debugThrow(); + error("value is %1% while a Boolean was expected", showType(v)).withFrame(env, *e).debugThrow(); return v.boolean; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -1160,7 +1149,7 @@ inline void EvalState::evalAttrs(Env & env, Expr * e, Value & v, const PosIdx po try { e->eval(*this, env, v); if (v.type() != nAttrs) - error("value is %1% while a set was expected", showType(v)).withFrame(env, *e).debugThrow(); + error("value is %1% while a set was expected", showType(v)).withFrame(env, *e).debugThrow(); } catch (Error & e) { e.addTrace(positions[pos], errorCtx); throw; @@ -1269,7 +1258,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) auto nameSym = state.symbols.create(nameVal.string.s); Bindings::iterator j = v.attrs->find(nameSym); if (j != v.attrs->end()) - state.error("dynamic attribute '%1%' already defined at %2%", state.symbols[nameSym], state.positions[j->pos]).atPos(i.pos).withFrame(env, *this).debugThrow(); + state.error("dynamic attribute '%1%' already defined at %2%", state.symbols[nameSym], state.positions[j->pos]).atPos(i.pos).withFrame(env, *this).debugThrow(); i.valueExpr->setName(nameSym); /* Keep sorted order so find can catch duplicates */ @@ -1372,8 +1361,8 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) for (auto & attr : *vAttrs->attrs) allAttrNames.insert(state.symbols[attr.name]); auto suggestions = Suggestions::bestMatches(allAttrNames, state.symbols[name]); - state.error("attribute '%1%' missing", state.symbols[name]) - .atPos(pos).suggestions(suggestions).withFrame(env, *this).debugThrow(); + state.error("attribute '%1%' missing", state.symbols[name]) + .atPos(pos).withSuggestions(suggestions).withFrame(env, *this).debugThrow(); } } vAttrs = j->value; @@ -1483,13 +1472,13 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & auto j = args[0]->attrs->get(i.name); if (!j) { if (!i.def) { - error("function '%1%' called without required argument '%2%'", + error("function '%1%' called without required argument '%2%'", (lambda.name ? std::string(symbols[lambda.name]) : "anonymous lambda"), symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") .withFrame(*fun.lambda.env, lambda) - .debugThrow(); + .debugThrow(); } env2.values[displ++] = i.def->maybeThunk(*this, env2); } else { @@ -1509,14 +1498,14 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & for (auto & formal : lambda.formals->formals) formalNames.insert(symbols[formal.name]); auto suggestions = Suggestions::bestMatches(formalNames, symbols[i.name]); - error("function '%1%' called with unexpected argument '%2%'", + error("function '%1%' called with unexpected argument '%2%'", (lambda.name ? std::string(symbols[lambda.name]) : "anonymous lambda"), symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") - .suggestions(suggestions) + .withSuggestions(suggestions) .withFrame(*fun.lambda.env, lambda) - .debugThrow(); + .debugThrow(); } abort(); // can't happen } @@ -1647,7 +1636,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & } else - error("attempt to call something which is not a function but %1%", showType(vCur)).atPos(pos).debugThrow(); + error("attempt to call something which is not a function but %1%", showType(vCur)).atPos(pos).debugThrow(); } vRes = vCur; @@ -1711,12 +1700,12 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) if (j != args.end()) { attrs.insert(*j); } else if (!i.def) { - error(R"(cannot evaluate a function that has an argument without a value ('%1%') + error(R"(cannot evaluate a function that has an argument without a value ('%1%') Nix attempted to evaluate a function as a top level expression; in this case it must have its arguments supplied either by default values, or passed explicitly with '--arg' or '--argstr'. See https://nixos.org/manual/nix/stable/expressions/language-constructs.html#functions.)", symbols[i.name]) - .atPos(i.pos).withFrame(*fun.lambda.env, *fun.lambda.fun).debugThrow(); + .atPos(i.pos).withFrame(*fun.lambda.env, *fun.lambda.fun).debugThrow(); } } } @@ -1749,7 +1738,7 @@ void ExprAssert::eval(EvalState & state, Env & env, Value & v) if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) { std::ostringstream out; cond->show(state.symbols, out); - state.error("assertion '%1%' failed", out.str()).atPos(pos).withFrame(env, *this).debugThrow(); + state.error("assertion '%1%' failed", out.str()).atPos(pos).withFrame(env, *this).debugThrow(); } body->eval(state, env, v); } @@ -1926,14 +1915,14 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) nf = n; nf += vTmp.fpoint; } else - state.error("cannot add %1% to an integer", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); + state.error("cannot add %1% to an integer", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); } else if (firstType == nFloat) { if (vTmp.type() == nInt) { nf += vTmp.integer; } else if (vTmp.type() == nFloat) { nf += vTmp.fpoint; } else - state.error("cannot add %1% to a float", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); + state.error("cannot add %1% to a float", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); } else { if (s.empty()) s.reserve(es->size()); /* skip canonization of first path, which would only be not @@ -1953,7 +1942,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) v.mkFloat(nf); else if (firstType == nPath) { if (!context.empty()) - state.error("a string that refers to a store path cannot be appended to a path").atPos(pos).withFrame(env, *this).debugThrow(); + state.error("a string that refers to a store path cannot be appended to a path").atPos(pos).withFrame(env, *this).debugThrow(); v.mkPath(canonPath(str())); } else v.mkStringMove(c_str(), context); @@ -2008,7 +1997,7 @@ NixInt EvalState::forceInt(Value & v, const PosIdx pos, std::string_view errorCt try { forceValue(v, pos); if (v.type() != nInt) - error("value is %1% while an integer was expected", showType(v)).debugThrow(); + error("value is %1% while an integer was expected", showType(v)).debugThrow(); return v.integer; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -2024,7 +2013,7 @@ NixFloat EvalState::forceFloat(Value & v, const PosIdx pos, std::string_view err if (v.type() == nInt) return v.integer; else if (v.type() != nFloat) - error("value is %1% while a float was expected", showType(v)).debugThrow(); + error("value is %1% while a float was expected", showType(v)).debugThrow(); return v.fpoint; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -2038,7 +2027,7 @@ bool EvalState::forceBool(Value & v, const PosIdx pos, std::string_view errorCtx try { forceValue(v, pos); if (v.type() != nBool) - error("value is %1% while a Boolean was expected", showType(v)).debugThrow(); + error("value is %1% while a Boolean was expected", showType(v)).debugThrow(); return v.boolean; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -2058,7 +2047,7 @@ void EvalState::forceFunction(Value & v, const PosIdx pos, std::string_view erro try { forceValue(v, pos); if (v.type() != nFunction && !isFunctor(v)) - error("value is %1% while a function was expected", showType(v)).debugThrow(); + error("value is %1% while a function was expected", showType(v)).debugThrow(); } catch (Error & e) { e.addTrace(positions[pos], errorCtx); throw; @@ -2071,7 +2060,7 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string try { forceValue(v, pos); if (v.type() != nString) - error("value is %1% while a string was expected", showType(v)).debugThrow(); + error("value is %1% while a string was expected", showType(v)).debugThrow(); return v.string.s; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -2132,7 +2121,7 @@ std::string_view EvalState::forceStringNoCtx(Value & v, const PosIdx pos, std::s { auto s = forceString(v, pos, errorCtx); if (v.string.context) { - error("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string.s, v.string.context[0]).withTrace(pos, errorCtx).debugThrow(); + error("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string.s, v.string.context[0]).withTrace(pos, errorCtx).debugThrow(); } return s; } @@ -2188,7 +2177,7 @@ BackedStringView EvalState::coerceToString(const PosIdx pos, Value & v, PathSet return std::move(*maybeString); auto i = v.attrs->find(sOutPath); if (i == v.attrs->end()) - error("cannot coerce a set to a string", showType(v)).withTrace(pos, errorCtx).debugThrow(); + error("cannot coerce a set to a string", showType(v)).withTrace(pos, errorCtx).debugThrow(); return coerceToString(pos, *i->value, context, coerceMore, copyToStore, canonicalizePath, errorCtx); } @@ -2223,14 +2212,14 @@ BackedStringView EvalState::coerceToString(const PosIdx pos, Value & v, PathSet } } - error("cannot coerce %1% to a string", showType(v)).withTrace(pos, errorCtx).debugThrow(); + error("cannot coerce %1% to a string", showType(v)).withTrace(pos, errorCtx).debugThrow(); } std::string EvalState::copyPathToStore(PathSet & context, const Path & path) { if (nix::isDerivation(path)) - error("file names are not allowed to end in '%1%'", drvExtension).debugThrow(); + error("file names are not allowed to end in '%1%'", drvExtension).debugThrow(); Path dstPath; auto i = srcToStore.find(path); @@ -2255,7 +2244,7 @@ Path EvalState::coerceToPath(const PosIdx pos, Value & v, PathSet & context, std { auto path = coerceToString(pos, v, context, false, false, true, errorCtx).toOwned(); if (path == "" || path[0] != '/') - error("string '%1%' doesn't represent an absolute path", path).withTrace(pos, errorCtx).debugThrow(); + error("string '%1%' doesn't represent an absolute path", path).withTrace(pos, errorCtx).debugThrow(); return path; } @@ -2265,7 +2254,7 @@ StorePath EvalState::coerceToStorePath(const PosIdx pos, Value & v, PathSet & co auto path = coerceToString(pos, v, context, false, false, true, errorCtx).toOwned(); if (auto storePath = store->maybeParseStorePath(path)) return *storePath; - error("path '%1%' is not in the Nix store", path).withTrace(pos, errorCtx).debugThrow(); + error("path '%1%' is not in the Nix store", path).withTrace(pos, errorCtx).debugThrow(); } @@ -2342,7 +2331,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v return v1.fpoint == v2.fpoint; default: - error("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow(); + error("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow(); } } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 5e88eb950..1f610a02f 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -87,48 +87,40 @@ struct DebugTrace { void debugError(Error * e, Env & env, Expr & expr); -template class ErrorBuilder { - + private: EvalState & state; ErrorInfo info; + ErrorBuilder(EvalState & s, ErrorInfo && i): state(s), info(i) { } + public: - [[gnu::noinline]] - ErrorBuilder(EvalState & s); - - [[gnu::noinline]] - ErrorBuilder & atPos(PosIdx pos); - template - [[gnu::noinline]] - ErrorBuilder & msg(const std::string & fs, const Args & ... args) + [[nodiscard, gnu::noinline]] + static ErrorBuilder * create(EvalState & s, const Args & ... args) { - hintformat f(fs); - formatHelper(f, args...); - info.msg = f; - return *this; + return new ErrorBuilder(s, ErrorInfo { .msg = hintfmt(args...) }); } - [[gnu::noinline]] - ErrorBuilder & withTrace(PosIdx pos, const std::string_view text); + [[nodiscard, gnu::noinline]] + ErrorBuilder & atPos(PosIdx pos); - [[gnu::noinline]] - ErrorBuilder & withFrameTrace(PosIdx pos, const std::string_view text); + [[nodiscard, gnu::noinline]] + ErrorBuilder & withTrace(PosIdx pos, const std::string_view text); - [[gnu::noinline]] - ErrorBuilder & suggestions(Suggestions & s); + [[nodiscard, gnu::noinline]] + ErrorBuilder & withFrameTrace(PosIdx pos, const std::string_view text); - [[gnu::noinline]] - ErrorBuilder & withFrame(const Env & e, const Expr & ex); + [[nodiscard, gnu::noinline]] + ErrorBuilder & withSuggestions(Suggestions & s); + [[nodiscard, gnu::noinline]] + ErrorBuilder & withFrame(const Env & e, const Expr & ex); + + template [[gnu::noinline, gnu::noreturn]] - void ErrorBuilder::debugThrow() { - // NOTE: We always use the -LastTrace version as we push the new trace in withFrame() - state.debugThrowLastTrace(ErrorType(info)); - } - + void debugThrow(); }; @@ -212,10 +204,12 @@ public: throw std::move(error); } - template - ErrorBuilder & error(const std::string & fs, const Args & ... args) { - ErrorBuilder * errorBuilder = new ErrorBuilder(*this); - errorBuilder->msg(fs, args ...); + ErrorBuilder * errorBuilder; + + template + [[nodiscard, gnu::noinline]] + ErrorBuilder & error(const Args & ... args) { + errorBuilder = ErrorBuilder::create(*this, args...); return *errorBuilder; } @@ -648,6 +642,13 @@ extern EvalSettings evalSettings; static const std::string corepkgsPrefix{"/__corepkgs__/"}; +template +void ErrorBuilder::debugThrow() +{ + // NOTE: We always use the -LastTrace version as we push the new trace in withFrame() + state.debugThrowLastTrace(ErrorType(info)); +} + } #include "eval-inline.hh" diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 508608183..b95f54851 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -560,10 +560,7 @@ struct CompareValues if (v1->type() == nInt && v2->type() == nFloat) return v1->integer < v2->fpoint; if (v1->type() != v2->type()) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("%scannot compare %s with %s", errorCtx, showType(*v1), showType(*v2)), - .errPos = std::nullopt, - })); + state.error("cannot compare %s with %s", showType(*v1), showType(*v2)).debugThrow(); switch (v1->type()) { case nInt: return v1->integer < v2->integer; @@ -581,14 +578,11 @@ struct CompareValues } else if (i == v1->listSize()) { return true; } else if (!state.eqValues(*v1->listElems()[i], *v2->listElems()[i], pos, errorCtx)) { - return (*this)(v1->listElems()[i], v2->listElems()[i], "while comparing two lists"); + return (*this)(v1->listElems()[i], v2->listElems()[i], "while comparing two list elements"); } } default: - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("%scannot compare %s with %s; values of that type are incomparable", errorCtx, showType(*v1), showType(*v2)), - .errPos = std::nullopt, - })); + state.error("cannot compare %s with %s; values of that type are incomparable", showType(*v1), showType(*v2)).debugThrow(); } } catch (Error & e) { e.addTrace(std::nullopt, errorCtx); @@ -614,7 +608,7 @@ static Bindings::iterator getAttr( Bindings::iterator value = attrSet->find(attrSym); if (value == attrSet->end()) { throw TypeError({ - .msg = hintfmt("attribute '%s' missing %s", state.symbols[attrSym], errorCtx), + .msg = hintfmt("attribute '%s' missing %s", state.symbols[attrSym], normaltxt(errorCtx)), .errPos = state.positions[attrSet->pos], }); // TODO XXX @@ -628,7 +622,7 @@ static Bindings::iterator getAttr( static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - state.forceAttrs(*args[0], noPos, "while evaluating the first argument pased to builtins.genericClosure"); + state.forceAttrs(*args[0], noPos, "while evaluating the first argument passed to builtins.genericClosure"); /* Get the start set. */ Bindings::iterator startSet = getAttr(state, state.sStartSet, args[0]->attrs, "in the attrset passed as argument to builtins.genericClosure"); diff --git a/src/libexpr/tests/error_traces.cc b/src/libexpr/tests/error_traces.cc new file mode 100644 index 000000000..6905dd561 --- /dev/null +++ b/src/libexpr/tests/error_traces.cc @@ -0,0 +1,96 @@ +#include +#include + +#include "libexprtests.hh" + +namespace nix { + + using namespace testing; + + // Testing eval of PrimOp's + class ErrorTraceTest : public LibExprTest { }; + +#define ASSERT_TRACE1(args, type, message) \ + ASSERT_THROW( \ + try { \ + eval("builtins." args); \ + } catch (BaseError & e) { \ + ASSERT_EQ(PrintToString(e.info().msg), \ + PrintToString(message)); \ + auto trace = e.info().traces.rbegin(); \ + ASSERT_EQ(PrintToString(trace->hint), \ + PrintToString(hintfmt("while calling the '%s' builtin", "genericClosure"))); \ + throw; \ + } \ + , type \ + ) + +#define ASSERT_TRACE2(args, type, message, context) \ + ASSERT_THROW( \ + try { \ + eval("builtins." args); \ + } catch (BaseError & e) { \ + ASSERT_EQ(PrintToString(e.info().msg), \ + PrintToString(message)); \ + auto trace = e.info().traces.rbegin(); \ + ASSERT_EQ(PrintToString(trace->hint), \ + PrintToString(context)); \ + ++trace; \ + ASSERT_EQ(PrintToString(trace->hint), \ + PrintToString(hintfmt("while calling the '%s' builtin", "genericClosure"))); \ + throw; \ + } \ + , type \ + ) + + //TEST_F(ErrorTraceTest, genericClosure) { + //ASSERT_THROW( + //try { + //eval("builtins.genericClosure 1 1"); + //} catch (BaseError & e) { + //ASSERT_EQ(PrintToString(e.info().msg), PrintToString(hintfmt("value is %s while a set was expected", "an integer"))); + //throw; + //}, TypeError); + //} + + TEST_F(ErrorTraceTest, genericClosure) { \ + ASSERT_TRACE2("genericClosure 1", + TypeError, + hintfmt("value is %s while a set was expected", "an integer"), + hintfmt("while evaluating the first argument passed to builtins.genericClosure")); + + ASSERT_TRACE1("genericClosure {}", + TypeError, + hintfmt("attribute '%s' missing %s", "startSet", normaltxt("in the attrset passed as argument to builtins.genericClosure"))); + + ASSERT_TRACE2("genericClosure { startSet = 1; }", + TypeError, + hintfmt("value is %s while a list was expected", "an integer"), + hintfmt("while evaluating the 'startSet' attribute passed as argument to builtins.genericClosure")); + // Okay: "genericClosure { startSet = []; }" + ASSERT_TRACE2("genericClosure { startSet = [{ key = 1;}]; operator = true; }", + TypeError, + hintfmt("value is %s while a function was expected", "a Boolean"), + hintfmt("while evaluating the 'operator' attribute passed as argument to builtins.genericClosure")); + + ASSERT_TRACE2("genericClosure { startSet = [{ key = 1;}]; operator = item: true; }", + TypeError, + hintfmt("value is %s while a list was expected", "a Boolean"), + hintfmt("while evaluating the return value of the `operator` passed to builtins.genericClosure")); // TODO: inconsistent naming + + ASSERT_TRACE2("genericClosure { startSet = [{ key = 1;}]; operator = item: [ true ]; }", + TypeError, + hintfmt("value is %s while a set was expected", "a Boolean"), + hintfmt("while evaluating one of the elements generated by (or initially passed to) builtins.genericClosure")); + + ASSERT_TRACE1("genericClosure { startSet = [{ key = 1;}]; operator = item: [ {} ]; }", + TypeError, + hintfmt("attribute '%s' missing %s", "key", normaltxt("in one of the attrsets generated by (or initially passed to) builtins.genericClosure"))); + + ASSERT_TRACE2("genericClosure { startSet = [{ key = 1;}]; operator = item: [{ key = ''a''; }]; }", + EvalError, + hintfmt("cannot compare %s with %s", "a string", "an integer"), + hintfmt("while comparing the `key` attributes of two genericClosure elements")); + } + +} /* namespace nix */ diff --git a/src/libexpr/tests/primops.cc b/src/libexpr/tests/primops.cc index 16cf66d2c..a4102753c 100644 --- a/src/libexpr/tests/primops.cc +++ b/src/libexpr/tests/primops.cc @@ -836,4 +836,10 @@ namespace nix { for (const auto [n, elem] : enumerate(v.listItems())) ASSERT_THAT(*elem, IsStringEq(expected[n])); } + + TEST_F(PrimOpTest, genericClosure_not_strict) { + // Operator should not be used when startSet is empty + auto v = eval("builtins.genericClosure { startSet = []; }"); + ASSERT_THAT(v, IsListOfSize(0)); + } } /* namespace nix */ diff --git a/src/libutil/error.hh b/src/libutil/error.hh index bf99581e2..6db77bcbf 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -192,6 +192,8 @@ public: void addTrace(std::optional e, hintformat hint, bool frame = false); bool hasTrace() const { return !err.traces.empty(); } + + const ErrorInfo & info() { return err; }; }; #define MakeError(newClass, superClass) \