From 9b33ef3879a764bed4cc2404a08344c3a697a646 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 18 Jan 2023 01:19:07 +0100 Subject: Revert "Merge pull request #6204 from layus/coerce-string" This reverts commit a75b7ba30f1e4f8b15e810fd18e63ee9552e0815, reversing changes made to 9af16c5f742300e831a2cc400e43df1e22f87f31. --- src/libutil/error.cc | 122 +++------------------------------------------------ src/libutil/error.hh | 16 ++----- 2 files changed, 8 insertions(+), 130 deletions(-) (limited to 'src/libutil') diff --git a/src/libutil/error.cc b/src/libutil/error.cc index e4f0d4677..1a1aecea5 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -9,9 +9,9 @@ namespace nix { const std::string nativeSystem = SYSTEM; -void BaseError::addTrace(std::shared_ptr && e, hintformat hint, bool frame) +void BaseError::addTrace(std::shared_ptr && e, hintformat hint) { - err.traces.push_front(Trace { .pos = std::move(e), .hint = hint, .frame = frame }); + err.traces.push_front(Trace { .pos = std::move(e), .hint = hint }); } // c++ std::exception descendants must have a 'const char* what()' function. @@ -200,125 +200,13 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s auto noSource = ANSI_ITALIC " (source not available)" ANSI_NORMAL "\n"; - /* - * Traces - * ------ - * - * The semantics of traces is a bit weird. We have only one option to - * print them and to make them verbose (--show-trace). In the code they - * are always collected, but they are not printed by default. The code - * also collects more traces when the option is on. This means that there - * is no way to print the simplified traces at all. - * - * I (layus) designed the code to attach positions to a restricted set of - * messages. This means that we have a lot of traces with no position at - * all, including most of the base error messages. For example "type - * error: found a string while a set was expected" has no position, but - * will come with several traces detailing it's precise relation to the - * closest know position. This makes erroring without printing traces - * quite useless. - * - * This is why I introduced the idea to always print a few traces on - * error. The number 3 is quite arbitrary, and was selected so as not to - * clutter the console on error. For the same reason, a trace with an - * error position takes more space, and counts as two traces towards the - * limit. - * - * The rest is truncated, unless --show-trace is passed. This preserves - * the same bad semantics of --show-trace to both show the trace and - * augment it with new data. Not too sure what is the best course of - * action. - * - * The issue is that it is fundamentally hard to provide a trace for a - * lazy language. The trace will only cover the current spine of the - * evaluation, missing things that have been evaluated before. For - * example, most type errors are hard to inspect because there is not - * trace for the faulty value. These errors should really print the faulty - * value itself. - * - * In function calls, the --show-trace flag triggers extra traces for each - * function invocation. These work as scopes, allowing to follow the - * current spine of the evaluation graph. Without that flag, the error - * trace should restrict itself to a restricted prefix of that trace, - * until the first scope. If we ever get to such a precise error - * reporting, there would be no need to add an arbitrary limit here. We - * could always print the full trace, and it would just be small without - * the flag. - * - * One idea I had is for XxxError.addTrace() to perform nothing if one - * scope has already been traced. Alternatively, we could stop here when - * we encounter such a scope instead of after an arbitrary number of - * traces. This however requires to augment traces with the notion of - * "scope". - * - * This is particularly visible in code like evalAttrs(...) where we have - * to make a decision between the two following options. - * - * ``` long traces - * inline void EvalState::evalAttrs(Env & env, Expr * e, Value & v, const Pos & pos, std::string_view errorCtx) - * { - * try { - * e->eval(*this, env, v); - * if (v.type() != nAttrs) - * throwTypeError("value is %1% while a set was expected", v); - * } catch (Error & e) { - * e.addTrace(pos, errorCtx); - * throw; - * } - * } - * ``` - * - * ``` short traces - * inline void EvalState::evalAttrs(Env & env, Expr * e, Value & v, const Pos & pos, std::string_view errorCtx) - * { - * e->eval(*this, env, v); - * try { - * if (v.type() != nAttrs) - * throwTypeError("value is %1% while a set was expected", v); - * } catch (Error & e) { - * e.addTrace(pos, errorCtx); - * throw; - * } - * } - * ``` - * - * The second example can be rewritten more concisely, but kept in this - * form to highlight the symmetry. The first option adds more information, - * because whatever caused an error down the line, in the generic eval - * function, will get annotated with the code location that uses and - * required it. The second option is less verbose, but does not provide - * any context at all as to where and why a failing value was required. - * - * Scopes would fix that, by adding context only when --show-trace is - * passed, and keeping the trace terse otherwise. - * - */ - - // Enough indent to align with with the `... ` - // prepended to each element of the trace - auto ellipsisIndent = " "; - - bool frameOnly = false; - if (!einfo.traces.empty()) { - size_t count = 0; + // traces + if (showTrace && !einfo.traces.empty()) { for (const auto & trace : einfo.traces) { - if (!showTrace && count > 3) { - oss << "\n" << ANSI_WARNING "(stack trace truncated; use '--show-trace' to show the full trace)" ANSI_NORMAL << "\n"; - break; - } - - if (trace.hint.str().empty()) continue; - if (frameOnly && !trace.frame) continue; - - count++; - frameOnly = trace.frame; - oss << "\n" << "… " << trace.hint.str() << "\n"; if (trace.pos) { - count++; - - oss << "\n" << ellipsisIndent << ANSI_BLUE << "at " ANSI_WARNING << *trace.pos << ANSI_NORMAL << ":"; + oss << "\n" << ANSI_BLUE << "at " ANSI_WARNING << *trace.pos << ANSI_NORMAL << ":"; if (auto loc = trace.pos->getCodeLines()) { oss << "\n"; diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 7d236028c..c3bb8c0df 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -86,7 +86,6 @@ void printCodeLines(std::ostream & out, struct Trace { std::shared_ptr pos; hintformat hint; - bool frame; }; struct ErrorInfo { @@ -115,8 +114,6 @@ protected: public: unsigned int status = 1; // exit status - BaseError(const BaseError &) = default; - template BaseError(unsigned int status, const Args & ... args) : err { .level = lvlError, .msg = hintfmt(args...) } @@ -155,22 +152,15 @@ public: const std::string & msg() const { return calcWhat(); } const ErrorInfo & info() const { calcWhat(); return err; } - void pushTrace(Trace trace) - { - err.traces.push_front(trace); - } - template - void addTrace(std::shared_ptr && e, std::string_view fs, const Args & ... args) + void addTrace(std::shared_ptr && e, const std::string & fs, const Args & ... args) { - addTrace(std::move(e), hintfmt(std::string(fs), args...)); + addTrace(std::move(e), hintfmt(fs, args...)); } - void addTrace(std::shared_ptr && e, hintformat hint, bool frame = false); + void addTrace(std::shared_ptr && e, hintformat hint); bool hasTrace() const { return !err.traces.empty(); } - - const ErrorInfo & info() { return err; }; }; #define MakeError(newClass, superClass) \ -- cgit v1.2.3