From 173dcb0af9249487c2d9ad5de7218fcf203873bd Mon Sep 17 00:00:00 2001 From: Florian Friesdorf Date: Tue, 22 Nov 2022 12:46:55 +0000 Subject: [PATCH 1/4] Don't reverse stack trace when showing When debugging nix expressions the outermost trace tends to be more useful than the innermost. It is therefore printed last to save developers from scrolling. --- src/libutil/error.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 9172f67a6..9cac6ac91 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -287,7 +287,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s // traces if (showTrace && !einfo.traces.empty()) { - for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) { + for (auto iter = einfo.traces.begin(); iter != einfo.traces.end(); ++iter) { oss << "\n" << "… " << iter->hint.str() << "\n"; if (iter->pos.has_value() && (*iter->pos)) { From d269976be6def2928e6a315ab2b85b947f4308f2 Mon Sep 17 00:00:00 2001 From: Florian Friesdorf Date: Tue, 22 Nov 2022 16:45:58 +0000 Subject: [PATCH 2/4] Show stack trace above error message Save developers from scrolling by displaying the error message last, below the stack trace. --- src/libutil/error.cc | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 9cac6ac91..449baaad1 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -262,6 +262,28 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s prefix += ":" ANSI_NORMAL " "; std::ostringstream oss; + + // traces + if (showTrace && !einfo.traces.empty()) { + for (auto iter = einfo.traces.begin(); iter != einfo.traces.end(); ++iter) { + oss << "\n" << "… " << iter->hint.str() << "\n"; + + if (iter->pos.has_value() && (*iter->pos)) { + auto pos = iter->pos.value(); + oss << "\n"; + printAtPos(pos, oss); + + auto loc = getCodeLines(pos); + if (loc.has_value()) { + oss << "\n"; + printCodeLines(oss, "", pos, *loc); + oss << "\n"; + } + } + } + oss << "\n" << prefix; + } + oss << einfo.msg << "\n"; if (einfo.errPos.has_value() && *einfo.errPos) { @@ -285,26 +307,6 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s "?" << std::endl; } - // traces - if (showTrace && !einfo.traces.empty()) { - for (auto iter = einfo.traces.begin(); iter != einfo.traces.end(); ++iter) { - oss << "\n" << "… " << iter->hint.str() << "\n"; - - if (iter->pos.has_value() && (*iter->pos)) { - auto pos = iter->pos.value(); - oss << "\n"; - printAtPos(pos, oss); - - auto loc = getCodeLines(pos); - if (loc.has_value()) { - oss << "\n"; - printCodeLines(oss, "", pos, *loc); - oss << "\n"; - } - } - } - } - out << indent(prefix, std::string(filterANSIEscapes(prefix, true).size(), ' '), chomp(oss.str())); return out; From 7b122d43a49a3bf05436cb2c9b23934ff4bcba2c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 28 Nov 2022 10:39:28 -0500 Subject: [PATCH 3/4] Fix stack context notes to not rely on order Make everything be in the form "while ..." (most things were already), and in particular *don't* use other propositions that must go after or before specific "while ..." clauses to make sense. --- src/libexpr/eval.cc | 2 +- src/libexpr/flake/flake.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 6955aacbf..0d9226d3b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1660,7 +1660,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & (lambda.name ? concatStrings("'", symbols[lambda.name], "'") : "anonymous lambda")); - addErrorTrace(e, pos, "from call site%s", ""); + addErrorTrace(e, pos, "while evaluating call site%s", ""); } throw; } diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 6b5d6f6b3..6344fb253 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -143,7 +143,7 @@ static FlakeInput parseFlakeInput(EvalState & state, } catch (Error & e) { e.addTrace( state.positions[attr.pos], - hintfmt("in flake attribute '%s'", state.symbols[attr.name])); + hintfmt("while evaluating flake attribute '%s'", state.symbols[attr.name])); throw; } } @@ -152,7 +152,7 @@ static FlakeInput parseFlakeInput(EvalState & state, try { input.ref = FlakeRef::fromAttrs(attrs); } catch (Error & e) { - e.addTrace(state.positions[pos], hintfmt("in flake input")); + e.addTrace(state.positions[pos], hintfmt("while evaluating flake input")); throw; } else { From 8618c6cc75e19ed658649bd806b218de954ea3bc Mon Sep 17 00:00:00 2001 From: Florian Friesdorf Date: Fri, 9 Dec 2022 17:36:25 +0000 Subject: [PATCH 4/4] Simplify loop, feedback from @tfc and @Ericson2314 --- src/libutil/error.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 449baaad1..3bb3efb0e 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -265,11 +265,11 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s // traces if (showTrace && !einfo.traces.empty()) { - for (auto iter = einfo.traces.begin(); iter != einfo.traces.end(); ++iter) { - oss << "\n" << "… " << iter->hint.str() << "\n"; + for (const auto & trace : einfo.traces) { + oss << "\n" << "… " << trace.hint.str() << "\n"; - if (iter->pos.has_value() && (*iter->pos)) { - auto pos = iter->pos.value(); + if (trace.pos.has_value() && (*trace.pos)) { + auto pos = trace.pos.value(); oss << "\n"; printAtPos(pos, oss);