From 4c50f5d130fc3adc3b048923e267d936df5b6250 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 3 Sep 2023 13:44:32 +0200 Subject: [PATCH 1/3] traces: Do not print unknown location Solves 1/3 of the infinite recursion at unknown location meme. See #8879 for ensuring we always have a trace (for stack overflows) We might want to re-add this for finding missing location info *while hacking on that problem only*. --- src/libutil/error.cc | 12 ++++-------- src/libutil/error.hh | 7 +++++++ tests/lang/eval-fail-fromTOML-timestamps.err.exp | 2 -- tests/lang/eval-fail-hashfile-missing.err.exp | 4 ---- tests/lang/eval-fail-set-override.err.exp | 2 -- tests/lang/eval-fail-substring.err.exp | 2 -- tests/lang/eval-fail-to-path.err.exp | 2 -- 7 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 4a1b346ef..3917fffc3 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -203,8 +203,6 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s std::ostringstream oss; - auto noSource = ANSI_ITALIC " (source not available)" ANSI_NORMAL "\n"; - /* * Traces * ------ @@ -320,7 +318,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s oss << "\n" << "… " << trace.hint.str() << "\n"; - if (trace.pos) { + if (trace.pos && *trace.pos) { count++; oss << "\n" << ellipsisIndent << ANSI_BLUE << "at " ANSI_WARNING << *trace.pos << ANSI_NORMAL << ":"; @@ -329,8 +327,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s oss << "\n"; printCodeLines(oss, "", *trace.pos, *loc); oss << "\n"; - } else - oss << noSource; + } } } oss << "\n" << prefix; @@ -338,15 +335,14 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s oss << einfo.msg << "\n"; - if (einfo.errPos) { + if (einfo.errPos && *einfo.errPos) { oss << "\n" << ANSI_BLUE << "at " ANSI_WARNING << *einfo.errPos << ANSI_NORMAL << ":"; if (auto loc = einfo.errPos->getCodeLines()) { oss << "\n"; printCodeLines(oss, "", *einfo.errPos, *loc); oss << "\n"; - } else - oss << noSource; + } } auto suggestions = einfo.suggestions.trim(); diff --git a/src/libutil/error.hh b/src/libutil/error.hh index be5c5e252..c04dcbd77 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -70,6 +70,13 @@ struct AbstractPos uint32_t line = 0; uint32_t column = 0; + /** + * An AbstractPos may be a "null object", representing an unknown position. + * + * Return true if this position is known. + */ + inline operator bool() const { return line != 0; }; + /** * Return the contents of the source file. */ diff --git a/tests/lang/eval-fail-fromTOML-timestamps.err.exp b/tests/lang/eval-fail-fromTOML-timestamps.err.exp index f6bd19f5a..5b60d253d 100644 --- a/tests/lang/eval-fail-fromTOML-timestamps.err.exp +++ b/tests/lang/eval-fail-fromTOML-timestamps.err.exp @@ -8,5 +8,3 @@ error: 2| key = "value" error: while parsing a TOML string: Dates and times are not supported - - at «none»:0: (source not available) diff --git a/tests/lang/eval-fail-hashfile-missing.err.exp b/tests/lang/eval-fail-hashfile-missing.err.exp index 8e77dec1e..6d38608c0 100644 --- a/tests/lang/eval-fail-hashfile-missing.err.exp +++ b/tests/lang/eval-fail-hashfile-missing.err.exp @@ -10,10 +10,6 @@ error: … while evaluating the first argument passed to builtins.toString - at «none»:0: (source not available) - … while calling the 'hashFile' builtin - at «none»:0: (source not available) - error: opening file '/pwd/lang/this-file-is-definitely-not-there-7392097': No such file or directory diff --git a/tests/lang/eval-fail-set-override.err.exp b/tests/lang/eval-fail-set-override.err.exp index beb29d678..71481683d 100644 --- a/tests/lang/eval-fail-set-override.err.exp +++ b/tests/lang/eval-fail-set-override.err.exp @@ -1,6 +1,4 @@ error: … while evaluating the `__overrides` attribute - at «none»:0: (source not available) - error: value is an integer while a set was expected diff --git a/tests/lang/eval-fail-substring.err.exp b/tests/lang/eval-fail-substring.err.exp index dc26a00bd..5c58be29a 100644 --- a/tests/lang/eval-fail-substring.err.exp +++ b/tests/lang/eval-fail-substring.err.exp @@ -8,5 +8,3 @@ error: 2| error: negative start position in 'substring' - - at «none»:0: (source not available) diff --git a/tests/lang/eval-fail-to-path.err.exp b/tests/lang/eval-fail-to-path.err.exp index 43ed2bdfc..4ffa2cf6d 100644 --- a/tests/lang/eval-fail-to-path.err.exp +++ b/tests/lang/eval-fail-to-path.err.exp @@ -9,6 +9,4 @@ error: … while evaluating the first argument passed to builtins.toPath - at «none»:0: (source not available) - error: string 'foo/bar' doesn't represent an absolute path From f1aeeea32b113ac5ae9518221015a945625fa112 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 3 Sep 2023 14:00:55 +0200 Subject: [PATCH 2/3] traces: DRY printPosMaybe --- src/libutil/error.cc | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 3917fffc3..5b604c328 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -155,6 +155,25 @@ static std::string indent(std::string_view indentFirst, std::string_view indentR return res; } +/** + * Print a position, if it is known. + * + * @return true if a position was printed. + */ +static bool printPosMaybe(std::ostream & oss, std::string_view indent, const std::shared_ptr & pos) { + bool hasPos = pos && *pos; + if (hasPos) { + oss << "\n" << indent << ANSI_BLUE << "at " ANSI_WARNING << *pos << ANSI_NORMAL << ":"; + + if (auto loc = pos->getCodeLines()) { + oss << "\n"; + printCodeLines(oss, "", *pos, *loc); + oss << "\n"; + } + } + return hasPos; +} + std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool showTrace) { std::string prefix; @@ -318,32 +337,15 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s oss << "\n" << "… " << trace.hint.str() << "\n"; - if (trace.pos && *trace.pos) { + if (printPosMaybe(oss, ellipsisIndent, trace.pos)) count++; - - oss << "\n" << ellipsisIndent << ANSI_BLUE << "at " ANSI_WARNING << *trace.pos << ANSI_NORMAL << ":"; - - if (auto loc = trace.pos->getCodeLines()) { - oss << "\n"; - printCodeLines(oss, "", *trace.pos, *loc); - oss << "\n"; - } - } } oss << "\n" << prefix; } oss << einfo.msg << "\n"; - if (einfo.errPos && *einfo.errPos) { - oss << "\n" << ANSI_BLUE << "at " ANSI_WARNING << *einfo.errPos << ANSI_NORMAL << ":"; - - if (auto loc = einfo.errPos->getCodeLines()) { - oss << "\n"; - printCodeLines(oss, "", *einfo.errPos, *loc); - oss << "\n"; - } - } + printPosMaybe(oss, "", einfo.errPos); auto suggestions = einfo.suggestions.trim(); if (!suggestions.suggestions.empty()) { From 477bc617bba5617d1d1da3ce039b5bd296a3e56e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 3 Sep 2023 14:08:03 +0200 Subject: [PATCH 3/3] traces: Add _NIX_DEVELOPER_SHOW_UNKNOWN_LOCATIONS --- src/libutil/error.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 5b604c328..dd9612471 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -155,6 +155,15 @@ static std::string indent(std::string_view indentFirst, std::string_view indentR return res; } +/** + * A development aid for finding missing positions, to improve error messages. Example use: + * + * NIX_DEVELOPER_SHOW_UNKNOWN_LOCATIONS=1 _NIX_TEST_ACCEPT=1 make tests/lang.sh.test + * git diff -U20 tests + * + */ +static bool printUnknownLocations = getEnv("_NIX_DEVELOPER_SHOW_UNKNOWN_LOCATIONS").has_value(); + /** * Print a position, if it is known. * @@ -170,6 +179,8 @@ static bool printPosMaybe(std::ostream & oss, std::string_view indent, const std printCodeLines(oss, "", *pos, *loc); oss << "\n"; } + } else if (printUnknownLocations) { + oss << "\n" << indent << ANSI_BLUE << "at " ANSI_RED << "UNKNOWN LOCATION" << ANSI_NORMAL << "\n"; } return hasPos; }