From a9b813cc3bcf89f03de0db96fc2e88d1c83b8303 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 8 Mar 2024 09:47:09 +0100 Subject: [PATCH] Merge pull request #10066 from 9999years/print-all-frames Do not skip any stack frames when `--show-trace` is given (cherry picked from commit 0b47783d0a879875d558f0b56e49584f25ceb2d0) Change-Id: Ia0f18266dbcf97543110110c655c219c7a3e3270 --- src/libexpr/eval-error.cc | 14 +++----------- src/libexpr/eval-error.hh | 2 +- src/libexpr/eval.cc | 15 ++++++++------- src/libexpr/eval.hh | 6 ++++-- src/libexpr/primops.cc | 7 +++---- src/libutil/error.cc | 10 +++------- src/libutil/error.hh | 3 +-- .../lang/eval-fail-duplicate-traces.err.exp | 7 +++++++ ...fail-foldlStrict-strict-op-application.err.exp | 7 +++++++ .../lang/eval-fail-mutual-recursion.err.exp | 7 +++++++ 10 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/libexpr/eval-error.cc b/src/libexpr/eval-error.cc index f4cdeec5c..8db03610b 100644 --- a/src/libexpr/eval-error.cc +++ b/src/libexpr/eval-error.cc @@ -28,15 +28,7 @@ template EvalErrorBuilder & EvalErrorBuilder::withTrace(PosIdx pos, const std::string_view text) { error.err.traces.push_front( - Trace{.pos = error.state.positions[pos], .hint = HintFmt(std::string(text)), .frame = false}); - return *this; -} - -template -EvalErrorBuilder & EvalErrorBuilder::withFrameTrace(PosIdx pos, const std::string_view text) -{ - error.err.traces.push_front( - Trace{.pos = error.state.positions[pos], .hint = HintFmt(std::string(text)), .frame = true}); + Trace{.pos = error.state.positions[pos], .hint = HintFmt(std::string(text))}); return *this; } @@ -63,9 +55,9 @@ EvalErrorBuilder & EvalErrorBuilder::withFrame(const Env & env, const Expr } template -EvalErrorBuilder & EvalErrorBuilder::addTrace(PosIdx pos, HintFmt hint, bool frame) +EvalErrorBuilder & EvalErrorBuilder::addTrace(PosIdx pos, HintFmt hint) { - error.addTrace(error.state.positions[pos], hint, frame); + error.addTrace(error.state.positions[pos], hint); return *this; } diff --git a/src/libexpr/eval-error.hh b/src/libexpr/eval-error.hh index 2bb607280..43e6718b0 100644 --- a/src/libexpr/eval-error.hh +++ b/src/libexpr/eval-error.hh @@ -90,7 +90,7 @@ public: [[nodiscard, gnu::noinline]] EvalErrorBuilder & withFrame(const Env & e, const Expr & ex); - [[nodiscard, gnu::noinline]] EvalErrorBuilder & addTrace(PosIdx pos, HintFmt hint, bool frame = false); + [[nodiscard, gnu::noinline]] EvalErrorBuilder & addTrace(PosIdx pos, HintFmt hint); template [[nodiscard, gnu::noinline]] EvalErrorBuilder & diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index e15781404..bb3e6f3bd 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -812,14 +812,16 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & } } -void EvalState::addErrorTrace(Error & e, const char * s, const std::string & s2) const +template +void EvalState::addErrorTrace(Error & e, const Args & ... formatArgs) const { - e.addTrace(nullptr, s, s2); + e.addTrace(nullptr, HintFmt(formatArgs...)); } -void EvalState::addErrorTrace(Error & e, const PosIdx pos, const char * s, const std::string & s2, bool frame) const +template +void EvalState::addErrorTrace(Error & e, const PosIdx pos, const Args & ... formatArgs) const { - e.addTrace(positions[pos], HintFmt(s, s2), frame); + e.addTrace(positions[pos], HintFmt(formatArgs...)); } template @@ -1606,9 +1608,8 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & "while calling %s", lambda.name ? concatStrings("'", symbols[lambda.name], "'") - : "anonymous lambda", - true); - if (pos) addErrorTrace(e, pos, "from call site%s", "", true); + : "anonymous lambda"); + if (pos) addErrorTrace(e, pos, "from call site"); } throw; } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 5cdf34a47..d2d8140e9 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -434,10 +434,12 @@ public: std::string_view forceString(Value & v, NixStringContext & context, const PosIdx pos, std::string_view errorCtx); std::string_view forceStringNoCtx(Value & v, const PosIdx pos, std::string_view errorCtx); + template [[gnu::noinline]] - void addErrorTrace(Error & e, const char * s, const std::string & s2) const; + void addErrorTrace(Error & e, const Args & ... formatArgs) const; + template [[gnu::noinline]] - void addErrorTrace(Error & e, const PosIdx pos, const char * s, const std::string & s2, bool frame = false) const; + void addErrorTrace(Error & e, const PosIdx pos, const Args & ... formatArgs) const; public: /** diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index b5b28efcd..83eccb818 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -805,7 +805,7 @@ static void prim_addErrorContext(EvalState & state, const PosIdx pos, Value * * auto message = state.coerceToString(pos, *args[0], context, "while evaluating the error message passed to builtins.addErrorContext", false, false).toOwned(); - e.addTrace(nullptr, HintFmt(message), true); + e.addTrace(nullptr, HintFmt(message)); throw; } } @@ -1069,7 +1069,7 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * * e.addTrace(nullptr, HintFmt( "while evaluating derivation '%s'\n" " whose name attribute is located at %s", - drvName, pos), true); + drvName, pos)); throw; } } @@ -1227,8 +1227,7 @@ drvName, Bindings * attrs, Value & v) } catch (Error & e) { e.addTrace(state.positions[i->pos], - HintFmt("while evaluating attribute '%1%' of derivation '%2%'", key, drvName), - true); + HintFmt("while evaluating attribute '%1%' of derivation '%2%'", key, drvName)); throw; } } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 49a18fa1c..d30da58c0 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -10,9 +10,9 @@ namespace nix { const std::string nativeSystem = SYSTEM; -void BaseError::addTrace(std::shared_ptr && e, HintFmt hint, bool frame) +void BaseError::addTrace(std::shared_ptr && e, HintFmt 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 }); } void throwExceptionSelfCheck(){ @@ -60,8 +60,7 @@ inline bool operator<(const Trace& lhs, const Trace& rhs) // This formats a freshly formatted hint string and then throws it away, which // shouldn't be much of a problem because it only runs when pos is equal, and this function is // used for trace printing, which is infrequent. - return std::forward_as_tuple(lhs.hint.str(), lhs.frame) - < std::forward_as_tuple(rhs.hint.str(), rhs.frame); + return lhs.hint.str() < rhs.hint.str(); } inline bool operator> (const Trace& lhs, const Trace& rhs) { return rhs < lhs; } inline bool operator<=(const Trace& lhs, const Trace& rhs) { return !(lhs > rhs); } @@ -372,7 +371,6 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s // prepended to each element of the trace auto ellipsisIndent = " "; - bool frameOnly = false; if (!einfo.traces.empty()) { // Stack traces seen since we last printed a chunk of `duplicate frames // omitted`. @@ -383,7 +381,6 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s for (const auto & trace : einfo.traces) { if (trace.hint.str().empty()) continue; - if (frameOnly && !trace.frame) continue; if (!showTrace && count > 3) { oss << "\n" << ANSI_WARNING "(stack trace truncated; use '--show-trace' to show the full trace)" ANSI_NORMAL << "\n"; @@ -399,7 +396,6 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s printSkippedTracesMaybe(oss, ellipsisIndent, count, skippedTraces, tracesSeen); count++; - frameOnly = trace.frame; printTrace(oss, ellipsisIndent, count, trace); } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 0d978803e..9cb3a624d 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -64,7 +64,6 @@ void printCodeLines(std::ostream & out, struct Trace { std::shared_ptr pos; HintFmt hint; - bool frame; }; inline bool operator<(const Trace& lhs, const Trace& rhs); @@ -162,7 +161,7 @@ public: addTrace(std::move(e), HintFmt(std::string(fs), args...)); } - void addTrace(std::shared_ptr && e, HintFmt hint, bool frame = false); + void addTrace(std::shared_ptr && e, HintFmt hint); bool hasTrace() const { return !err.traces.empty(); } diff --git a/tests/functional/lang/eval-fail-duplicate-traces.err.exp b/tests/functional/lang/eval-fail-duplicate-traces.err.exp index 32ad9b376..cedaebd3b 100644 --- a/tests/functional/lang/eval-fail-duplicate-traces.err.exp +++ b/tests/functional/lang/eval-fail-duplicate-traces.err.exp @@ -41,4 +41,11 @@ error: | ^ 5| if n > 0 + … while calling the 'throw' builtin + at /pwd/lang/eval-fail-duplicate-traces.nix:7:10: + 6| then throwAfter (n - 1) + 7| else throw "Uh oh!"; + | ^ + 8| in + error: Uh oh! diff --git a/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp b/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp index 7cb08af8a..4903bc82d 100644 --- a/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp +++ b/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp @@ -27,4 +27,11 @@ error: | ^ 6| + … while calling the 'throw' builtin + at /pwd/lang/eval-fail-foldlStrict-strict-op-application.nix:5:9: + 4| null + 5| [ (_: throw "Not the final value, but is still forced!") (_: 23) ] + | ^ + 6| + error: Not the final value, but is still forced! diff --git a/tests/functional/lang/eval-fail-mutual-recursion.err.exp b/tests/functional/lang/eval-fail-mutual-recursion.err.exp index dc2e11766..c034afcd5 100644 --- a/tests/functional/lang/eval-fail-mutual-recursion.err.exp +++ b/tests/functional/lang/eval-fail-mutual-recursion.err.exp @@ -54,4 +54,11 @@ error: (21 duplicate frames omitted) + … while calling the 'throw' builtin + at /pwd/lang/eval-fail-mutual-recursion.nix:34:10: + 33| then throwAfterB true 10 + 34| else throw "Uh oh!"; + | ^ + 35| in + error: Uh oh!