From 54e8f550c9d5cc88c1161035d366871bb82d4a0c Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 13:44:08 -0600 Subject: [PATCH] addErrorTrace --- src/libexpr/eval.cc | 24 +-- src/libexpr/primops.cc | 10 +- src/libmain/shared.cc | 10 +- src/libutil/error.cc | 51 +++++- src/libutil/error.hh | 13 +- src/nix-env/nix-env.cc | 4 +- src/nix/repl.cc | 376 +++++++++++++++++++++-------------------- src/nix/search.cc | 2 +- 8 files changed, 270 insertions(+), 220 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b90a64357..c249add86 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -592,19 +592,19 @@ LocalNoInlineNoReturn(void throwUndefinedVarError(const Pos & pos, const char * }); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2)) +LocalNoInline(void addErrorTrace(Error & e, const char * s, const string & s2)) { - e.addPrefix(format(s) % s2); + e.addTrace(std::nullopt, hintfmt(s) % s2); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s, const ExprLambda & fun, const Pos & pos)) +LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const ExprLambda & fun)) { - e.addPrefix(format(s) % fun.showNamePos() % pos); + e.addTrace(pos, hintfmt(s) % fun.showNamePos()); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2, const Pos & pos)) +LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, const string & s2)) { - e.addPrefix(format(s) % s2 % pos); + e.addTrace(pos, hintfmt(s) % s2); } @@ -818,7 +818,7 @@ void EvalState::evalFile(const Path & path_, Value & v) try { eval(e, v); } catch (Error & e) { - addErrorPrefix(e, "while evaluating the file '%1%':\n", path2); + addErrorTrace(e, "while evaluating the file '%1%':", path2); throw; } @@ -1068,8 +1068,8 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) } catch (Error & e) { if (pos2 && pos2->file != state.sDerivationNix) - addErrorPrefix(e, "while evaluating the attribute '%1%' at %2%:\n", - showAttrPath(state, env, attrPath), *pos2); + addErrorTrace(e, *pos2, "while evaluating the attribute '%1%'", + showAttrPath(state, env, attrPath)); throw; } @@ -1241,7 +1241,9 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v, const Pos & po try { lambda.body->eval(*this, env2, v); } catch (Error & e) { - addErrorPrefix(e, "while evaluating %1%, called from %2%:\n", lambda, pos); + // TODO something different for 'called from' than usual addTrace? + // addErrorTrace(e, pos, "while evaluating %1%, called from %2%:", lambda); + addErrorTrace(e, pos, "while evaluating %1%:", lambda); throw; } else @@ -1516,7 +1518,7 @@ void EvalState::forceValueDeep(Value & v) try { recurse(*i.value); } catch (Error & e) { - addErrorPrefix(e, "while evaluating the attribute '%1%' at %2%:\n", i.name, *i.pos); + addErrorTrace(e, *i.pos, "while evaluating the attribute '%1%'", i.name); throw; } } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 7b80d76b9..791fef27d 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -471,7 +471,8 @@ static void prim_addErrorContext(EvalState & state, const Pos & pos, Value * * a v = *args[1]; } catch (Error & e) { PathSet context; - e.addPrefix(format("%1%\n") % state.coerceToString(pos, *args[0], context)); + // TODO: is this right, include this pos?? Test it. esp with LOC. + e.addTrace(pos, hintfmt("%1%") % state.coerceToString(pos, *args[0], context)); throw; } } @@ -563,7 +564,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * try { drvName = state.forceStringNoCtx(*attr->value, pos); } catch (Error & e) { - e.addPrefix(fmt("while evaluating the derivation attribute 'name' at %1%:\n", posDrvName)); + e.addTrace(posDrvName, hintfmt("while evaluating the derivation attribute 'name'")); throw; } @@ -696,8 +697,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * } } catch (Error & e) { - e.addPrefix(format("while evaluating the attribute '%1%' of the derivation '%2%' at %3%:\n") - % key % drvName % posDrvName); + e.addTrace(posDrvName, + hintfmt("while evaluating the attribute '%1%' of the derivation '%2%'", + key, drvName)); throw; } } diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 1cb422967..27a4d5fc1 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -323,11 +323,13 @@ int handleExceptions(const string & programName, std::function fun) printError("Try '%1% --help' for more information.", programName); return 1; } catch (BaseError & e) { - if (settings.showTrace && e.prefix() != "") - printError(e.prefix()); + // TODO showTrace as argument, or have calcWhat check settings? + // if (settings.showTrace && e.prefix() != "") + // printError(e.prefix()); logError(e.info()); - if (e.prefix() != "" && !settings.showTrace) - printError("(use '--show-trace' to show detailed location information)"); + // TODO fix to detect non-empty trace here. + // if (e.prefix() != "" && !settings.showTrace) + // printError("(use '--show-trace' to show detailed location information)"); return e.status; } catch (std::bad_alloc & e) { printError(error + "out of memory"); diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 5f5b54251..222d9cbef 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -12,20 +12,43 @@ const std::string nativeSystem = SYSTEM; // addPrefix is used for show-trace. Strings added with addPrefix // will print ahead of the error itself. -BaseError & BaseError::addPrefix(const FormatOrString & fs) -{ - prefix_ = fs.s + prefix_; - return *this; -} +// BaseError & BaseError::addPrefix(const FormatOrString & fs) +// { +// prefix_ = fs.s + prefix_; +// return *this; +// } + +// const string & prefix() const +// { +// // build prefix string on demand?? + +// } +// ; // { return prefix_; } + // addPrefix is used for show-trace. Strings added with addPrefix // will print ahead of the error itself. -BaseError & BaseError::addTrace(hintformat hint, ErrPos e) +BaseError & BaseError::addTrace(std::optional e, hintformat hint) { err.traces.push_front(Trace { .pos = e, .hint = hint}); return *this; } +// const string& BaseError::calcTrace() const +// { +// if (trace_.has_value()) +// return *trace_; +// else { +// err.name = sname(); + +// std::ostringstream oss; +// oss << err; +// trace_ = oss.str(); + +// return *trace_; +// } +// } + // c++ std::exception descendants must have a 'const char* what()' function. // This stringifies the error and caches it for use by what(), or similarly by msg(). const string& BaseError::calcWhat() const @@ -284,6 +307,22 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo) nl = true; } + // traces + for (auto iter = einfo.traces.begin(); iter != einfo.traces.end(); ++iter) + { + + try { + auto pos = *iter->pos; + out << iter->hint.str() << showErrPos(pos) << std::endl; + NixCode nc { .errPos = pos }; + getCodeLines(nc); + printCodeLines(out, prefix, nc); + } catch(const std::bad_optional_access& e) { + out << iter->hint.str() << std::endl; + } + + } + // description if (einfo.description != "") { if (nl) diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 10bdeea1d..75ba80365 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -94,7 +94,7 @@ struct NixCode { }; struct Trace { - ErrPos pos; + std::optional pos; hintformat hint; }; @@ -116,9 +116,12 @@ std::ostream& operator<<(std::ostream &out, const ErrorInfo &einfo); class BaseError : public std::exception { protected: - string prefix_; // used for location traces etc. + // string prefix_; // used for location traces etc. mutable ErrorInfo err; + // mutable std::optional trace_; + // const string& calcTrace() const; + mutable std::optional what_; const string& calcWhat() const; @@ -164,9 +167,9 @@ public: #endif const string & msg() const { return calcWhat(); } - const string & prefix() const { return prefix_; } - BaseError & addPrefix(const FormatOrString & fs); - BaseError & addTrace(ErrPos e, hintformat hint); + // const string & trace() const { return calcTrace(); } + // BaseError & addPrefix(const FormatOrString & fs); + BaseError & addTrace(std::optional e, hintformat hint); const ErrorInfo & info() { calcWhat(); return err; } }; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 8b0692035..f5dfbf9f6 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -593,7 +593,7 @@ static void upgradeDerivations(Globals & globals, } else newElems.push_back(i); } catch (Error & e) { - e.addPrefix(fmt("while trying to find an upgrade for '%s':\n", i.queryName())); + e.addTrace(std::nullopt, hintfmt("while trying to find an upgrade for '%s'", i.queryName())); throw; } } @@ -1185,7 +1185,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) } catch (AssertionError & e) { printMsg(lvlTalkative, "skipping derivation named '%1%' which gives an assertion failure", i.queryName()); } catch (Error & e) { - e.addPrefix(fmt("while querying the derivation named '%1%':\n", i.queryName())); + e.addTrace(std::nullopt, hintfmt("while querying the derivation named '%1%'", i.queryName())); throw; } } diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 617d49614..a75157a82 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -101,74 +101,74 @@ NixRepl::~NixRepl() static NixRepl * curRepl; // ugly static char * completionCallback(char * s, int *match) { - auto possible = curRepl->completePrefix(s); - if (possible.size() == 1) { - *match = 1; - auto *res = strdup(possible.begin()->c_str() + strlen(s)); - if (!res) throw Error("allocation failure"); - return res; - } else if (possible.size() > 1) { - auto checkAllHaveSameAt = [&](size_t pos) { - auto &first = *possible.begin(); - for (auto &p : possible) { - if (p.size() <= pos || p[pos] != first[pos]) - return false; - } - return true; - }; - size_t start = strlen(s); - size_t len = 0; - while (checkAllHaveSameAt(start + len)) ++len; - if (len > 0) { - *match = 1; - auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); - if (!res) throw Error("allocation failure"); - return res; + auto possible = curRepl->completePrefix(s); + if (possible.size() == 1) { + *match = 1; + auto *res = strdup(possible.begin()->c_str() + strlen(s)); + if (!res) throw Error("allocation failure"); + return res; + } else if (possible.size() > 1) { + auto checkAllHaveSameAt = [&](size_t pos) { + auto &first = *possible.begin(); + for (auto &p : possible) { + if (p.size() <= pos || p[pos] != first[pos]) + return false; + } + return true; + }; + size_t start = strlen(s); + size_t len = 0; + while (checkAllHaveSameAt(start + len)) ++len; + if (len > 0) { + *match = 1; + auto *res = strdup(std::string(*possible.begin(), start, len).c_str()); + if (!res) throw Error("allocation failure"); + return res; + } } - } - *match = 0; - return nullptr; + *match = 0; + return nullptr; } static int listPossibleCallback(char *s, char ***avp) { - auto possible = curRepl->completePrefix(s); + auto possible = curRepl->completePrefix(s); - if (possible.size() > (INT_MAX / sizeof(char*))) - throw Error("too many completions"); + if (possible.size() > (INT_MAX / sizeof(char*))) + throw Error("too many completions"); - int ac = 0; - char **vp = nullptr; + int ac = 0; + char **vp = nullptr; - auto check = [&](auto *p) { - if (!p) { - if (vp) { - while (--ac >= 0) - free(vp[ac]); - free(vp); - } - throw Error("allocation failure"); - } - return p; - }; + auto check = [&](auto *p) { + if (!p) { + if (vp) { + while (--ac >= 0) + free(vp[ac]); + free(vp); + } + throw Error("allocation failure"); + } + return p; + }; - vp = check((char **)malloc(possible.size() * sizeof(char*))); + vp = check((char **)malloc(possible.size() * sizeof(char*))); - for (auto & p : possible) - vp[ac++] = check(strdup(p.c_str())); + for (auto & p : possible) + vp[ac++] = check(strdup(p.c_str())); - *avp = vp; + *avp = vp; - return ac; + return ac; } namespace { - // Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. - volatile sig_atomic_t g_signal_received = 0; +// Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. +volatile sig_atomic_t g_signal_received = 0; - void sigintHandler(int signo) { - g_signal_received = signo; - } +void sigintHandler(int signo) { + g_signal_received = signo; +} } void NixRepl::mainLoop(const std::vector & files) @@ -211,12 +211,14 @@ void NixRepl::mainLoop(const std::vector & files) // input without clearing the input so far. continue; } else { - printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + printMsg(lvlError, e.msg()); } } catch (Error & e) { - printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + // printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + printMsg(lvlError, e.msg()); } catch (Interrupted & e) { - printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + // printMsg(lvlError, error + "%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + printMsg(lvlError, e.msg()); } // We handled the current input fully, so we should clear it @@ -233,24 +235,24 @@ bool NixRepl::getLine(string & input, const std::string &prompt) sigset_t savedSignalMask, set; auto setupSignals = [&]() { - act.sa_handler = sigintHandler; - sigfillset(&act.sa_mask); - act.sa_flags = 0; - if (sigaction(SIGINT, &act, &old)) - throw SysError("installing handler for SIGINT"); + act.sa_handler = sigintHandler; + sigfillset(&act.sa_mask); + act.sa_flags = 0; + if (sigaction(SIGINT, &act, &old)) + throw SysError("installing handler for SIGINT"); - sigemptyset(&set); - sigaddset(&set, SIGINT); - if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) - throw SysError("unblocking SIGINT"); - }; + sigemptyset(&set); + sigaddset(&set, SIGINT); + if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) + throw SysError("unblocking SIGINT"); + }; auto restoreSignals = [&]() { - if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) - throw SysError("restoring signals"); + if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) + throw SysError("restoring signals"); - if (sigaction(SIGINT, &old, 0)) - throw SysError("restoring handler for SIGINT"); - }; + if (sigaction(SIGINT, &old, 0)) + throw SysError("restoring handler for SIGINT"); + }; setupSignals(); char * s = readline(prompt.c_str()); @@ -264,7 +266,7 @@ bool NixRepl::getLine(string & input, const std::string &prompt) } if (!s) - return false; + return false; input += s; input += '\n'; return true; @@ -397,21 +399,21 @@ bool NixRepl::processLine(string line) if (command == ":?" || command == ":help") { std::cout - << "The following commands are available:\n" - << "\n" - << " Evaluate and print expression\n" - << " = Bind expression to variable\n" - << " :a Add attributes from resulting set to scope\n" - << " :b Build derivation\n" - << " :e Open the derivation in $EDITOR\n" - << " :i Build derivation, then install result into current profile\n" - << " :l Load Nix expression and add it to scope\n" - << " :p Evaluate and print expression recursively\n" - << " :q Exit nix-repl\n" - << " :r Reload all files\n" - << " :s Build dependencies of derivation, then start nix-shell\n" - << " :t Describe result of evaluation\n" - << " :u Build derivation, then start nix-shell\n"; + << "The following commands are available:\n" + << "\n" + << " Evaluate and print expression\n" + << " = Bind expression to variable\n" + << " :a Add attributes from resulting set to scope\n" + << " :b Build derivation\n" + << " :e Open the derivation in $EDITOR\n" + << " :i Build derivation, then install result into current profile\n" + << " :l Load Nix expression and add it to scope\n" + << " :p Evaluate and print expression recursively\n" + << " :q Exit nix-repl\n" + << " :r Reload all files\n" + << " :s Build dependencies of derivation, then start nix-shell\n" + << " :t Describe result of evaluation\n" + << " :u Build derivation, then start nix-shell\n"; } else if (command == ":a" || command == ":add") { @@ -637,118 +639,118 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m switch (v.type) { - case tInt: - str << ANSI_CYAN << v.integer << ANSI_NORMAL; - break; + case tInt: + str << ANSI_CYAN << v.integer << ANSI_NORMAL; + break; - case tBool: - str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL; - break; + case tBool: + str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL; + break; - case tString: - str << ANSI_YELLOW; - printStringValue(str, v.string.s); - str << ANSI_NORMAL; - break; + case tString: + str << ANSI_YELLOW; + printStringValue(str, v.string.s); + str << ANSI_NORMAL; + break; - case tPath: - str << ANSI_GREEN << v.path << ANSI_NORMAL; // !!! escaping? - break; + case tPath: + str << ANSI_GREEN << v.path << ANSI_NORMAL; // !!! escaping? + break; - case tNull: - str << ANSI_CYAN "null" ANSI_NORMAL; - break; + case tNull: + str << ANSI_CYAN "null" ANSI_NORMAL; + break; - case tAttrs: { - seen.insert(&v); + case tAttrs: { + seen.insert(&v); - bool isDrv = state->isDerivation(v); + bool isDrv = state->isDerivation(v); - if (isDrv) { - str << "«derivation "; - Bindings::iterator i = v.attrs->find(state->sDrvPath); - PathSet context; - Path drvPath = i != v.attrs->end() ? state->coerceToPath(*i->pos, *i->value, context) : "???"; - str << drvPath << "»"; + if (isDrv) { + str << "«derivation "; + Bindings::iterator i = v.attrs->find(state->sDrvPath); + PathSet context; + Path drvPath = i != v.attrs->end() ? state->coerceToPath(*i->pos, *i->value, context) : "???"; + str << drvPath << "»"; + } + + else if (maxDepth > 0) { + str << "{ "; + + typedef std::map Sorted; + Sorted sorted; + for (auto & i : *v.attrs) + sorted[i.name] = i.value; + + for (auto & i : sorted) { + if (isVarName(i.first)) + str << i.first; + else + printStringValue(str, i.first.c_str()); + str << " = "; + if (seen.find(i.second) != seen.end()) + str << "«repeated»"; + else + try { + printValue(str, *i.second, maxDepth - 1, seen); + } catch (AssertionError & e) { + str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; + } + str << "; "; + } + + str << "}"; + } else + str << "{ ... }"; + + break; } - else if (maxDepth > 0) { - str << "{ "; + case tList1: + case tList2: + case tListN: + seen.insert(&v); - typedef std::map Sorted; - Sorted sorted; - for (auto & i : *v.attrs) - sorted[i.name] = i.value; + str << "[ "; + if (maxDepth > 0) + for (unsigned int n = 0; n < v.listSize(); ++n) { + if (seen.find(v.listElems()[n]) != seen.end()) + str << "«repeated»"; + else + try { + printValue(str, *v.listElems()[n], maxDepth - 1, seen); + } catch (AssertionError & e) { + str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; + } + str << " "; + } + else + str << "... "; + str << "]"; + break; - for (auto & i : sorted) { - if (isVarName(i.first)) - str << i.first; - else - printStringValue(str, i.first.c_str()); - str << " = "; - if (seen.find(i.second) != seen.end()) - str << "«repeated»"; - else - try { - printValue(str, *i.second, maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << "; "; - } + case tLambda: { + std::ostringstream s; + s << v.lambda.fun->pos; + str << ANSI_BLUE "«lambda @ " << filterANSIEscapes(s.str()) << "»" ANSI_NORMAL; + break; + } - str << "}"; - } else - str << "{ ... }"; + case tPrimOp: + str << ANSI_MAGENTA "«primop»" ANSI_NORMAL; + break; - break; - } + case tPrimOpApp: + str << ANSI_BLUE "«primop-app»" ANSI_NORMAL; + break; - case tList1: - case tList2: - case tListN: - seen.insert(&v); + case tFloat: + str << v.fpoint; + break; - str << "[ "; - if (maxDepth > 0) - for (unsigned int n = 0; n < v.listSize(); ++n) { - if (seen.find(v.listElems()[n]) != seen.end()) - str << "«repeated»"; - else - try { - printValue(str, *v.listElems()[n], maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << " "; - } - else - str << "... "; - str << "]"; - break; - - case tLambda: { - std::ostringstream s; - s << v.lambda.fun->pos; - str << ANSI_BLUE "«lambda @ " << filterANSIEscapes(s.str()) << "»" ANSI_NORMAL; - break; - } - - case tPrimOp: - str << ANSI_MAGENTA "«primop»" ANSI_NORMAL; - break; - - case tPrimOpApp: - str << ANSI_BLUE "«primop-app»" ANSI_NORMAL; - break; - - case tFloat: - str << v.fpoint; - break; - - default: - str << ANSI_RED "«unknown»" ANSI_NORMAL; - break; + default: + str << ANSI_RED "«unknown»" ANSI_NORMAL; + break; } return str; @@ -771,10 +773,10 @@ struct CmdRepl : StoreCommand, MixEvalArgs Examples examples() override { return { - Example{ - "Display all special commands within the REPL:", - "nix repl\n nix-repl> :?" - } + Example{ + "Display all special commands within the REPL:", + "nix repl\n nix-repl> :?" + } }; } diff --git a/src/nix/search.cc b/src/nix/search.cc index ba72c1e79..d9c730796 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -216,7 +216,7 @@ struct CmdSearch : SourceExprCommand, MixJSON } catch (AssertionError & e) { } catch (Error & e) { if (!toplevel) { - e.addPrefix(fmt("While evaluating the attribute '%s':\n", attrPath)); + e.addTrace(std::nullopt, hintfmt("While evaluating the attribute '%s'", attrPath)); throw; } }