From c62c21e29af20f1c14a59ab37d7a25dd0b70f69e Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Thu, 1 Feb 2024 13:07:45 -0800 Subject: [PATCH 1/6] Move `PodIdx` to `pos-idx.hh` and `PosTable` to `pos-table.hh` --- src/libexpr/nixexpr.hh | 86 +--------------------------------------- src/libexpr/pos-idx.hh | 48 ++++++++++++++++++++++ src/libexpr/pos-table.hh | 83 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 84 deletions(-) create mode 100644 src/libexpr/pos-idx.hh create mode 100644 src/libexpr/pos-table.hh diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index b6189c2a9..da0ec6e9d 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -9,6 +9,8 @@ #include "error.hh" #include "chunked-vector.hh" #include "position.hh" +#include "pos-idx.hh" +#include "pos-table.hh" namespace nix { @@ -29,90 +31,6 @@ public: using EvalError::EvalError; }; -class PosIdx { - friend class PosTable; - -private: - uint32_t id; - - explicit PosIdx(uint32_t id): id(id) {} - -public: - PosIdx() : id(0) {} - - explicit operator bool() const { return id > 0; } - - bool operator <(const PosIdx other) const { return id < other.id; } - - bool operator ==(const PosIdx other) const { return id == other.id; } - - bool operator !=(const PosIdx other) const { return id != other.id; } -}; - -class PosTable -{ -public: - class Origin { - friend PosTable; - private: - // must always be invalid by default, add() replaces this with the actual value. - // subsequent add() calls use this index as a token to quickly check whether the - // current origins.back() can be reused or not. - mutable uint32_t idx = std::numeric_limits::max(); - - // Used for searching in PosTable::[]. - explicit Origin(uint32_t idx): idx(idx), origin{std::monostate()} {} - - public: - const Pos::Origin origin; - - Origin(Pos::Origin origin): origin(origin) {} - }; - - struct Offset { - uint32_t line, column; - }; - -private: - std::vector origins; - ChunkedVector offsets; - -public: - PosTable(): offsets(1024) - { - origins.reserve(1024); - } - - PosIdx add(const Origin & origin, uint32_t line, uint32_t column) - { - const auto idx = offsets.add({line, column}).second; - if (origins.empty() || origins.back().idx != origin.idx) { - origin.idx = idx; - origins.push_back(origin); - } - return PosIdx(idx + 1); - } - - Pos operator[](PosIdx p) const - { - if (p.id == 0 || p.id > offsets.size()) - return {}; - const auto idx = p.id - 1; - /* we want the last key <= idx, so we'll take prev(first key > idx). - this is guaranteed to never rewind origin.begin because the first - key is always 0. */ - const auto pastOrigin = std::upper_bound( - origins.begin(), origins.end(), Origin(idx), - [] (const auto & a, const auto & b) { return a.idx < b.idx; }); - const auto origin = *std::prev(pastOrigin); - const auto offset = offsets[idx]; - return {offset.line, offset.column, origin.origin}; - } -}; - -inline PosIdx noPos = {}; - - struct Env; struct Value; class EvalState; diff --git a/src/libexpr/pos-idx.hh b/src/libexpr/pos-idx.hh new file mode 100644 index 000000000..9949f1dc5 --- /dev/null +++ b/src/libexpr/pos-idx.hh @@ -0,0 +1,48 @@ +#pragma once + +#include + +namespace nix { + +class PosIdx +{ + friend class PosTable; + +private: + uint32_t id; + + explicit PosIdx(uint32_t id) + : id(id) + { + } + +public: + PosIdx() + : id(0) + { + } + + explicit operator bool() const + { + return id > 0; + } + + bool operator<(const PosIdx other) const + { + return id < other.id; + } + + bool operator==(const PosIdx other) const + { + return id == other.id; + } + + bool operator!=(const PosIdx other) const + { + return id != other.id; + } +}; + +inline PosIdx noPos = {}; + +} diff --git a/src/libexpr/pos-table.hh b/src/libexpr/pos-table.hh new file mode 100644 index 000000000..1decf3c85 --- /dev/null +++ b/src/libexpr/pos-table.hh @@ -0,0 +1,83 @@ +#pragma once + +#include +#include +#include + +#include "chunked-vector.hh" +#include "pos-idx.hh" +#include "position.hh" + +namespace nix { + +class PosTable +{ +public: + class Origin + { + friend PosTable; + private: + // must always be invalid by default, add() replaces this with the actual value. + // subsequent add() calls use this index as a token to quickly check whether the + // current origins.back() can be reused or not. + mutable uint32_t idx = std::numeric_limits::max(); + + // Used for searching in PosTable::[]. + explicit Origin(uint32_t idx) + : idx(idx) + , origin{std::monostate()} + { + } + + public: + const Pos::Origin origin; + + Origin(Pos::Origin origin) + : origin(origin) + { + } + }; + + struct Offset + { + uint32_t line, column; + }; + +private: + std::vector origins; + ChunkedVector offsets; + +public: + PosTable() + : offsets(1024) + { + origins.reserve(1024); + } + + PosIdx add(const Origin & origin, uint32_t line, uint32_t column) + { + const auto idx = offsets.add({line, column}).second; + if (origins.empty() || origins.back().idx != origin.idx) { + origin.idx = idx; + origins.push_back(origin); + } + return PosIdx(idx + 1); + } + + Pos operator[](PosIdx p) const + { + if (p.id == 0 || p.id > offsets.size()) + return {}; + const auto idx = p.id - 1; + /* we want the last key <= idx, so we'll take prev(first key > idx). + this is guaranteed to never rewind origin.begin because the first + key is always 0. */ + const auto pastOrigin = std::upper_bound( + origins.begin(), origins.end(), Origin(idx), [](const auto & a, const auto & b) { return a.idx < b.idx; }); + const auto origin = *std::prev(pastOrigin); + const auto offset = offsets[idx]; + return {offset.line, offset.column, origin.origin}; + } +}; + +} From c6a89c1a1659b31694c0fbcd21d78a6dd521c732 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Mon, 22 Jan 2024 17:08:29 -0800 Subject: [PATCH 2/6] libexpr: Support structured error classes While preparing PRs like #9753, I've had to change error messages in dozens of code paths. It would be nice if instead of EvalError("expected 'boolean' but found '%1%'", showType(v)) we could write TypeError(v, "boolean") or similar. Then, changing the error message could be a mechanical refactor with the compiler pointing out places the constructor needs to be changed, rather than the error-prone process of grepping through the codebase. Structured errors would also help prevent the "same" error from having multiple slightly different messages, and could be a first step towards error codes / an error index. This PR reworks the exception infrastructure in `libexpr` to support exception types with different constructor signatures than `BaseError`. Actually refactoring the exceptions to use structured data will come in a future PR (this one is big enough already, as it has to touch every exception in `libexpr`). The core design is in `eval-error.hh`. Generally, errors like this: state.error("'%s' is not a string", getAttrPathStr()) .debugThrow() are transformed like this: state.error("'%s' is not a string", getAttrPathStr()) .debugThrow() The type annotation has moved from `ErrorBuilder::debugThrow` to `EvalState::error`. --- src/libcmd/repl.cc | 2 - src/libexpr/attr-path.cc | 8 +- src/libexpr/eval-cache.cc | 30 +-- src/libexpr/eval-error.cc | 113 ++++++++ src/libexpr/eval-error.hh | 118 +++++++++ src/libexpr/eval-inline.hh | 19 +- src/libexpr/eval.cc | 217 +++++++--------- src/libexpr/eval.hh | 91 +------ src/libexpr/flake/flake.cc | 16 +- src/libexpr/get-drvs.cc | 5 +- src/libexpr/json-to-value.cc | 4 +- src/libexpr/json-to-value.hh | 7 +- src/libexpr/lexer.l | 12 +- src/libexpr/nixexpr.cc | 8 +- src/libexpr/nixexpr.hh | 17 +- src/libexpr/parser-state.hh | 8 +- src/libexpr/parser.y | 8 +- src/libexpr/primops.cc | 244 ++++++++---------- src/libexpr/primops/context.cc | 50 ++-- src/libexpr/primops/fetchClosure.cc | 22 +- src/libexpr/primops/fetchMercurial.cc | 10 +- src/libexpr/primops/fetchTree.cc | 68 ++--- src/libexpr/primops/fromTOML.cc | 5 +- src/libexpr/value-to-json.cc | 18 +- src/libexpr/value.hh | 2 +- src/libmain/shared.cc | 2 +- src/libstore/build/entry-points.cc | 4 +- src/libstore/daemon.cc | 2 +- src/libutil/error.cc | 6 +- src/libutil/error.hh | 27 +- src/libutil/logging.cc | 2 +- src/nix-store/nix-store.cc | 4 +- src/nix/eval.cc | 2 +- src/nix/flake.cc | 6 +- tests/functional/fetchGit.sh | 4 +- .../lang/eval-fail-attr-name-type.err.exp | 5 + .../eval-fail-fromTOML-timestamps.err.exp | 2 +- .../functional/lang/eval-fail-toJSON.err.exp | 5 + .../eval-fail-using-set-as-attr-name.err.exp | 5 + tests/unit/libexpr/error_traces.cc | 20 +- 40 files changed, 653 insertions(+), 545 deletions(-) create mode 100644 src/libexpr/eval-error.cc create mode 100644 src/libexpr/eval-error.hh diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index d7d8f9819..714d3adb5 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -422,8 +422,6 @@ StringSet NixRepl::completePrefix(const std::string & prefix) // Quietly ignore parse errors. } catch (EvalError & e) { // Quietly ignore evaluation errors. - } catch (UndefinedVarError & e) { - // Quietly ignore undefined variable errors. } catch (BadURL & e) { // Quietly ignore BadURL flake-related errors. } diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index 7481a2232..d6befd362 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -65,10 +65,10 @@ std::pair findAlongAttrPath(EvalState & state, const std::strin if (!attrIndex) { if (v->type() != nAttrs) - throw TypeError( + state.error( "the expression selected by the selection path '%1%' should be a set but is %2%", attrPath, - showType(*v)); + showType(*v)).debugThrow(); if (attr.empty()) throw Error("empty attribute name in selection path '%1%'", attrPath); @@ -88,10 +88,10 @@ std::pair findAlongAttrPath(EvalState & state, const std::strin else { if (!v->isList()) - throw TypeError( + state.error( "the expression selected by the selection path '%1%' should be a list but is %2%", attrPath, - showType(*v)); + showType(*v)).debugThrow(); if (*attrIndex >= v->listSize()) throw AttrPathNotFound("list index %1% in selection path '%2%' is out of range", *attrIndex, attrPath); diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 5808d58b6..2fc69e796 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -491,7 +491,7 @@ std::shared_ptr AttrCursor::maybeGetAttr(Symbol name, bool forceErro if (forceErrors) debug("reevaluating failed cached attribute '%s'", getAttrPathStr(name)); else - throw CachedEvalError("cached failure of attribute '%s'", getAttrPathStr(name)); + throw CachedEvalError(root->state, "cached failure of attribute '%s'", getAttrPathStr(name)); } else return std::make_shared(root, std::make_pair(shared_from_this(), name), nullptr, std::move(attr)); @@ -500,7 +500,7 @@ std::shared_ptr AttrCursor::maybeGetAttr(Symbol name, bool forceErro // evaluate to see whether 'name' exists } else return nullptr; - //throw TypeError("'%s' is not an attribute set", getAttrPathStr()); + //error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); } } @@ -508,7 +508,7 @@ std::shared_ptr AttrCursor::maybeGetAttr(Symbol name, bool forceErro if (v.type() != nAttrs) return nullptr; - //throw TypeError("'%s' is not an attribute set", getAttrPathStr()); + //error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); auto attr = v.attrs->get(name); @@ -574,14 +574,14 @@ std::string AttrCursor::getString() debug("using cached string attribute '%s'", getAttrPathStr()); return s->first; } else - root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); } } auto & v = forceValue(); if (v.type() != nString && v.type() != nPath) - root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); return v.type() == nString ? v.c_str() : v.path().to_string(); } @@ -616,7 +616,7 @@ string_t AttrCursor::getStringWithContext() return *s; } } else - root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string", getAttrPathStr()).debugThrow(); } } @@ -630,7 +630,7 @@ string_t AttrCursor::getStringWithContext() else if (v.type() == nPath) return {v.path().to_string(), {}}; else - root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a string but %s", getAttrPathStr()).debugThrow(); } bool AttrCursor::getBool() @@ -643,14 +643,14 @@ bool AttrCursor::getBool() debug("using cached Boolean attribute '%s'", getAttrPathStr()); return *b; } else - root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); } } auto & v = forceValue(); if (v.type() != nBool) - root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not a Boolean", getAttrPathStr()).debugThrow(); return v.boolean; } @@ -665,14 +665,14 @@ NixInt AttrCursor::getInt() debug("using cached integer attribute '%s'", getAttrPathStr()); return i->x; } else - throw TypeError("'%s' is not an integer", getAttrPathStr()); + root->state.error("'%s' is not an integer", getAttrPathStr()).debugThrow(); } } auto & v = forceValue(); if (v.type() != nInt) - throw TypeError("'%s' is not an integer", getAttrPathStr()); + root->state.error("'%s' is not an integer", getAttrPathStr()).debugThrow(); return v.integer; } @@ -687,7 +687,7 @@ std::vector AttrCursor::getListOfStrings() debug("using cached list of strings attribute '%s'", getAttrPathStr()); return *l; } else - throw TypeError("'%s' is not a list of strings", getAttrPathStr()); + root->state.error("'%s' is not a list of strings", getAttrPathStr()).debugThrow(); } } @@ -697,7 +697,7 @@ std::vector AttrCursor::getListOfStrings() root->state.forceValue(v, noPos); if (v.type() != nList) - throw TypeError("'%s' is not a list", getAttrPathStr()); + root->state.error("'%s' is not a list", getAttrPathStr()).debugThrow(); std::vector res; @@ -720,14 +720,14 @@ std::vector AttrCursor::getAttrs() debug("using cached attrset attribute '%s'", getAttrPathStr()); return *attrs; } else - root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); } } auto & v = forceValue(); if (v.type() != nAttrs) - root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); + root->state.error("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); std::vector attrs; for (auto & attr : *getValue().attrs) diff --git a/src/libexpr/eval-error.cc b/src/libexpr/eval-error.cc new file mode 100644 index 000000000..b9411cbf4 --- /dev/null +++ b/src/libexpr/eval-error.cc @@ -0,0 +1,113 @@ +#include "eval-error.hh" +#include "eval.hh" +#include "value.hh" + +namespace nix { + +template +EvalErrorBuilder & EvalErrorBuilder::withExitStatus(unsigned int exitStatus) +{ + error.withExitStatus(exitStatus); + return *this; +} + +template +EvalErrorBuilder & EvalErrorBuilder::atPos(PosIdx pos) +{ + error.err.pos = error.state.positions[pos]; + return *this; +} + +template +EvalErrorBuilder & EvalErrorBuilder::atPos(Value & value, PosIdx fallback) +{ + return atPos(value.determinePos(fallback)); +} + +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 = hintformat(std::string(text)), .frame = true}); + return *this; +} + +template +EvalErrorBuilder & EvalErrorBuilder::withSuggestions(Suggestions & s) +{ + error.err.suggestions = s; + return *this; +} + +template +EvalErrorBuilder & EvalErrorBuilder::withFrame(const Env & env, const Expr & expr) +{ + // NOTE: This is abusing side-effects. + // TODO: check compatibility with nested debugger calls. + // TODO: What side-effects?? + error.state.debugTraces.push_front(DebugTrace{ + .pos = error.state.positions[expr.getPos()], + .expr = expr, + .env = env, + .hint = hintformat("Fake frame for debugging purposes"), + .isError = true}); + return *this; +} + +template +EvalErrorBuilder & EvalErrorBuilder::addTrace(PosIdx pos, hintformat hint, bool frame) +{ + error.addTrace(error.state.positions[pos], hint, frame); + return *this; +} + +template +template +EvalErrorBuilder & +EvalErrorBuilder::addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs) +{ + + addTrace(error.state.positions[pos], hintfmt(std::string(formatString), formatArgs...)); + return *this; +} + +template +void EvalErrorBuilder::debugThrow() +{ + if (error.state.debugRepl && !error.state.debugTraces.empty()) { + const DebugTrace & last = error.state.debugTraces.front(); + const Env * env = &last.env; + const Expr * expr = &last.expr; + error.state.runDebugRepl(&error, *env, *expr); + } + + // `EvalState` is the only class that can construct an `EvalErrorBuilder`, + // and it does so in dynamic storage. This is the final method called on + // any such instancve and must delete itself before throwing the underlying + // error. + auto error = std::move(this->error); + delete this; + + throw error; +} + +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; +template class EvalErrorBuilder; + +} diff --git a/src/libexpr/eval-error.hh b/src/libexpr/eval-error.hh new file mode 100644 index 000000000..ee69dce64 --- /dev/null +++ b/src/libexpr/eval-error.hh @@ -0,0 +1,118 @@ +#pragma once + +#include + +#include "error.hh" +#include "pos-idx.hh" + +namespace nix { + +struct Env; +struct Expr; +struct Value; + +class EvalState; +template +class EvalErrorBuilder; + +class EvalError : public Error +{ + template + friend class EvalErrorBuilder; +public: + EvalState & state; + + EvalError(EvalState & state, ErrorInfo && errorInfo) + : Error(errorInfo) + , state(state) + { + } + + template + explicit EvalError(EvalState & state, const std::string & formatString, const Args &... formatArgs) + : Error(formatString, formatArgs...) + , state(state) + { + } +}; + +MakeError(ParseError, Error); +MakeError(AssertionError, EvalError); +MakeError(ThrownError, AssertionError); +MakeError(Abort, EvalError); +MakeError(TypeError, EvalError); +MakeError(UndefinedVarError, EvalError); +MakeError(MissingArgumentError, EvalError); +MakeError(CachedEvalError, EvalError); +MakeError(InfiniteRecursionError, EvalError); + +struct InvalidPathError : public EvalError +{ +public: + Path path; + InvalidPathError(EvalState & state, const Path & path) + : EvalError(state, "path '%s' is not valid", path) + { + } +}; + +template +class EvalErrorBuilder final +{ + friend class EvalState; + + template + explicit EvalErrorBuilder(EvalState & state, const Args &... args) + : error(T(state, args...)) + { + } + +public: + T error; + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & withExitStatus(unsigned int exitStatus); + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & atPos(PosIdx pos); + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & atPos(Value & value, PosIdx fallback = noPos); + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & withTrace(PosIdx pos, const std::string_view text); + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & withFrameTrace(PosIdx pos, const std::string_view text); + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & withSuggestions(Suggestions & s); + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & withFrame(const Env & e, const Expr & ex); + + [[nodiscard, gnu::noinline]] EvalErrorBuilder & addTrace(PosIdx pos, hintformat hint, bool frame = false); + + template + [[nodiscard, gnu::noinline]] EvalErrorBuilder & + addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs); + + [[gnu::noinline, gnu::noreturn]] void debugThrow(); +}; + +/** + * The size needed to allocate any `EvalErrorBuilder`. + * + * The list of classes here needs to be kept in sync with the list of `template + * class` declarations in `eval-error.cc`. + * + * This is used by `EvalState` to preallocate a buffer of sufficient size for + * any `EvalErrorBuilder` to avoid allocating while evaluating Nix code. + */ +constexpr size_t EVAL_ERROR_BUILDER_SIZE = std::max({ + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), + sizeof(EvalErrorBuilder), +}); + +} diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index 42cb68bbe..03320c7c9 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -3,6 +3,7 @@ #include "print.hh" #include "eval.hh" +#include "eval-error.hh" namespace nix { @@ -115,10 +116,11 @@ inline void EvalState::forceAttrs(Value & v, Callable getPos, std::string_view e PosIdx pos = getPos(); forceValue(v, pos); if (v.type() != nAttrs) { - error("expected a set but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .withTrace(pos, errorCtx).debugThrow(); + error( + "expected a set but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).withTrace(pos, errorCtx).debugThrow(); } } @@ -128,10 +130,11 @@ inline void EvalState::forceList(Value & v, const PosIdx pos, std::string_view e { forceValue(v, pos); if (!v.isList()) { - error("expected a list but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .withTrace(pos, errorCtx).debugThrow(); + error( + "expected a list but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).withTrace(pos, errorCtx).debugThrow(); } } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 91fd3ddf8..ded4415cc 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -339,46 +339,6 @@ void initGC() gcInitialised = true; } - -ErrorBuilder & ErrorBuilder::atPos(PosIdx pos) -{ - info.errPos = state.positions[pos]; - return *this; -} - -ErrorBuilder & ErrorBuilder::withTrace(PosIdx pos, const std::string_view text) -{ - info.traces.push_front(Trace{ .pos = state.positions[pos], .hint = hintformat(std::string(text)), .frame = false }); - return *this; -} - -ErrorBuilder & ErrorBuilder::withFrameTrace(PosIdx pos, const std::string_view text) -{ - info.traces.push_front(Trace{ .pos = state.positions[pos], .hint = hintformat(std::string(text)), .frame = true }); - return *this; -} - -ErrorBuilder & ErrorBuilder::withSuggestions(Suggestions & s) -{ - info.suggestions = s; - return *this; -} - -ErrorBuilder & ErrorBuilder::withFrame(const Env & env, const Expr & expr) -{ - // NOTE: This is abusing side-effects. - // TODO: check compatibility with nested debugger calls. - state.debugTraces.push_front(DebugTrace { - .pos = nullptr, - .expr = expr, - .env = env, - .hint = hintformat("Fake frame for debugging purposes"), - .isError = true - }); - return *this; -} - - EvalState::EvalState( const SearchPath & _searchPath, ref store, @@ -811,7 +771,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & ? std::make_unique( *this, DebugTrace { - .pos = error->info().errPos ? error->info().errPos : positions[expr.getPos()], + .pos = error->info().pos ? error->info().pos : positions[expr.getPos()], .expr = expr, .env = env, .hint = error->info().msg, @@ -930,7 +890,7 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) return j->value; } if (!fromWith->parentWith) - error("undefined variable '%1%'", symbols[var.name]).atPos(var.pos).withFrame(*env, var).debugThrow(); + error("undefined variable '%1%'", symbols[var.name]).atPos(var.pos).withFrame(*env, var).debugThrow(); for (size_t l = fromWith->prevWith; l; --l, env = env->up) ; fromWith = fromWith->parentWith; } @@ -1136,7 +1096,7 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial) // computation. if (mustBeTrivial && !(dynamic_cast(e))) - error("file '%s' must be an attribute set", path).debugThrow(); + error("file '%s' must be an attribute set", path).debugThrow(); eval(e, v); } catch (Error & e) { addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath.to_string()); @@ -1167,10 +1127,11 @@ inline bool EvalState::evalBool(Env & env, Expr * e, const PosIdx pos, std::stri Value v; e->eval(*this, env, v); if (v.type() != nBool) - error("expected a Boolean but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .withFrame(env, *e).debugThrow(); + error( + "expected a Boolean but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).atPos(pos).withFrame(env, *e).debugThrow(); return v.boolean; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -1184,10 +1145,11 @@ inline void EvalState::evalAttrs(Env & env, Expr * e, Value & v, const PosIdx po try { e->eval(*this, env, v); if (v.type() != nAttrs) - error("expected a set but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .withFrame(env, *e).debugThrow(); + error( + "expected a set but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).withFrame(env, *e).debugThrow(); } catch (Error & e) { e.addTrace(positions[pos], errorCtx); throw; @@ -1296,7 +1258,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) auto nameSym = state.symbols.create(nameVal.string_view()); Bindings::iterator j = v.attrs->find(nameSym); if (j != v.attrs->end()) - state.error("dynamic attribute '%1%' already defined at %2%", state.symbols[nameSym], state.positions[j->pos]).atPos(i.pos).withFrame(env, *this).debugThrow(); + state.error("dynamic attribute '%1%' already defined at %2%", state.symbols[nameSym], state.positions[j->pos]).atPos(i.pos).withFrame(env, *this).debugThrow(); i.valueExpr->setName(nameSym); /* Keep sorted order so find can catch duplicates */ @@ -1408,8 +1370,8 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) for (auto & attr : *vAttrs->attrs) allAttrNames.insert(state.symbols[attr.name]); auto suggestions = Suggestions::bestMatches(allAttrNames, state.symbols[name]); - state.error("attribute '%1%' missing", state.symbols[name]) - .atPos(pos).withSuggestions(suggestions).withFrame(env, *this).debugThrow(); + state.error("attribute '%1%' missing", state.symbols[name]) + .atPos(pos).withSuggestions(suggestions).withFrame(env, *this).debugThrow(); } } vAttrs = j->value; @@ -1482,7 +1444,7 @@ public: void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & vRes, const PosIdx pos) { if (callDepth > evalSettings.maxCallDepth) - error("stack overflow; max-call-depth exceeded").atPos(pos).template debugThrow(); + error("stack overflow; max-call-depth exceeded").atPos(pos).debugThrow(); CallDepth _level(callDepth); auto trace = evalSettings.traceFunctionCalls @@ -1540,13 +1502,13 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & auto j = args[0]->attrs->get(i.name); if (!j) { if (!i.def) { - error("function '%1%' called without required argument '%2%'", + error("function '%1%' called without required argument '%2%'", (lambda.name ? std::string(symbols[lambda.name]) : "anonymous lambda"), symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") .withFrame(*fun.lambda.env, lambda) - .debugThrow(); + .debugThrow(); } env2.values[displ++] = i.def->maybeThunk(*this, env2); } else { @@ -1566,14 +1528,14 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & for (auto & formal : lambda.formals->formals) formalNames.insert(symbols[formal.name]); auto suggestions = Suggestions::bestMatches(formalNames, symbols[i.name]); - error("function '%1%' called with unexpected argument '%2%'", + error("function '%1%' called with unexpected argument '%2%'", (lambda.name ? std::string(symbols[lambda.name]) : "anonymous lambda"), symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") .withSuggestions(suggestions) .withFrame(*fun.lambda.env, lambda) - .debugThrow(); + .debugThrow(); } abort(); // can't happen } @@ -1705,11 +1667,12 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & } else - error("attempt to call something which is not a function but %1%: %2%", + error( + "attempt to call something which is not a function but %1%: %2%", showType(vCur), ValuePrinter(*this, vCur, errorPrintOptions)) .atPos(pos) - .debugThrow(); + .debugThrow(); } vRes = vCur; @@ -1779,12 +1742,12 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) if (j != args.end()) { attrs.insert(*j); } else if (!i.def) { - error(R"(cannot evaluate a function that has an argument without a value ('%1%') + error(R"(cannot evaluate a function that has an argument without a value ('%1%') Nix attempted to evaluate a function as a top level expression; in this case it must have its arguments supplied either by default values, or passed explicitly with '--arg' or '--argstr'. See https://nixos.org/manual/nix/stable/language/constructs.html#functions.)", symbols[i.name]) - .atPos(i.pos).withFrame(*fun.lambda.env, *fun.lambda.fun).debugThrow(); + .atPos(i.pos).withFrame(*fun.lambda.env, *fun.lambda.fun).debugThrow(); } } } @@ -1815,7 +1778,7 @@ void ExprAssert::eval(EvalState & state, Env & env, Value & v) if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) { std::ostringstream out; cond->show(state.symbols, out); - state.error("assertion '%1%' failed", out.str()).atPos(pos).withFrame(env, *this).debugThrow(); + state.error("assertion '%1%' failed", out.str()).atPos(pos).withFrame(env, *this).debugThrow(); } body->eval(state, env, v); } @@ -1993,14 +1956,14 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) nf = n; nf += vTmp.fpoint; } else - state.error("cannot add %1% to an integer", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); + state.error("cannot add %1% to an integer", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); } else if (firstType == nFloat) { if (vTmp.type() == nInt) { nf += vTmp.integer; } else if (vTmp.type() == nFloat) { nf += vTmp.fpoint; } else - state.error("cannot add %1% to a float", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); + state.error("cannot add %1% to a float", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow(); } else { if (s.empty()) s.reserve(es->size()); /* skip canonization of first path, which would only be not @@ -2022,7 +1985,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) v.mkFloat(nf); else if (firstType == nPath) { if (!context.empty()) - state.error("a string that refers to a store path cannot be appended to a path").atPos(pos).withFrame(env, *this).debugThrow(); + state.error("a string that refers to a store path cannot be appended to a path").atPos(pos).withFrame(env, *this).debugThrow(); v.mkPath(state.rootPath(CanonPath(canonPath(str())))); } else v.mkStringMove(c_str(), context); @@ -2037,8 +2000,9 @@ void ExprPos::eval(EvalState & state, Env & env, Value & v) void ExprBlackHole::eval(EvalState & state, Env & env, Value & v) { - state.error("infinite recursion encountered") - .debugThrow(); + state.error("infinite recursion encountered") + .atPos(v.determinePos(noPos)) + .debugThrow(); } // always force this to be separate, otherwise forceValue may inline it and take @@ -2052,7 +2016,7 @@ void EvalState::tryFixupBlackHolePos(Value & v, PosIdx pos) try { std::rethrow_exception(e); } catch (InfiniteRecursionError & e) { - e.err.errPos = positions[pos]; + e.atPos(positions[pos]); } catch (...) { } } @@ -2100,15 +2064,18 @@ NixInt EvalState::forceInt(Value & v, const PosIdx pos, std::string_view errorCt try { forceValue(v, pos); if (v.type() != nInt) - error("expected an integer but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .debugThrow(); + error( + "expected an integer but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).atPos(pos).debugThrow(); return v.integer; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); throw; } + + return v.integer; } @@ -2119,10 +2086,11 @@ NixFloat EvalState::forceFloat(Value & v, const PosIdx pos, std::string_view err if (v.type() == nInt) return v.integer; else if (v.type() != nFloat) - error("expected a float but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .debugThrow(); + error( + "expected a float but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).atPos(pos).debugThrow(); return v.fpoint; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -2136,15 +2104,18 @@ bool EvalState::forceBool(Value & v, const PosIdx pos, std::string_view errorCtx try { forceValue(v, pos); if (v.type() != nBool) - error("expected a Boolean but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .debugThrow(); + error( + "expected a Boolean but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).atPos(pos).debugThrow(); return v.boolean; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); throw; } + + return v.boolean; } @@ -2159,10 +2130,11 @@ void EvalState::forceFunction(Value & v, const PosIdx pos, std::string_view erro try { forceValue(v, pos); if (v.type() != nFunction && !isFunctor(v)) - error("expected a function but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .debugThrow(); + error( + "expected a function but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).atPos(pos).debugThrow(); } catch (Error & e) { e.addTrace(positions[pos], errorCtx); throw; @@ -2175,10 +2147,11 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string try { forceValue(v, pos); if (v.type() != nString) - error("expected a string but found %1%: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) - .debugThrow(); + error( + "expected a string but found %1%: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ).atPos(pos).debugThrow(); return v.string_view(); } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -2207,7 +2180,7 @@ std::string_view EvalState::forceStringNoCtx(Value & v, const PosIdx pos, std::s { auto s = forceString(v, pos, errorCtx); if (v.context()) { - error("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string_view(), v.context()[0]).withTrace(pos, errorCtx).debugThrow(); + error("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string_view(), v.context()[0]).withTrace(pos, errorCtx).debugThrow(); } return s; } @@ -2272,11 +2245,13 @@ BackedStringView EvalState::coerceToString( return std::move(*maybeString); auto i = v.attrs->find(sOutPath); if (i == v.attrs->end()) { - error("cannot coerce %1% to a string: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) + error( + "cannot coerce %1% to a string: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ) .withTrace(pos, errorCtx) - .debugThrow(); + .debugThrow(); } return coerceToString(pos, *i->value, context, errorCtx, coerceMore, copyToStore, canonicalizePath); @@ -2284,7 +2259,7 @@ BackedStringView EvalState::coerceToString( if (v.type() == nExternal) { try { - return v.external->coerceToString(positions[pos], context, coerceMore, copyToStore); + return v.external->coerceToString(*this, pos, context, coerceMore, copyToStore); } catch (Error & e) { e.addTrace(nullptr, errorCtx); throw; @@ -2320,18 +2295,19 @@ BackedStringView EvalState::coerceToString( } } - error("cannot coerce %1% to a string: %2%", - showType(v), - ValuePrinter(*this, v, errorPrintOptions)) + error("cannot coerce %1% to a string: %2%", + showType(v), + ValuePrinter(*this, v, errorPrintOptions) + ) .withTrace(pos, errorCtx) - .debugThrow(); + .debugThrow(); } StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePath & path) { if (nix::isDerivation(path.path.abs())) - error("file names are not allowed to end in '%1%'", drvExtension).debugThrow(); + error("file names are not allowed to end in '%1%'", drvExtension).debugThrow(); auto i = srcToStore.find(path); @@ -2380,7 +2356,7 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext relative to the root filesystem. */ auto path = coerceToString(pos, v, context, errorCtx, false, false, true).toOwned(); if (path == "" || path[0] != '/') - error("string '%1%' doesn't represent an absolute path", path).withTrace(pos, errorCtx).debugThrow(); + error("string '%1%' doesn't represent an absolute path", path).withTrace(pos, errorCtx).debugThrow(); return rootPath(CanonPath(path)); } @@ -2390,7 +2366,7 @@ StorePath EvalState::coerceToStorePath(const PosIdx pos, Value & v, NixStringCon auto path = coerceToString(pos, v, context, errorCtx, false, false, true).toOwned(); if (auto storePath = store->maybeParseStorePath(path)) return *storePath; - error("path '%1%' is not in the Nix store", path).withTrace(pos, errorCtx).debugThrow(); + error("path '%1%' is not in the Nix store", path).withTrace(pos, errorCtx).debugThrow(); } @@ -2400,18 +2376,18 @@ std::pair EvalState::coerceToSingleDerivedP auto s = forceString(v, context, pos, errorCtx); auto csize = context.size(); if (csize != 1) - error( + error( "string '%s' has %d entries in its context. It should only have exactly one entry", s, csize) - .withTrace(pos, errorCtx).debugThrow(); + .withTrace(pos, errorCtx).debugThrow(); auto derivedPath = std::visit(overloaded { [&](NixStringContextElem::Opaque && o) -> SingleDerivedPath { return std::move(o); }, [&](NixStringContextElem::DrvDeep &&) -> SingleDerivedPath { - error( + error( "string '%s' has a context which refers to a complete source and binary closure. This is not supported at this time", - s).withTrace(pos, errorCtx).debugThrow(); + s).withTrace(pos, errorCtx).debugThrow(); }, [&](NixStringContextElem::Built && b) -> SingleDerivedPath { return std::move(b); @@ -2434,16 +2410,16 @@ SingleDerivedPath EvalState::coerceToSingleDerivedPath(const PosIdx pos, Value & error message. */ std::visit(overloaded { [&](const SingleDerivedPath::Opaque & o) { - error( + error( "path string '%s' has context with the different path '%s'", s, sExpected) - .withTrace(pos, errorCtx).debugThrow(); + .withTrace(pos, errorCtx).debugThrow(); }, [&](const SingleDerivedPath::Built & b) { - error( + error( "string '%s' has context with the output '%s' from derivation '%s', but the string is not the right placeholder for this derivation output. It should be '%s'", s, b.output, b.drvPath->to_string(*store), sExpected) - .withTrace(pos, errorCtx).debugThrow(); + .withTrace(pos, errorCtx).debugThrow(); } }, derivedPath.raw()); } @@ -2528,7 +2504,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v case nThunk: // Must not be left by forceValue default: - error("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow(); + error("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow(); } } @@ -2767,13 +2743,12 @@ SourcePath EvalState::findFile(const SearchPath & searchPath, const std::string_ if (hasPrefix(path, "nix/")) return {corepkgsFS, CanonPath(path.substr(3))}; - debugThrow(ThrownError({ - .msg = hintfmt(evalSettings.pureEval + error( + evalSettings.pureEval ? "cannot look up '<%s>' in pure evaluation mode (use '--impure' to override)" : "file '%s' was not found in the Nix search path (add it using $NIX_PATH or -I)", - path), - .errPos = positions[pos] - }), 0, 0); + path + ).atPos(pos).debugThrow(); } @@ -2856,11 +2831,11 @@ Expr * EvalState::parse( } -std::string ExternalValueBase::coerceToString(const Pos & pos, NixStringContext & context, bool copyMore, bool copyToStore) const +std::string ExternalValueBase::coerceToString(EvalState & state, const PosIdx & pos, NixStringContext & context, bool copyMore, bool copyToStore) const { - throw TypeError({ - .msg = hintfmt("cannot coerce %1% to a string: %2%", showType(), *this) - }); + state.error( + "cannot coerce %1% to a string: %2%", showType(), *this + ).atPos(pos).debugThrow(); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 2368187b1..afe89cd30 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -2,6 +2,7 @@ ///@file #include "attr-set.hh" +#include "eval-error.hh" #include "types.hh" #include "value.hh" #include "nixexpr.hh" @@ -151,45 +152,6 @@ struct DebugTrace { bool isError; }; -void debugError(Error * e, Env & env, Expr & expr); - -class ErrorBuilder -{ - private: - EvalState & state; - ErrorInfo info; - - ErrorBuilder(EvalState & s, ErrorInfo && i): state(s), info(i) { } - - public: - template - [[nodiscard, gnu::noinline]] - static ErrorBuilder * create(EvalState & s, const Args & ... args) - { - return new ErrorBuilder(s, ErrorInfo { .msg = hintfmt(args...) }); - } - - [[nodiscard, gnu::noinline]] - ErrorBuilder & atPos(PosIdx pos); - - [[nodiscard, gnu::noinline]] - ErrorBuilder & withTrace(PosIdx pos, const std::string_view text); - - [[nodiscard, gnu::noinline]] - ErrorBuilder & withFrameTrace(PosIdx pos, const std::string_view text); - - [[nodiscard, gnu::noinline]] - ErrorBuilder & withSuggestions(Suggestions & s); - - [[nodiscard, gnu::noinline]] - ErrorBuilder & withFrame(const Env & e, const Expr & ex); - - template - [[gnu::noinline, gnu::noreturn]] - void debugThrow(); -}; - - class EvalState : public std::enable_shared_from_this { public: @@ -274,39 +236,10 @@ public: void runDebugRepl(const Error * error, const Env & env, const Expr & expr); - template - [[gnu::noinline, gnu::noreturn]] - void debugThrowLastTrace(E && error) - { - debugThrow(error, nullptr, nullptr); - } - - template - [[gnu::noinline, gnu::noreturn]] - void debugThrow(E && error, const Env * env, const Expr * expr) - { - if (debugRepl && ((env && expr) || !debugTraces.empty())) { - if (!env || !expr) { - const DebugTrace & last = debugTraces.front(); - env = &last.env; - expr = &last.expr; - } - runDebugRepl(&error, *env, *expr); - } - - throw std::move(error); - } - - // This is dangerous, but gets in line with the idea that error creation and - // throwing should not allocate on the stack of hot functions. - // as long as errors are immediately thrown, it works. - ErrorBuilder * errorBuilder; - - template + template [[nodiscard, gnu::noinline]] - ErrorBuilder & error(const Args & ... args) { - errorBuilder = ErrorBuilder::create(*this, args...); - return *errorBuilder; + EvalErrorBuilder & error(const Args & ... args) { + return *new EvalErrorBuilder(*this, args...); } private: @@ -845,22 +778,6 @@ SourcePath resolveExprPath(SourcePath path); */ bool isAllowedURI(std::string_view uri, const Strings & allowedPaths); -struct InvalidPathError : EvalError -{ - Path path; - InvalidPathError(const Path & path); -#ifdef EXCEPTION_NEEDS_THROW_SPEC - ~InvalidPathError() throw () { }; -#endif -}; - -template -void ErrorBuilder::debugThrow() -{ - // NOTE: We always use the -LastTrace version as we push the new trace in withFrame() - state.debugThrowLastTrace(ErrorType(info)); -} - } #include "eval-inline.hh" diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index fee58792b..3396b0219 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -147,8 +147,8 @@ static FlakeInput parseFlakeInput(EvalState & state, NixStringContext emptyContext = {}; attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, *attr.value, pos, emptyContext).dump()); } else - throw TypeError("flake input attribute '%s' is %s while a string, Boolean, or integer is expected", - state.symbols[attr.name], showType(*attr.value)); + state.error("flake input attribute '%s' is %s while a string, Boolean, or integer is expected", + state.symbols[attr.name], showType(*attr.value)).debugThrow(); } #pragma GCC diagnostic pop } @@ -295,15 +295,15 @@ static Flake getFlake( std::vector ss; for (auto elem : setting.value->listItems()) { if (elem->type() != nString) - throw TypeError("list element in flake configuration setting '%s' is %s while a string is expected", - state.symbols[setting.name], showType(*setting.value)); + state.error("list element in flake configuration setting '%s' is %s while a string is expected", + state.symbols[setting.name], showType(*setting.value)).debugThrow(); ss.emplace_back(state.forceStringNoCtx(*elem, setting.pos, "")); } flake.config.settings.emplace(state.symbols[setting.name], ss); } else - throw TypeError("flake configuration setting '%s' is %s", - state.symbols[setting.name], showType(*setting.value)); + state.error("flake configuration setting '%s' is %s", + state.symbols[setting.name], showType(*setting.value)).debugThrow(); } } @@ -865,11 +865,11 @@ static void prim_flakeRefToString( attrs.emplace(state.symbols[attr.name], std::string(attr.value->string_view())); } else { - state.error( + state.error( "flake reference attribute sets may only contain integers, Booleans, " "and strings, but attribute '%s' is %s", state.symbols[attr.name], - showType(*attr.value)).debugThrow(); + showType(*attr.value)).debugThrow(); } } auto flakeRef = FlakeRef::fromAttrs(attrs); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 51449ccb3..e9ed1ef08 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -49,7 +49,7 @@ std::string PackageInfo::queryName() const { if (name == "" && attrs) { auto i = attrs->find(state->sName); - if (i == attrs->end()) throw TypeError("derivation name missing"); + if (i == attrs->end()) state->error("derivation name missing").debugThrow(); name = state->forceStringNoCtx(*i->value, noPos, "while evaluating the 'name' attribute of a derivation"); } return name; @@ -396,7 +396,8 @@ static void getDerivations(EvalState & state, Value & vIn, } } - else throw TypeError("expression does not evaluate to a derivation (or a set or list of those)"); + else + state.error("expression does not evaluate to a derivation (or a set or list of those)").debugThrow(); } diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 99a475ff9..2d12c47c5 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -1,4 +1,6 @@ #include "json-to-value.hh" +#include "value.hh" +#include "eval.hh" #include #include @@ -159,7 +161,7 @@ public: } bool parse_error(std::size_t, const std::string&, const nlohmann::detail::exception& ex) { - throw JSONParseError(ex.what()); + throw JSONParseError("%s", ex.what()); } }; diff --git a/src/libexpr/json-to-value.hh b/src/libexpr/json-to-value.hh index 3b8ec000f..3c8fa5cc0 100644 --- a/src/libexpr/json-to-value.hh +++ b/src/libexpr/json-to-value.hh @@ -1,13 +1,16 @@ #pragma once ///@file -#include "eval.hh" +#include "error.hh" #include namespace nix { -MakeError(JSONParseError, EvalError); +class EvalState; +struct Value; + +MakeError(JSONParseError, Error); void parseJSON(EvalState & state, const std::string_view & s, Value & v); diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index d7a0b5048..af67e847d 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -146,9 +146,9 @@ or { return OR_KW; } try { yylval->n = boost::lexical_cast(yytext); } catch (const boost::bad_lexical_cast &) { - throw ParseError({ + throw ParseError(ErrorInfo{ .msg = hintfmt("invalid integer '%1%'", yytext), - .errPos = state->positions[CUR_POS], + .pos = state->positions[CUR_POS], }); } return INT_LIT; @@ -156,9 +156,9 @@ or { return OR_KW; } {FLOAT} { errno = 0; yylval->nf = strtod(yytext, 0); if (errno != 0) - throw ParseError({ + throw ParseError(ErrorInfo{ .msg = hintfmt("invalid float '%1%'", yytext), - .errPos = state->positions[CUR_POS], + .pos = state->positions[CUR_POS], }); return FLOAT_LIT; } @@ -285,9 +285,9 @@ or { return OR_KW; } {ANY} | <> { - throw ParseError({ + throw ParseError(ErrorInfo{ .msg = hintfmt("path has a trailing slash"), - .errPos = state->positions[CUR_POS], + .pos = state->positions[CUR_POS], }); } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 6fe4ba81b..6b8f33c42 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -296,10 +296,10 @@ void ExprVar::bindVars(EvalState & es, const std::shared_ptr & enclosing `with'. If there is no `with', then we can issue an "undefined variable" error now. */ if (withLevel == -1) - throw UndefinedVarError({ - .msg = hintfmt("undefined variable '%1%'", es.symbols[name]), - .errPos = es.positions[pos] - }); + es.error( + "undefined variable '%1%'", + es.symbols[name] + ).atPos(pos).debugThrow(); for (auto * e = env.get(); e && !fromWith; e = e->up) fromWith = e->isWith; this->level = withLevel; diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index da0ec6e9d..1f944f10b 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -9,28 +9,13 @@ #include "error.hh" #include "chunked-vector.hh" #include "position.hh" +#include "eval-error.hh" #include "pos-idx.hh" #include "pos-table.hh" namespace nix { -MakeError(EvalError, Error); -MakeError(ParseError, Error); -MakeError(AssertionError, EvalError); -MakeError(ThrownError, AssertionError); -MakeError(Abort, EvalError); -MakeError(TypeError, EvalError); -MakeError(UndefinedVarError, Error); -MakeError(MissingArgumentError, EvalError); - -class InfiniteRecursionError : public EvalError -{ - friend class EvalState; -public: - using EvalError::EvalError; -}; - struct Env; struct Value; class EvalState; diff --git a/src/libexpr/parser-state.hh b/src/libexpr/parser-state.hh index 0a9f076dc..bdd5bbabe 100644 --- a/src/libexpr/parser-state.hh +++ b/src/libexpr/parser-state.hh @@ -66,7 +66,7 @@ inline void ParserState::dupAttr(const AttrPath & attrPath, const PosIdx pos, co throw ParseError({ .msg = hintfmt("attribute '%1%' already defined at %2%", showAttrPath(symbols, attrPath), positions[prevPos]), - .errPos = positions[pos] + .pos = positions[pos] }); } @@ -74,7 +74,7 @@ inline void ParserState::dupAttr(Symbol attr, const PosIdx pos, const PosIdx pre { throw ParseError({ .msg = hintfmt("attribute '%1%' already defined at %2%", symbols[attr], positions[prevPos]), - .errPos = positions[pos] + .pos = positions[pos] }); } @@ -155,13 +155,13 @@ inline Formals * ParserState::validateFormals(Formals * formals, PosIdx pos, Sym if (duplicate) throw ParseError({ .msg = hintfmt("duplicate formal function argument '%1%'", symbols[duplicate->first]), - .errPos = positions[duplicate->second] + .pos = positions[duplicate->second] }); if (arg && formals->has(arg)) throw ParseError({ .msg = hintfmt("duplicate formal function argument '%1%'", symbols[arg]), - .errPos = positions[pos] + .pos = positions[pos] }); return formals; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index e95da37f7..95f45c80a 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -66,7 +66,7 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParserState * state, const char * { throw ParseError({ .msg = hintfmt(error), - .errPos = state->positions[state->at(*loc)] + .pos = state->positions[state->at(*loc)] }); } @@ -155,7 +155,7 @@ expr_function { if (!$2->dynamicAttrs.empty()) throw ParseError({ .msg = hintfmt("dynamic attributes not allowed in let"), - .errPos = state->positions[CUR_POS] + .pos = state->positions[CUR_POS] }); $$ = new ExprLet($2, $4); } @@ -245,7 +245,7 @@ expr_simple if (noURLLiterals) throw ParseError({ .msg = hintfmt("URL literals are disabled"), - .errPos = state->positions[CUR_POS] + .pos = state->positions[CUR_POS] }); $$ = new ExprString(std::string($1)); } @@ -341,7 +341,7 @@ attrs } else throw ParseError({ .msg = hintfmt("dynamic attributes not allowed in inherit"), - .errPos = state->positions[state->at(@2)] + .pos = state->positions[state->at(@2)] }); } | { $$ = new AttrPath; } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 1197b6e13..1eec6f961 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -39,10 +39,6 @@ namespace nix { * Miscellaneous *************************************************************/ - -InvalidPathError::InvalidPathError(const Path & path) : - EvalError("path '%s' is not valid", path), path(path) {} - StringMap EvalState::realiseContext(const NixStringContext & context) { std::vector drvs; @@ -51,7 +47,7 @@ StringMap EvalState::realiseContext(const NixStringContext & context) for (auto & c : context) { auto ensureValid = [&](const StorePath & p) { if (!store->isValidPath(p)) - debugThrowLastTrace(InvalidPathError(store->printStorePath(p))); + error(store->printStorePath(p)).debugThrow(); }; std::visit(overloaded { [&](const NixStringContextElem::Built & b) { @@ -78,9 +74,10 @@ StringMap EvalState::realiseContext(const NixStringContext & context) if (drvs.empty()) return {}; if (!evalSettings.enableImportFromDerivation) - debugThrowLastTrace(Error( + error( "cannot build '%1%' during evaluation because the option 'allow-import-from-derivation' is disabled", - drvs.begin()->to_string(*store))); + drvs.begin()->to_string(*store) + ).debugThrow(); /* Build/substitute the context. */ std::vector buildReqs; @@ -340,16 +337,16 @@ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Valu void *handle = dlopen(path.path.c_str(), RTLD_LAZY | RTLD_LOCAL); if (!handle) - state.debugThrowLastTrace(EvalError("could not open '%1%': %2%", path, dlerror())); + state.error("could not open '%1%': %2%", path, dlerror()).debugThrow(); dlerror(); ValueInitializer func = (ValueInitializer) dlsym(handle, sym.c_str()); if(!func) { char *message = dlerror(); if (message) - state.debugThrowLastTrace(EvalError("could not load symbol '%1%' from '%2%': %3%", sym, path, message)); + state.error("could not load symbol '%1%' from '%2%': %3%", sym, path, message).debugThrow(); else - state.debugThrowLastTrace(EvalError("symbol '%1%' from '%2%' resolved to NULL when a function pointer was expected", sym, path)); + state.error("symbol '%1%' from '%2%' resolved to NULL when a function pointer was expected", sym, path).debugThrow(); } (func)(state, v); @@ -365,7 +362,7 @@ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v) auto elems = args[0]->listElems(); auto count = args[0]->listSize(); if (count == 0) - state.error("at least one argument to 'exec' required").atPos(pos).debugThrow(); + state.error("at least one argument to 'exec' required").atPos(pos).debugThrow(); NixStringContext context; auto program = state.coerceToString(pos, *elems[0], context, "while evaluating the first element of the argument passed to builtins.exec", @@ -380,7 +377,7 @@ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v) try { auto _ = state.realiseContext(context); // FIXME: Handle CA derivations } catch (InvalidPathError & e) { - state.error("cannot execute '%1%', since path '%2%' is not valid", program, e.path).atPos(pos).debugThrow(); + state.error("cannot execute '%1%', since path '%2%' is not valid", program, e.path).atPos(pos).debugThrow(); } auto output = runProgram(program, true, commandArgs); @@ -582,7 +579,7 @@ struct CompareValues if (v1->type() == nInt && v2->type() == nFloat) return v1->integer < v2->fpoint; if (v1->type() != v2->type()) - state.error("cannot compare %s with %s", showType(*v1), showType(*v2)).debugThrow(); + state.error("cannot compare %s with %s", showType(*v1), showType(*v2)).debugThrow(); // Allow selecting a subset of enum values #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wswitch-enum" @@ -610,7 +607,7 @@ struct CompareValues } } default: - state.error("cannot compare %s with %s; values of that type are incomparable", showType(*v1), showType(*v2)).debugThrow(); + state.error("cannot compare %s with %s; values of that type are incomparable", showType(*v1), showType(*v2)).debugThrow(); #pragma GCC diagnostic pop } } catch (Error & e) { @@ -637,7 +634,7 @@ static Bindings::iterator getAttr( { Bindings::iterator value = attrSet->find(attrSym); if (value == attrSet->end()) { - state.error("attribute '%s' missing", state.symbols[attrSym]).withTrace(noPos, errorCtx).debugThrow(); + state.error("attribute '%s' missing", state.symbols[attrSym]).withTrace(noPos, errorCtx).debugThrow(); } return value; } @@ -758,7 +755,7 @@ static RegisterPrimOp primop_break({ auto error = Error(ErrorInfo { .level = lvlInfo, .msg = hintfmt("breakpoint reached"), - .errPos = state.positions[pos], + .pos = state.positions[pos], }); auto & dt = state.debugTraces.front(); @@ -769,7 +766,7 @@ static RegisterPrimOp primop_break({ throw Error(ErrorInfo{ .level = lvlInfo, .msg = hintfmt("quit the debugger"), - .errPos = nullptr, + .pos = nullptr, }); } } @@ -790,7 +787,7 @@ static RegisterPrimOp primop_abort({ NixStringContext context; auto s = state.coerceToString(pos, *args[0], context, "while evaluating the error message passed to builtins.abort").toOwned(); - state.debugThrowLastTrace(Abort("evaluation aborted with the following error message: '%1%'", s)); + state.error("evaluation aborted with the following error message: '%1%'", s).debugThrow(); } }); @@ -809,7 +806,7 @@ static RegisterPrimOp primop_throw({ NixStringContext context; auto s = state.coerceToString(pos, *args[0], context, "while evaluating the error message passed to builtin.throw").toOwned(); - state.debugThrowLastTrace(ThrownError(s)); + state.error(s).debugThrow(); } }); @@ -1128,37 +1125,33 @@ drvName, Bindings * attrs, Value & v) experimentalFeatureSettings.require(Xp::DynamicDerivations); ingestionMethod = TextIngestionMethod {}; } else - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("invalid value '%s' for 'outputHashMode' attribute", s), - .errPos = state.positions[noPos] - })); + state.error( + "invalid value '%s' for 'outputHashMode' attribute", s + ).atPos(v).debugThrow(); }; auto handleOutputs = [&](const Strings & ss) { outputs.clear(); for (auto & j : ss) { if (outputs.find(j) != outputs.end()) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("duplicate derivation output '%1%'", j), - .errPos = state.positions[noPos] - })); + state.error("duplicate derivation output '%1%'", j) + .atPos(v) + .debugThrow(); /* !!! Check whether j is a valid attribute name. */ /* Derivations cannot be named ‘drv’, because then we'd have an attribute ‘drvPath’ in the resulting set. */ if (j == "drv") - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("invalid derivation output name 'drv'" ), - .errPos = state.positions[noPos] - })); + state.error("invalid derivation output name 'drv'") + .atPos(v) + .debugThrow(); outputs.insert(j); } if (outputs.empty()) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("derivation cannot have an empty set of outputs"), - .errPos = state.positions[noPos] - })); + state.error("derivation cannot have an empty set of outputs") + .atPos(v) + .debugThrow(); }; try { @@ -1281,16 +1274,14 @@ drvName, Bindings * attrs, Value & v) /* Do we have all required attributes? */ if (drv.builder == "") - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("required attribute 'builder' missing"), - .errPos = state.positions[noPos] - })); + state.error("required attribute 'builder' missing") + .atPos(v) + .debugThrow(); if (drv.platform == "") - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("required attribute 'system' missing"), - .errPos = state.positions[noPos] - })); + state.error("required attribute 'system' missing") + .atPos(v) + .debugThrow(); /* Check whether the derivation name is valid. */ if (isDerivation(drvName) && @@ -1298,10 +1289,10 @@ drvName, Bindings * attrs, Value & v) outputs.size() == 1 && *(outputs.begin()) == "out")) { - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("derivation names are allowed to end in '%s' only if they produce a single derivation file", drvExtension), - .errPos = state.positions[noPos] - })); + state.error( + "derivation names are allowed to end in '%s' only if they produce a single derivation file", + drvExtension + ).atPos(v).debugThrow(); } if (outputHash) { @@ -1310,10 +1301,9 @@ drvName, Bindings * attrs, Value & v) Ignore `__contentAddressed` because fixed output derivations are already content addressed. */ if (outputs.size() != 1 || *(outputs.begin()) != "out") - state.debugThrowLastTrace(Error({ - .msg = hintfmt("multiple outputs are not supported in fixed-output derivations"), - .errPos = state.positions[noPos] - })); + state.error( + "multiple outputs are not supported in fixed-output derivations" + ).atPos(v).debugThrow(); auto h = newHashAllowEmpty(*outputHash, parseHashAlgoOpt(outputHashAlgo)); @@ -1332,10 +1322,8 @@ drvName, Bindings * attrs, Value & v) else if (contentAddressed || isImpure) { if (contentAddressed && isImpure) - throw EvalError({ - .msg = hintfmt("derivation cannot be both content-addressed and impure"), - .errPos = state.positions[noPos] - }); + state.error("derivation cannot be both content-addressed and impure") + .atPos(v).debugThrow(); auto ha = parseHashAlgoOpt(outputHashAlgo).value_or(HashAlgorithm::SHA256); auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive); @@ -1376,10 +1364,10 @@ drvName, Bindings * attrs, Value & v) for (auto & i : outputs) { auto h = get(hashModulo.hashes, i); if (!h) - throw AssertionError({ - .msg = hintfmt("derivation produced no hash for output '%s'", i), - .errPos = state.positions[noPos], - }); + state.error( + "derivation produced no hash for output '%s'", + i + ).atPos(v).debugThrow(); auto outPath = state.store->makeOutputPath(i, *h, drvName); drv.env[i] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign( @@ -1485,10 +1473,10 @@ static RegisterPrimOp primop_toPath({ static void prim_storePath(EvalState & state, const PosIdx pos, Value * * args, Value & v) { if (evalSettings.pureEval) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("'%s' is not allowed in pure evaluation mode", "builtins.storePath"), - .errPos = state.positions[pos] - })); + state.error( + "'%s' is not allowed in pure evaluation mode", + "builtins.storePath" + ).atPos(pos).debugThrow(); NixStringContext context; auto path = state.coerceToPath(pos, *args[0], context, "while evaluating the first argument passed to 'builtins.storePath'").path; @@ -1498,10 +1486,8 @@ static void prim_storePath(EvalState & state, const PosIdx pos, Value * * args, if (!state.store->isStorePath(path.abs())) path = CanonPath(canonPath(path.abs(), true)); if (!state.store->isInStore(path.abs())) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("path '%1%' is not in the Nix store", path), - .errPos = state.positions[pos] - })); + state.error("path '%1%' is not in the Nix store", path) + .atPos(pos).debugThrow(); auto path2 = state.store->toStorePath(path.abs()).first; if (!settings.readOnlyMode) state.store->ensurePath(path2); @@ -1616,7 +1602,10 @@ static void prim_readFile(EvalState & state, const PosIdx pos, Value * * args, V auto path = realisePath(state, pos, *args[0]); auto s = path.readFile(); if (s.find((char) 0) != std::string::npos) - state.debugThrowLastTrace(Error("the contents of the file '%1%' cannot be represented as a Nix string", path)); + state.error( + "the contents of the file '%1%' cannot be represented as a Nix string", + path + ).atPos(pos).debugThrow(); StorePathSet refs; if (state.store->isInStore(path.path.abs())) { try { @@ -1673,10 +1662,11 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V auto rewrites = state.realiseContext(context); path = rewriteStrings(path, rewrites); } catch (InvalidPathError & e) { - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("cannot find '%1%', since path '%2%' is not valid", path, e.path), - .errPos = state.positions[pos] - })); + state.error( + "cannot find '%1%', since path '%2%' is not valid", + path, + e.path + ).atPos(pos).debugThrow(); } searchPath.elements.emplace_back(SearchPath::Elem { @@ -1745,10 +1735,7 @@ static void prim_hashFile(EvalState & state, const PosIdx pos, Value * * args, V auto algo = state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.hashFile"); std::optional ha = parseHashAlgo(algo); if (!ha) - state.debugThrowLastTrace(Error({ - .msg = hintfmt("unknown hash algo '%1%'", algo), - .errPos = state.positions[pos] - })); + state.error("unknown hash algorithm '%1%'", algo).atPos(pos).debugThrow(); auto path = realisePath(state, pos, *args[1]); @@ -2068,13 +2055,12 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val if (auto p = std::get_if(&c.raw)) refs.insert(p->path); else - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt( - "in 'toFile': the file named '%1%' must not contain a reference " - "to a derivation but contains (%2%)", - name, c.to_string()), - .errPos = state.positions[pos] - })); + state.error( + "files created by %1% may not reference derivations, but %2% references %3%", + "builtins.toFile", + name, + c.to_string() + ).atPos(pos).debugThrow(); } auto storePath = settings.readOnlyMode @@ -2243,7 +2229,10 @@ static void addPath( if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) { auto dstPath = fetchToStore(*state.store, path.resolveSymlinks(), name, method, filter.get(), state.repair); if (expectedHash && expectedStorePath != dstPath) - state.debugThrowLastTrace(Error("store path mismatch in (possibly filtered) path added from '%s'", path)); + state.error( + "store path mismatch in (possibly filtered) path added from '%s'", + path + ).atPos(pos).debugThrow(); state.allowAndSetStorePathString(dstPath, v); } else state.allowAndSetStorePathString(*expectedStorePath, v); @@ -2343,16 +2332,15 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value else if (n == "sha256") expectedHash = newHashAllowEmpty(state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the `sha256` attribute passed to builtins.path"), HashAlgorithm::SHA256); else - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("unsupported argument '%1%' to 'addPath'", state.symbols[attr.name]), - .errPos = state.positions[attr.pos] - })); + state.error( + "unsupported argument '%1%' to 'addPath'", + state.symbols[attr.name] + ).atPos(attr.pos).debugThrow(); } if (!path) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("missing required 'path' attribute in the first argument to builtins.path"), - .errPos = state.positions[pos] - })); + state.error( + "missing required 'path' attribute in the first argument to builtins.path" + ).atPos(pos).debugThrow(); if (name.empty()) name = path->baseName(); @@ -2770,10 +2758,7 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value * * arg return; } if (!args[0]->isLambda()) - state.debugThrowLastTrace(TypeError({ - .msg = hintfmt("'functionArgs' requires a function"), - .errPos = state.positions[pos] - })); + state.error("'functionArgs' requires a function").atPos(pos).debugThrow(); if (!args[0]->lambda.fun->hasFormals()) { v.mkAttrs(&state.emptyBindings); @@ -2943,10 +2928,10 @@ static void elemAt(EvalState & state, const PosIdx pos, Value & list, int n, Val { state.forceList(list, pos, "while evaluating the first argument passed to builtins.elemAt"); if (n < 0 || (unsigned int) n >= list.listSize()) - state.debugThrowLastTrace(Error({ - .msg = hintfmt("list index %1% is out of bounds", n), - .errPos = state.positions[pos] - })); + state.error( + "list index %1% is out of bounds", + n + ).atPos(pos).debugThrow(); state.forceValue(*list.listElems()[n], pos); v = *list.listElems()[n]; } @@ -2991,10 +2976,7 @@ static void prim_tail(EvalState & state, const PosIdx pos, Value * * args, Value { state.forceList(*args[0], pos, "while evaluating the first argument passed to builtins.tail"); if (args[0]->listSize() == 0) - state.debugThrowLastTrace(Error({ - .msg = hintfmt("'tail' called on an empty list"), - .errPos = state.positions[pos] - })); + state.error("'tail' called on an empty list").atPos(pos).debugThrow(); state.mkList(v, args[0]->listSize() - 1); for (unsigned int n = 0; n < v.listSize(); ++n) @@ -3251,7 +3233,7 @@ static void prim_genList(EvalState & state, const PosIdx pos, Value * * args, Va auto len = state.forceInt(*args[1], pos, "while evaluating the second argument passed to builtins.genList"); if (len < 0) - state.error("cannot create list of size %1%", len).debugThrow(); + state.error("cannot create list of size %1%", len).atPos(pos).debugThrow(); // More strict than striclty (!) necessary, but acceptable // as evaluating map without accessing any values makes little sense. @@ -3568,10 +3550,7 @@ static void prim_div(EvalState & state, const PosIdx pos, Value * * args, Value NixFloat f2 = state.forceFloat(*args[1], pos, "while evaluating the second operand of the division"); if (f2 == 0) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("division by zero"), - .errPos = state.positions[pos] - })); + state.error("division by zero").atPos(pos).debugThrow(); if (args[0]->type() == nFloat || args[1]->type() == nFloat) { v.mkFloat(state.forceFloat(*args[0], pos, "while evaluating the first operand of the division") / f2); @@ -3580,10 +3559,7 @@ static void prim_div(EvalState & state, const PosIdx pos, Value * * args, Value NixInt i2 = state.forceInt(*args[1], pos, "while evaluating the second operand of the division"); /* Avoid division overflow as it might raise SIGFPE. */ if (i1 == std::numeric_limits::min() && i2 == -1) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("overflow in integer division"), - .errPos = state.positions[pos] - })); + state.error("overflow in integer division").atPos(pos).debugThrow(); v.mkInt(i1 / i2); } @@ -3714,10 +3690,7 @@ static void prim_substring(EvalState & state, const PosIdx pos, Value * * args, int start = state.forceInt(*args[0], pos, "while evaluating the first argument (the start offset) passed to builtins.substring"); if (start < 0) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("negative start position in 'substring'"), - .errPos = state.positions[pos] - })); + state.error("negative start position in 'substring'").atPos(pos).debugThrow(); int len = state.forceInt(*args[1], pos, "while evaluating the second argument (the substring length) passed to builtins.substring"); @@ -3782,10 +3755,7 @@ static void prim_hashString(EvalState & state, const PosIdx pos, Value * * args, auto algo = state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.hashString"); std::optional ha = parseHashAlgo(algo); if (!ha) - state.debugThrowLastTrace(Error({ - .msg = hintfmt("unknown hash algo '%1%'", algo), - .errPos = state.positions[pos] - })); + state.error("unknown hash algorithm '%1%'", algo).atPos(pos).debugThrow(); NixStringContext context; // discarded auto s = state.forceString(*args[1], context, pos, "while evaluating the second argument passed to builtins.hashString"); @@ -3951,15 +3921,13 @@ void prim_match(EvalState & state, const PosIdx pos, Value * * args, Value & v) } catch (std::regex_error & e) { if (e.code() == std::regex_constants::error_space) { // limit is _GLIBCXX_REGEX_STATE_LIMIT for libstdc++ - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("memory limit exceeded by regular expression '%s'", re), - .errPos = state.positions[pos] - })); + state.error("memory limit exceeded by regular expression '%s'", re) + .atPos(pos) + .debugThrow(); } else - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("invalid regular expression '%s'", re), - .errPos = state.positions[pos] - })); + state.error("invalid regular expression '%s'", re) + .atPos(pos) + .debugThrow(); } } @@ -4055,15 +4023,13 @@ void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v) } catch (std::regex_error & e) { if (e.code() == std::regex_constants::error_space) { // limit is _GLIBCXX_REGEX_STATE_LIMIT for libstdc++ - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("memory limit exceeded by regular expression '%s'", re), - .errPos = state.positions[pos] - })); + state.error("memory limit exceeded by regular expression '%s'", re) + .atPos(pos) + .debugThrow(); } else - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("invalid regular expression '%s'", re), - .errPos = state.positions[pos] - })); + state.error("invalid regular expression '%s'", re) + .atPos(pos) + .debugThrow(); } } @@ -4139,7 +4105,9 @@ static void prim_replaceStrings(EvalState & state, const PosIdx pos, Value * * a state.forceList(*args[0], pos, "while evaluating the first argument passed to builtins.replaceStrings"); state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.replaceStrings"); if (args[0]->listSize() != args[1]->listSize()) - state.error("'from' and 'to' arguments passed to builtins.replaceStrings have different lengths").atPos(pos).debugThrow(); + state.error( + "'from' and 'to' arguments passed to builtins.replaceStrings have different lengths" + ).atPos(pos).debugThrow(); std::vector from; from.reserve(args[0]->listSize()); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index db940f277..1eec8b316 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -98,30 +98,30 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V auto contextSize = context.size(); if (contextSize != 1) { - throw EvalError({ - .msg = hintfmt("context of string '%s' must have exactly one element, but has %d", *s, contextSize), - .errPos = state.positions[pos] - }); + state.error( + "context of string '%s' must have exactly one element, but has %d", + *s, + contextSize + ).atPos(pos).debugThrow(); } NixStringContext context2 { (NixStringContextElem { std::visit(overloaded { [&](const NixStringContextElem::Opaque & c) -> NixStringContextElem::DrvDeep { if (!c.path.isDerivation()) { - throw EvalError({ - .msg = hintfmt("path '%s' is not a derivation", - state.store->printStorePath(c.path)), - .errPos = state.positions[pos], - }); + state.error( + "path '%s' is not a derivation", + state.store->printStorePath(c.path) + ).atPos(pos).debugThrow(); } return NixStringContextElem::DrvDeep { .drvPath = c.path, }; }, [&](const NixStringContextElem::Built & c) -> NixStringContextElem::DrvDeep { - throw EvalError({ - .msg = hintfmt("`addDrvOutputDependencies` can only act on derivations, not on a derivation output such as '%1%'", c.output), - .errPos = state.positions[pos], - }); + state.error( + "`addDrvOutputDependencies` can only act on derivations, not on a derivation output such as '%1%'", + c.output + ).atPos(pos).debugThrow(); }, [&](const NixStringContextElem::DrvDeep & c) -> NixStringContextElem::DrvDeep { /* Reuse original item because we want this to be idempotent. */ @@ -261,10 +261,10 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * ar for (auto & i : *args[1]->attrs) { const auto & name = state.symbols[i.name]; if (!state.store->isStorePath(name)) - throw EvalError({ - .msg = hintfmt("context key '%s' is not a store path", name), - .errPos = state.positions[i.pos] - }); + state.error( + "context key '%s' is not a store path", + name + ).atPos(i.pos).debugThrow(); auto namePath = state.store->parseStorePath(name); if (!settings.readOnlyMode) state.store->ensurePath(namePath); @@ -281,10 +281,10 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * ar if (iter != i.value->attrs->end()) { if (state.forceBool(*iter->value, iter->pos, "while evaluating the `allOutputs` attribute of a string context")) { if (!isDerivation(name)) { - throw EvalError({ - .msg = hintfmt("tried to add all-outputs context of %s, which is not a derivation, to a string", name), - .errPos = state.positions[i.pos] - }); + state.error( + "tried to add all-outputs context of %s, which is not a derivation, to a string", + name + ).atPos(i.pos).debugThrow(); } context.emplace(NixStringContextElem::DrvDeep { .drvPath = namePath, @@ -296,10 +296,10 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * ar if (iter != i.value->attrs->end()) { state.forceList(*iter->value, iter->pos, "while evaluating the `outputs` attribute of a string context"); if (iter->value->listSize() && !isDerivation(name)) { - throw EvalError({ - .msg = hintfmt("tried to add derivation output context of %s, which is not a derivation, to a string", name), - .errPos = state.positions[i.pos] - }); + state.error( + "tried to add derivation output context of %s, which is not a derivation, to a string", + name + ).atPos(i.pos).debugThrow(); } for (auto elem : iter->value->listItems()) { auto outputName = state.forceStringNoCtx(*elem, iter->pos, "while evaluating an output name within a string context"); diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 27147a5d1..5806b3ff9 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -27,7 +27,7 @@ static void runFetchClosureWithRewrite(EvalState & state, const PosIdx pos, Stor state.store->printStorePath(fromPath), state.store->printStorePath(rewrittenPath), state.store->printStorePath(*toPathMaybe)), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); if (!toPathMaybe) throw Error({ @@ -36,7 +36,7 @@ static void runFetchClosureWithRewrite(EvalState & state, const PosIdx pos, Stor "Use this value for the 'toPath' attribute passed to 'fetchClosure'", state.store->printStorePath(fromPath), state.store->printStorePath(rewrittenPath)), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); } @@ -54,7 +54,7 @@ static void runFetchClosureWithRewrite(EvalState & state, const PosIdx pos, Stor "The 'toPath' value '%s' is input-addressed, so it can't possibly be the result of rewriting to a content-addressed path.\n\n" "Set 'toPath' to an empty string to make Nix report the correct content-addressed path.", state.store->printStorePath(toPath)), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); } @@ -80,7 +80,7 @@ static void runFetchClosureWithContentAddressedPath(EvalState & state, const Pos "to the 'fetchClosure' arguments.\n\n" "Note that to ensure authenticity input-addressed store paths, users must configure a trusted binary cache public key on their systems. This is not needed for content-addressed paths.", state.store->printStorePath(fromPath)), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); } @@ -103,7 +103,7 @@ static void runFetchClosureWithInputAddressedPath(EvalState & state, const PosId "The store object referred to by 'fromPath' at '%s' is not input-addressed, but 'inputAddressed' is set to 'true'.\n\n" "Remove the 'inputAddressed' attribute (it defaults to 'false') to expect 'fromPath' to be content-addressed", state.store->printStorePath(fromPath)), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); } @@ -154,14 +154,14 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg else throw Error({ .msg = hintfmt("attribute '%s' isn't supported in call to 'fetchClosure'", attrName), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); } if (!fromPath) throw Error({ .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromPath"), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); bool inputAddressed = inputAddressedMaybe.value_or(false); @@ -172,14 +172,14 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg .msg = hintfmt("attribute '%s' is set to true, but '%s' is also set. Please remove one of them", "inputAddressed", "toPath"), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); } if (!fromStoreUrl) throw Error({ .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); auto parsedURL = parseURL(*fromStoreUrl); @@ -189,13 +189,13 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg !(getEnv("_NIX_IN_TEST").has_value() && parsedURL.scheme == "file")) throw Error({ .msg = hintfmt("'fetchClosure' only supports http:// and https:// stores"), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); if (!parsedURL.query.empty()) throw Error({ .msg = hintfmt("'fetchClosure' does not support URL query parameters (in '%s')", *fromStoreUrl), - .errPos = state.positions[pos] + .pos = state.positions[pos] }); auto fromStore = openStore(parsedURL.to_string()); diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index 58fe6f173..bb029b5b3 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -38,17 +38,11 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value * * a else if (n == "name") name = state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the `name` attribute passed to builtins.fetchMercurial"); else - throw EvalError({ - .msg = hintfmt("unsupported argument '%s' to 'fetchMercurial'", state.symbols[attr.name]), - .errPos = state.positions[attr.pos] - }); + state.error("unsupported argument '%s' to 'fetchMercurial'", state.symbols[attr.name]).atPos(attr.pos).debugThrow(); } if (url.empty()) - throw EvalError({ - .msg = hintfmt("'url' argument required"), - .errPos = state.positions[pos] - }); + state.error("'url' argument required").atPos(pos).debugThrow(); } else url = state.coerceToString(pos, *args[0], context, diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index a943095bb..1997d5513 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -100,16 +100,14 @@ static void fetchTree( if (auto aType = args[0]->attrs->get(state.sType)) { if (type) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("unexpected attribute 'type'"), - .errPos = state.positions[pos] - })); + state.error( + "unexpected attribute 'type'" + ).atPos(pos).debugThrow(); type = state.forceStringNoCtx(*aType->value, aType->pos, "while evaluating the `type` attribute passed to builtins.fetchTree"); } else if (!type) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("attribute 'type' is missing in call to 'fetchTree'"), - .errPos = state.positions[pos] - })); + state.error( + "attribute 'type' is missing in call to 'fetchTree'" + ).atPos(pos).debugThrow(); attrs.emplace("type", type.value()); @@ -132,8 +130,8 @@ static void fetchTree( attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, *attr.value, pos, context).dump()); } else - state.debugThrowLastTrace(TypeError("fetchTree argument '%s' is %s while a string, Boolean or integer is expected", - state.symbols[attr.name], showType(*attr.value))); + state.error("fetchTree argument '%s' is %s while a string, Boolean or integer is expected", + state.symbols[attr.name], showType(*attr.value)).debugThrow(); } if (params.isFetchGit && !attrs.contains("exportIgnore") && (!attrs.contains("submodules") || !*fetchers::maybeGetBoolAttr(attrs, "submodules"))) { @@ -142,10 +140,9 @@ static void fetchTree( if (!params.allowNameArgument) if (auto nameIter = attrs.find("name"); nameIter != attrs.end()) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("attribute 'name' isn’t supported in call to 'fetchTree'"), - .errPos = state.positions[pos] - })); + state.error( + "attribute 'name' isn’t supported in call to 'fetchTree'" + ).atPos(pos).debugThrow(); input = fetchers::Input::fromAttrs(std::move(attrs)); } else { @@ -163,10 +160,9 @@ static void fetchTree( input = fetchers::Input::fromAttrs(std::move(attrs)); } else { if (!experimentalFeatureSettings.isEnabled(Xp::Flakes)) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("passing a string argument to 'fetchTree' requires the 'flakes' experimental feature"), - .errPos = state.positions[pos] - })); + state.error( + "passing a string argument to 'fetchTree' requires the 'flakes' experimental feature" + ).atPos(pos).debugThrow(); input = fetchers::Input::fromURL(url); } } @@ -175,10 +171,14 @@ static void fetchTree( input = lookupInRegistries(state.store, input).first; if (evalSettings.pureEval && !input.isLocked()) { + auto fetcher = "fetchTree"; if (params.isFetchGit) - state.debugThrowLastTrace(EvalError("in pure evaluation mode, 'fetchGit' requires a locked input, at %s", state.positions[pos])); - else - state.debugThrowLastTrace(EvalError("in pure evaluation mode, 'fetchTree' requires a locked input, at %s", state.positions[pos])); + fetcher = "fetchGit"; + + state.error( + "in pure evaluation mode, %s requires a locked input", + fetcher + ).atPos(pos).debugThrow(); } state.checkURI(input.toURLString()); @@ -432,17 +432,13 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v else if (n == "name") name = state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the name of the content we should fetch"); else - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("unsupported argument '%s' to '%s'", n, who), - .errPos = state.positions[attr.pos] - })); + state.error("unsupported argument '%s' to '%s'", n, who) + .atPos(pos).debugThrow(); } if (!url) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("'url' argument required"), - .errPos = state.positions[pos] - })); + state.error( + "'url' argument required").atPos(pos).debugThrow(); } else url = state.forceStringNoCtx(*args[0], pos, "while evaluating the url we should fetch"); @@ -455,7 +451,7 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v name = baseNameOf(*url); if (evalSettings.pureEval && !expectedHash) - state.debugThrowLastTrace(EvalError("in pure evaluation mode, '%s' requires a 'sha256' argument", who)); + state.error("in pure evaluation mode, '%s' requires a 'sha256' argument", who).atPos(pos).debugThrow(); // early exit if pinned and already in the store if (expectedHash && expectedHash->algo == HashAlgorithm::SHA256) { @@ -484,9 +480,15 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v auto hash = unpack ? state.store->queryPathInfo(storePath)->narHash : hashFile(HashAlgorithm::SHA256, state.store->toRealPath(storePath)); - if (hash != *expectedHash) - state.debugThrowLastTrace(EvalError((unsigned int) 102, "hash mismatch in file downloaded from '%s':\n specified: %s\n got: %s", - *url, expectedHash->to_string(HashFormat::Nix32, true), hash.to_string(HashFormat::Nix32, true))); + if (hash != *expectedHash) { + state.error( + "hash mismatch in file downloaded from '%s':\n specified: %s\n got: %s", + *url, + expectedHash->to_string(HashFormat::Nix32, true), + hash.to_string(HashFormat::Nix32, true) + ).withExitStatus(102) + .debugThrow(); + } } state.allowAndSetStorePathString(storePath, v); diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index 2f4d4022e..94be7960a 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -83,10 +83,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value * * args, V try { visit(val, toml::parse(tomlStream, "fromTOML" /* the "filename" */)); } catch (std::exception & e) { // TODO: toml::syntax_error - throw EvalError({ - .msg = hintfmt("while parsing a TOML string: %s", e.what()), - .errPos = state.positions[pos] - }); + state.error("while parsing TOML: %s", e.what()).atPos(pos).debugThrow(); } } diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index 74b3ebf13..b2f116390 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -80,7 +80,7 @@ json printValueAsJSON(EvalState & state, bool strict, try { out.push_back(printValueAsJSON(state, strict, *elem, pos, context, copyToStore)); } catch (Error & e) { - e.addTrace({}, + e.addTrace(state.positions[pos], hintfmt("while evaluating list element at index %1%", i)); throw; } @@ -99,13 +99,12 @@ json printValueAsJSON(EvalState & state, bool strict, case nThunk: case nFunction: - auto e = TypeError({ - .msg = hintfmt("cannot convert %1% to JSON", showType(v)), - .errPos = state.positions[v.determinePos(pos)] - }); - e.addTrace(state.positions[pos], hintfmt("message for the trace")); - state.debugThrowLastTrace(e); - throw e; + state.error( + "cannot convert %1% to JSON", + showType(v) + ) + .atPos(v.determinePos(pos)) + .debugThrow(); } return out; } @@ -119,7 +118,8 @@ void printValueAsJSON(EvalState & state, bool strict, json ExternalValueBase::printValueAsJSON(EvalState & state, bool strict, NixStringContext & context, bool copyToStore) const { - state.debugThrowLastTrace(TypeError("cannot convert %1% to JSON", showType())); + state.error("cannot convert %1% to JSON", showType()) + .debugThrow(); } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 214d52271..e7aea4949 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -105,7 +105,7 @@ class ExternalValueBase * Coerce the value to a string. Defaults to uncoercable, i.e. throws an * error. */ - virtual std::string coerceToString(const Pos & pos, NixStringContext & context, bool copyMore, bool copyToStore) const; + virtual std::string coerceToString(EvalState & state, const PosIdx & pos, NixStringContext & context, bool copyMore, bool copyToStore) const; /** * Compare to another value of the same type. Defaults to uncomparable, diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 862ef355b..7b9b3c5b5 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -340,7 +340,7 @@ int handleExceptions(const std::string & programName, std::function fun) return 1; } catch (BaseError & e) { logError(e.info()); - return e.status; + return e.info().status; } catch (std::bad_alloc & e) { printError(error + "out of memory"); return 1; diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 7f0a05d5d..d4bead28e 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -33,7 +33,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod } if (failed.size() == 1 && ex) { - ex->status = worker.failingExitStatus(); + ex->withExitStatus(worker.failingExitStatus()); throw std::move(*ex); } else if (!failed.empty()) { if (ex) logError(ex->info()); @@ -104,7 +104,7 @@ void Store::ensurePath(const StorePath & path) if (goal->exitCode != Goal::ecSuccess) { if (goal->ex) { - goal->ex->status = worker.failingExitStatus(); + goal->ex->withExitStatus(worker.failingExitStatus()); throw std::move(*goal->ex); } else throw Error(worker.failingExitStatus(), "path '%s' does not exist and cannot be created", printStorePath(path)); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 27ad14ed4..8db93fa39 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -119,7 +119,7 @@ struct TunnelLogger : public Logger if (GET_PROTOCOL_MINOR(clientVersion) >= 26) { to << STDERR_ERROR << *ex; } else { - to << STDERR_ERROR << ex->what() << ex->status; + to << STDERR_ERROR << ex->what() << ex->info().status; } } } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 1f0cb08c9..e4e50d73b 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -335,7 +335,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s * try { * e->eval(*this, env, v); * if (v.type() != nAttrs) - * throwTypeError("expected a set but found %1%", v); + * error("expected a set but found %1%", v); * } catch (Error & e) { * e.addTrace(pos, errorCtx); * throw; @@ -349,7 +349,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s * e->eval(*this, env, v); * try { * if (v.type() != nAttrs) - * throwTypeError("expected a set but found %1%", v); + * error("expected a set but found %1%", v); * } catch (Error & e) { * e.addTrace(pos, errorCtx); * throw; @@ -411,7 +411,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s oss << einfo.msg << "\n"; - printPosMaybe(oss, "", einfo.errPos); + printPosMaybe(oss, "", einfo.pos); auto suggestions = einfo.suggestions.trim(); if (!suggestions.suggestions.empty()) { diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 764fac1ce..9f9302020 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -84,9 +84,14 @@ inline bool operator>=(const Trace& lhs, const Trace& rhs); struct ErrorInfo { Verbosity level; hintformat msg; - std::shared_ptr errPos; + std::shared_ptr pos; std::list traces; + /** + * Exit status. + */ + unsigned int status = 1; + Suggestions suggestions; static std::optional programName; @@ -103,18 +108,21 @@ class BaseError : public std::exception protected: mutable ErrorInfo err; + /** + * Cached formatted contents of `err.msg`. + */ mutable std::optional what_; + /** + * Format `err.msg` and set `what_` to the resulting value. + */ const std::string & calcWhat() const; 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...) } - , status(status) + : err { .level = lvlError, .msg = hintfmt(args...), .status = status } { } template @@ -149,6 +157,15 @@ public: const std::string & msg() const { return calcWhat(); } const ErrorInfo & info() const { calcWhat(); return err; } + void withExitStatus(unsigned int status) + { + err.status = status; + } + + void atPos(std::shared_ptr pos) { + err.pos = pos; + } + void pushTrace(Trace trace) { err.traces.push_front(trace); diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index d68ddacc0..89fbd194a 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -199,7 +199,7 @@ struct JSONLogger : Logger { json["level"] = ei.level; json["msg"] = oss.str(); json["raw_msg"] = ei.msg.str(); - to_json(json, ei.errPos); + to_json(json, ei.pos); if (loggerSettings.showTrace.get() && !ei.traces.empty()) { nlohmann::json traces = nlohmann::json::array(); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 40378e123..017818ed5 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -950,8 +950,8 @@ static void opServe(Strings opFlags, Strings opArgs) store->buildPaths(toDerivedPaths(paths)); out << 0; } catch (Error & e) { - assert(e.status); - out << e.status << e.msg(); + assert(e.info().status); + out << e.info().status << e.msg(); } break; } diff --git a/src/nix/eval.cc b/src/nix/eval.cc index a89fa7412..2e0837c8e 100644 --- a/src/nix/eval.cc +++ b/src/nix/eval.cc @@ -104,7 +104,7 @@ struct CmdEval : MixJSON, InstallableValueCommand, MixReadOnlyOption } } else - throw TypeError("value at '%s' is not a string or an attribute set", state->positions[pos]); + state->error("value at '%s' is not a string or an attribute set", state->positions[pos]).debugThrow(); }; recurse(*v, pos, *writeTo); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 0e34bd76a..646e4c831 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -848,10 +848,10 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand auto templateDir = templateDirAttr->getString(); if (!store->isInStore(templateDir)) - throw TypeError( + evalState->error( "'%s' was not found in the Nix store\n" "If you've set '%s' to a string, try using a path instead.", - templateDir, templateDirAttr->getAttrPathStr()); + templateDir, templateDirAttr->getAttrPathStr()).debugThrow(); std::vector changedFiles; std::vector conflictedFiles; @@ -1321,7 +1321,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON { auto aType = visitor.maybeGetAttr("type"); if (!aType || aType->getString() != "app") - throw EvalError("not an app definition"); + state->error("not an app definition").debugThrow(); if (json) { j.emplace("type", "app"); } else { diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index c6a482035..ea90f8ebe 100644 --- a/tests/functional/fetchGit.sh +++ b/tests/functional/fetchGit.sh @@ -67,7 +67,7 @@ path2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \" [[ $(nix eval --raw --expr "builtins.readFile (fetchGit { url = file://$repo; rev = \"$rev2\"; } + \"/hello\")") = world ]] # But without a hash, it fails -expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'fetchGit' requires a locked input" +expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "fetchGit requires a locked input" # Fetch again. This should be cached. mv $repo ${repo}-tmp @@ -208,7 +208,7 @@ path6=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; ur [[ $path3 = $path6 ]] [[ $(nix eval --impure --expr "(builtins.fetchTree { type = \"git\"; url = \"file://$TEST_ROOT/shallow\"; ref = \"dev\"; shallow = true; }).revCount or 123") == 123 ]] -expectStderr 1 nix eval --expr 'builtins.fetchTree { type = "git"; url = "file:///foo"; }' | grepQuiet "'fetchTree' requires a locked input" +expectStderr 1 nix eval --expr 'builtins.fetchTree { type = "git"; url = "file:///foo"; }' | grepQuiet "fetchTree requires a locked input" # Explicit ref = "HEAD" should work, and produce the same outPath as without ref path7=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; }).outPath") diff --git a/tests/functional/lang/eval-fail-attr-name-type.err.exp b/tests/functional/lang/eval-fail-attr-name-type.err.exp index c8d56ba7d..6848a35ed 100644 --- a/tests/functional/lang/eval-fail-attr-name-type.err.exp +++ b/tests/functional/lang/eval-fail-attr-name-type.err.exp @@ -14,3 +14,8 @@ error: 8| error: expected a string but found an integer: 1 + at /pwd/lang/eval-fail-attr-name-type.nix:7:17: + 6| in + 7| attrs.puppy.${key} + | ^ + 8| diff --git a/tests/functional/lang/eval-fail-fromTOML-timestamps.err.exp b/tests/functional/lang/eval-fail-fromTOML-timestamps.err.exp index 73f9df8cc..9bbb251e1 100644 --- a/tests/functional/lang/eval-fail-fromTOML-timestamps.err.exp +++ b/tests/functional/lang/eval-fail-fromTOML-timestamps.err.exp @@ -5,4 +5,4 @@ error: | ^ 2| key = "value" - error: while parsing a TOML string: Dates and times are not supported + error: while parsing TOML: Dates and times are not supported diff --git a/tests/functional/lang/eval-fail-toJSON.err.exp b/tests/functional/lang/eval-fail-toJSON.err.exp index 4f6003437..ad267711b 100644 --- a/tests/functional/lang/eval-fail-toJSON.err.exp +++ b/tests/functional/lang/eval-fail-toJSON.err.exp @@ -20,6 +20,11 @@ error: 3| true … while evaluating list element at index 3 + at /pwd/lang/eval-fail-toJSON.nix:2:3: + 1| builtins.toJSON { + 2| a.b = [ + | ^ + 3| true … while evaluating attribute 'c' at /pwd/lang/eval-fail-toJSON.nix:7:7: diff --git a/tests/functional/lang/eval-fail-using-set-as-attr-name.err.exp b/tests/functional/lang/eval-fail-using-set-as-attr-name.err.exp index 94784c651..4326c9650 100644 --- a/tests/functional/lang/eval-fail-using-set-as-attr-name.err.exp +++ b/tests/functional/lang/eval-fail-using-set-as-attr-name.err.exp @@ -7,3 +7,8 @@ error: 6| error: expected a string but found a set: { } + at /pwd/lang/eval-fail-using-set-as-attr-name.nix:5:10: + 4| in + 5| attr.${key} + | ^ + 6| diff --git a/tests/unit/libexpr/error_traces.cc b/tests/unit/libexpr/error_traces.cc index 5fca79304..d0d7ca79c 100644 --- a/tests/unit/libexpr/error_traces.cc +++ b/tests/unit/libexpr/error_traces.cc @@ -12,33 +12,33 @@ namespace nix { TEST_F(ErrorTraceTest, TraceBuilder) { ASSERT_THROW( - state.error("Not much").debugThrow(), + state.error("puppy").debugThrow(), EvalError ); ASSERT_THROW( - state.error("Not much").withTrace(noPos, "No more").debugThrow(), + state.error("puppy").withTrace(noPos, "doggy").debugThrow(), EvalError ); ASSERT_THROW( try { try { - state.error("Not much").withTrace(noPos, "No more").debugThrow(); + state.error("puppy").withTrace(noPos, "doggy").debugThrow(); } catch (Error & e) { - e.addTrace(state.positions[noPos], "Something", ""); + e.addTrace(state.positions[noPos], "beans", ""); throw; } } catch (BaseError & e) { ASSERT_EQ(PrintToString(e.info().msg), - PrintToString(hintfmt("Not much"))); + PrintToString(hintfmt("puppy"))); auto trace = e.info().traces.rbegin(); ASSERT_EQ(e.info().traces.size(), 2); ASSERT_EQ(PrintToString(trace->hint), - PrintToString(hintfmt("No more"))); + PrintToString(hintfmt("doggy"))); trace++; ASSERT_EQ(PrintToString(trace->hint), - PrintToString(hintfmt("Something"))); + PrintToString(hintfmt("beans"))); throw; } , EvalError @@ -47,12 +47,12 @@ namespace nix { TEST_F(ErrorTraceTest, NestedThrows) { try { - state.error("Not much").withTrace(noPos, "No more").debugThrow(); + state.error("puppy").withTrace(noPos, "doggy").debugThrow(); } catch (BaseError & e) { try { - state.error("Not much more").debugThrow(); + state.error("beans").debugThrow(); } catch (Error & e2) { - e.addTrace(state.positions[noPos], "Something", ""); + e.addTrace(state.positions[noPos], "beans2", ""); //e2.addTrace(state.positions[noPos], "Something", ""); ASSERT_TRUE(e.info().traces.size() == 2); ASSERT_TRUE(e2.info().traces.size() == 0); From 87dc4bc7d139a7eccb257e71558314a0d99e8d6a Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Thu, 1 Feb 2024 13:08:06 -0800 Subject: [PATCH 3/6] Attach positions to errors in `derivationStrict` --- src/libexpr/primops.cc | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 1eec6f961..69f89e0e0 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1085,9 +1085,10 @@ drvName, Bindings * attrs, Value & v) /* Check whether attributes should be passed as a JSON file. */ using nlohmann::json; std::optional jsonObject; + auto pos = v.determinePos(noPos); auto attr = attrs->find(state.sStructuredAttrs); if (attr != attrs->end() && - state.forceBool(*attr->value, noPos, + state.forceBool(*attr->value, pos, "while evaluating the `__structuredAttrs` " "attribute passed to builtins.derivationStrict")) jsonObject = json::object(); @@ -1096,7 +1097,7 @@ drvName, Bindings * attrs, Value & v) bool ignoreNulls = false; attr = attrs->find(state.sIgnoreNulls); if (attr != attrs->end()) - ignoreNulls = state.forceBool(*attr->value, noPos, "while evaluating the `__ignoreNulls` attribute " "passed to builtins.derivationStrict"); + ignoreNulls = state.forceBool(*attr->value, pos, "while evaluating the `__ignoreNulls` attribute " "passed to builtins.derivationStrict"); /* Build the derivation expression by processing the attributes. */ Derivation drv; @@ -1160,16 +1161,16 @@ drvName, Bindings * attrs, Value & v) const std::string_view context_below(""); if (ignoreNulls) { - state.forceValue(*i->value, noPos); + state.forceValue(*i->value, pos); if (i->value->type() == nNull) continue; } - if (i->name == state.sContentAddressed && state.forceBool(*i->value, noPos, context_below)) { + if (i->name == state.sContentAddressed && state.forceBool(*i->value, pos, context_below)) { contentAddressed = true; experimentalFeatureSettings.require(Xp::CaDerivations); } - else if (i->name == state.sImpure && state.forceBool(*i->value, noPos, context_below)) { + else if (i->name == state.sImpure && state.forceBool(*i->value, pos, context_below)) { isImpure = true; experimentalFeatureSettings.require(Xp::ImpureDerivations); } @@ -1177,9 +1178,9 @@ drvName, Bindings * attrs, Value & v) /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ else if (i->name == state.sArgs) { - state.forceList(*i->value, noPos, context_below); + state.forceList(*i->value, pos, context_below); for (auto elem : i->value->listItems()) { - auto s = state.coerceToString(noPos, *elem, context, + auto s = state.coerceToString(pos, *elem, context, "while evaluating an element of the argument list", true).toOwned(); drv.args.push_back(s); @@ -1194,29 +1195,29 @@ drvName, Bindings * attrs, Value & v) if (i->name == state.sStructuredAttrs) continue; - (*jsonObject)[key] = printValueAsJSON(state, true, *i->value, noPos, context); + (*jsonObject)[key] = printValueAsJSON(state, true, *i->value, pos, context); if (i->name == state.sBuilder) - drv.builder = state.forceString(*i->value, context, noPos, context_below); + drv.builder = state.forceString(*i->value, context, pos, context_below); else if (i->name == state.sSystem) - drv.platform = state.forceStringNoCtx(*i->value, noPos, context_below); + drv.platform = state.forceStringNoCtx(*i->value, pos, context_below); else if (i->name == state.sOutputHash) - outputHash = state.forceStringNoCtx(*i->value, noPos, context_below); + outputHash = state.forceStringNoCtx(*i->value, pos, context_below); else if (i->name == state.sOutputHashAlgo) - outputHashAlgo = state.forceStringNoCtx(*i->value, noPos, context_below); + outputHashAlgo = state.forceStringNoCtx(*i->value, pos, context_below); else if (i->name == state.sOutputHashMode) - handleHashMode(state.forceStringNoCtx(*i->value, noPos, context_below)); + handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below)); else if (i->name == state.sOutputs) { /* Require ‘outputs’ to be a list of strings. */ - state.forceList(*i->value, noPos, context_below); + state.forceList(*i->value, pos, context_below); Strings ss; for (auto elem : i->value->listItems()) - ss.emplace_back(state.forceStringNoCtx(*elem, noPos, context_below)); + ss.emplace_back(state.forceStringNoCtx(*elem, pos, context_below)); handleOutputs(ss); } } else { - auto s = state.coerceToString(noPos, *i->value, context, context_below, true).toOwned(); + auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned(); drv.env.emplace(key, s); if (i->name == state.sBuilder) drv.builder = std::move(s); else if (i->name == state.sSystem) drv.platform = std::move(s); From faaccecbc82d98288582bdc8ca96991796561371 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Thu, 1 Feb 2024 13:08:19 -0800 Subject: [PATCH 4/6] Remove `EXCEPTION_NEEDS_THROW_SPEC` We're on C++ 20 now, we don't need this --- src/libutil/error.hh | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 9f9302020..4fb822843 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -31,15 +31,6 @@ #include #include -/* Before 4.7, gcc's std::exception uses empty throw() specifiers for - * its (virtual) destructor and what() in c++11 mode, in violation of spec - */ -#ifdef __GNUC__ -#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 7) -#define EXCEPTION_NEEDS_THROW_SPEC -#endif -#endif - namespace nix { @@ -147,13 +138,7 @@ public: : err(e) { } -#ifdef EXCEPTION_NEEDS_THROW_SPEC - ~BaseError() throw () { }; - const char * what() const throw () { return calcWhat().c_str(); } -#else const char * what() const noexcept override { return calcWhat().c_str(); } -#endif - const std::string & msg() const { return calcWhat(); } const ErrorInfo & info() const { calcWhat(); return err; } From 474fc4078acbe062fcc31ce91c69c8f33bf00d5f Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Tue, 6 Feb 2024 16:49:28 -0800 Subject: [PATCH 5/6] Add comments --- src/libexpr/eval-error.cc | 2 +- src/libexpr/eval-error.hh | 30 ++++++++---------------------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/libexpr/eval-error.cc b/src/libexpr/eval-error.cc index b9411cbf4..250c59a19 100644 --- a/src/libexpr/eval-error.cc +++ b/src/libexpr/eval-error.cc @@ -91,7 +91,7 @@ void EvalErrorBuilder::debugThrow() // `EvalState` is the only class that can construct an `EvalErrorBuilder`, // and it does so in dynamic storage. This is the final method called on - // any such instancve and must delete itself before throwing the underlying + // any such instance and must delete itself before throwing the underlying // error. auto error = std::move(this->error); delete this; diff --git a/src/libexpr/eval-error.hh b/src/libexpr/eval-error.hh index ee69dce64..711743886 100644 --- a/src/libexpr/eval-error.hh +++ b/src/libexpr/eval-error.hh @@ -56,6 +56,11 @@ public: } }; +/** + * `EvalErrorBuilder`s may only be constructed by `EvalState`. The `debugThrow` + * method must be the final method in any such `EvalErrorBuilder` usage, and it + * handles deleting the object. + */ template class EvalErrorBuilder final { @@ -90,29 +95,10 @@ public: [[nodiscard, gnu::noinline]] EvalErrorBuilder & addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs); + /** + * Delete the `EvalErrorBuilder` and throw the underlying exception. + */ [[gnu::noinline, gnu::noreturn]] void debugThrow(); }; -/** - * The size needed to allocate any `EvalErrorBuilder`. - * - * The list of classes here needs to be kept in sync with the list of `template - * class` declarations in `eval-error.cc`. - * - * This is used by `EvalState` to preallocate a buffer of sufficient size for - * any `EvalErrorBuilder` to avoid allocating while evaluating Nix code. - */ -constexpr size_t EVAL_ERROR_BUILDER_SIZE = std::max({ - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), - sizeof(EvalErrorBuilder), -}); - } From 9723f533d85133fa3c4d9421a58c7765cb61e733 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Tue, 6 Feb 2024 16:50:47 -0800 Subject: [PATCH 6/6] Add comment --- src/libexpr/eval.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index afe89cd30..3c7c5da27 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -239,6 +239,7 @@ public: template [[nodiscard, gnu::noinline]] EvalErrorBuilder & error(const Args & ... args) { + // `EvalErrorBuilder::debugThrow` performs the corresponding `delete`. return *new EvalErrorBuilder(*this, args...); }