diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index df88e32a7..c719f660c 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -210,17 +210,16 @@ static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positi out << dt.hint.str() << "\n"; // prefer direct pos, but if noPos then try the expr. - auto pos = *dt.pos - ? *dt.pos - : positions[dt.expr.getPos() ? dt.expr.getPos() : noPos]; + auto pos = dt.pos + ? dt.pos + : (std::shared_ptr) positions[dt.expr.getPos() ? dt.expr.getPos() : noPos]; if (pos) { - printAtPos(pos, out); + pos->print(out); - auto loc = getCodeLines(pos); - if (loc.has_value()) { + if (auto loc = pos->getCodeLines()) {; out << "\n"; - printCodeLines(out, "", pos, *loc); + printCodeLines(out, "", *pos, *loc); out << "\n"; } } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 2a554f0d1..20afd20fe 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -775,7 +775,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().errPos ? error->info().errPos : (std::shared_ptr) positions[expr.getPos()], .expr = expr, .env = env, .hint = error->info().msg, @@ -957,7 +957,7 @@ void EvalState::throwMissingArgumentError(const PosIdx pos, const char * s, cons void EvalState::addErrorTrace(Error & e, const char * s, const std::string & s2) const { - e.addTrace(std::nullopt, s, s2); + e.addTrace(nullptr, s, s2); } void EvalState::addErrorTrace(Error & e, const PosIdx pos, const char * s, const std::string & s2) const @@ -969,13 +969,13 @@ static std::unique_ptr makeDebugTraceStacker( EvalState & state, Expr & expr, Env & env, - std::optional pos, + std::shared_ptr && pos, const char * s, const std::string & s2) { return std::make_unique(state, DebugTrace { - .pos = pos, + .pos = std::move(pos), .expr = expr, .env = env, .hint = hintfmt(s, s2), @@ -1171,7 +1171,7 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial) *this, *e, this->baseEnv, - e->getPos() ? std::optional(ErrPos(positions[e->getPos()])) : std::nullopt, + e->getPos() ? (std::shared_ptr) positions[e->getPos()] : nullptr, "while evaluating the file '%1%':", resolvedPath.to_string()) : nullptr; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 45cee1d7d..11038317f 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -76,7 +76,7 @@ struct RegexCache; std::shared_ptr makeRegexCache(); struct DebugTrace { - std::optional pos; + std::shared_ptr pos; const Expr & expr; const Env & env; hintformat hint; diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 346741dd5..5ad5d1fd4 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -150,7 +150,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool withPaths, bool onlyOutputsToInstall /* Check for `meta.outputsToInstall` and return `outputs` reduced to that. */ const Value * outTI = queryMeta("outputsToInstall"); if (!outTI) return outputs; - const auto errMsg = Error("this derivation has bad 'meta.outputsToInstall'"); + auto errMsg = Error("this derivation has bad 'meta.outputsToInstall'"); /* ^ this shows during `nix-env -i` right under the bad derivation */ if (!outTI->isList()) throw errMsg; Outputs result; diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 4485ce39b..8f19b24d0 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -8,6 +8,64 @@ namespace nix { +struct SourcePathAdapter : AbstractPos +{ + std::string file; + + std::optional getSource() const override + { + return std::nullopt; + } + + void print(std::ostream & out) const override + { + out << fmt(ANSI_BLUE "at " ANSI_WARNING "%s:%s" ANSI_NORMAL ":", file, showErrPos()); + } +}; + +struct StringPosAdapter : AbstractPos +{ + void print(std::ostream & out) const override + { + out << fmt(ANSI_BLUE "at " ANSI_WARNING "«string»:%s" ANSI_NORMAL ":", showErrPos()); + } +}; + +struct StdinPosAdapter : AbstractPos +{ + void print(std::ostream & out) const override + { + out << fmt(ANSI_BLUE "at " ANSI_WARNING "«stdin»:%s" ANSI_NORMAL ":", showErrPos()); + } +}; + +Pos::operator std::shared_ptr() const +{ + if (!line) return nullptr; + + switch (origin) { + case foFile: { + auto pos = std::make_shared(); + pos->line = line; + pos->column = column; + pos->file = file; + return pos; + } + case foStdin: { + auto pos = std::make_shared(); + pos->line = line; + pos->column = column; + return pos; + } + case foString: + auto pos = std::make_shared(); + pos->line = line; + pos->column = column; + return pos; + } + assert(false); +} + /* Displaying abstract syntax trees. */ static void showString(std::ostream & str, std::string_view s) @@ -289,7 +347,6 @@ std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath) } - /* Computing levels/displacements for variables. */ void Expr::bindVars(EvalState & es, const std::shared_ptr & env) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index a9c3c3091..3fe75e09b 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -21,8 +21,14 @@ MakeError(TypeError, EvalError); MakeError(UndefinedVarError, Error); MakeError(MissingArgumentError, EvalError); -/* Position objects. */ +// FIXME: change this into a variant? +typedef enum { + foFile, + foStdin, + foString +} FileOrigin; +/* Position objects. */ struct Pos { std::string file; @@ -31,6 +37,8 @@ struct Pos uint32_t column; explicit operator bool() const { return line > 0; } + + operator std::shared_ptr() const; }; class PosIdx { diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f6317b9c7..eca9c5903 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -799,7 +799,7 @@ static void prim_addErrorContext(EvalState & state, const PosIdx pos, Value * * v = *args[1]; } catch (Error & e) { PathSet context; - e.addTrace(std::nullopt, state.coerceToString(pos, *args[0], context).toOwned()); + e.addTrace(nullptr, state.coerceToString(pos, *args[0], context).toOwned()); throw; } } diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index f4306ab91..e59acc007 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -129,7 +129,7 @@ public: log(*state, lvl, fs.s); } - void logEI(const ErrorInfo &ei) override + void logEI(const ErrorInfo & ei) override { auto state(state_.lock()); diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 9226c4e19..194f63bad 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -343,7 +343,7 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink) try { getFile(info->url, *decompressor); } catch (NoSuchBinaryCacheFile & e) { - throw SubstituteGone(e.info()); + throw SubstituteGone(std::move(e.info())); } decompressor->finish(); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3fff2385f..794f588dd 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -135,7 +135,7 @@ void DerivationGoal::killChild() void DerivationGoal::timedOut(Error && ex) { killChild(); - done(BuildResult::TimedOut, {}, ex); + done(BuildResult::TimedOut, {}, std::move(ex)); } @@ -958,7 +958,7 @@ void DerivationGoal::buildDone() BuildResult::PermanentFailure; } - done(st, {}, e); + done(st, {}, std::move(e)); return; } } @@ -1409,7 +1409,7 @@ void DerivationGoal::done( fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl; } - amDone(buildResult.success() ? ecSuccess : ecFailed, ex); + amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex)); } diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index bea7363db..e1b80165e 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -30,7 +30,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod if (ex) logError(i->ex->info()); else - ex = i->ex; + ex = std::move(i->ex); } if (i->exitCode != Goal::ecSuccess) { if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->drvPath); @@ -40,7 +40,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod if (failed.size() == 1 && ex) { ex->status = worker.exitStatus(); - throw *ex; + throw std::move(*ex); } else if (!failed.empty()) { if (ex) logError(ex->info()); throw Error(worker.exitStatus(), "build of %s failed", showPaths(failed)); @@ -109,7 +109,7 @@ void Store::ensurePath(const StorePath & path) if (goal->exitCode != Goal::ecSuccess) { if (goal->ex) { goal->ex->status = worker.exitStatus(); - throw *goal->ex; + throw std::move(*goal->ex); } else throw Error(worker.exitStatus(), "path '%s' does not exist and cannot be created", printStorePath(path)); } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index d1ec91ed5..5fce62d53 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -193,7 +193,7 @@ void LocalDerivationGoal::tryLocalBuild() { outputLocks.unlock(); buildUser.reset(); worker.permanentFailure = true; - done(BuildResult::InputRejected, {}, e); + done(BuildResult::InputRejected, {}, std::move(e)); return; } diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 8454ad7d2..4e06628a0 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -142,9 +142,9 @@ struct curlFileTransfer : public FileTransfer } template - void fail(const T & e) + void fail(T && e) { - failEx(std::make_exception_ptr(e)); + failEx(std::make_exception_ptr(std::move(e))); } LambdaSink finalSink; @@ -470,7 +470,7 @@ struct curlFileTransfer : public FileTransfer fileTransfer.enqueueItem(shared_from_this()); } else - fail(exc); + fail(std::move(exc)); } } }; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index bc36aef5d..6908406f8 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -447,7 +447,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, } catch (Error & e) { // Ugly backwards compatibility hack. if (e.msg().find("is not valid") != std::string::npos) - throw InvalidPath(e.info()); + throw InvalidPath(std::move(e.info())); throw; } if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) { diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 9172f67a6..52be6473d 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -9,9 +9,9 @@ namespace nix { const std::string nativeSystem = SYSTEM; -void BaseError::addTrace(std::optional e, hintformat hint) +void BaseError::addTrace(std::shared_ptr && e, hintformat hint) { - err.traces.push_front(Trace { .pos = e, .hint = hint }); + err.traces.push_front(Trace { .pos = std::move(e), .hint = hint }); } // c++ std::exception descendants must have a 'const char* what()' function. @@ -35,86 +35,41 @@ std::ostream & operator<<(std::ostream & os, const hintformat & hf) return os << hf.str(); } -std::string showErrPos(const ErrPos & errPos) +std::string AbstractPos::showErrPos() const { - if (errPos.line > 0) { - if (errPos.column > 0) { - return fmt("%d:%d", errPos.line, errPos.column); - } else { - return fmt("%d", errPos.line); - } - } - else { - return ""; + if (column > 0) { + return fmt("%d:%d", line, column); + } else { + return fmt("%d", line); } } -std::optional getCodeLines(const ErrPos & errPos) +std::optional AbstractPos::getCodeLines() const { - if (errPos.line <= 0) + if (line == 0) return std::nullopt; - if (errPos.origin == foFile) { - LinesOfCode loc; - try { - // FIXME: when running as the daemon, make sure we don't - // open a file to which the client doesn't have access. - AutoCloseFD fd = open(errPos.file.c_str(), O_RDONLY | O_CLOEXEC); - if (!fd) return {}; + if (auto source = getSource()) { - // count the newlines. - int count = 0; - std::string line; - int pl = errPos.line - 1; - do - { - line = readLine(fd.get()); - ++count; - if (count < pl) - ; - else if (count == pl) - loc.prevLineOfCode = line; - else if (count == pl + 1) - loc.errLineOfCode = line; - else if (count == pl + 2) { - loc.nextLineOfCode = line; - break; - } - } while (true); - return loc; - } - catch (EndOfFile & eof) { - if (loc.errLineOfCode.has_value()) - return loc; - else - return std::nullopt; - } - catch (std::exception & e) { - return std::nullopt; - } - } else { - std::istringstream iss(errPos.file); + std::istringstream iss(*source); // count the newlines. int count = 0; - std::string line; - int pl = errPos.line - 1; + std::string curLine; + int pl = line - 1; LinesOfCode loc; - do - { - std::getline(iss, line); + do { + std::getline(iss, curLine); ++count; if (count < pl) - { ; - } else if (count == pl) { - loc.prevLineOfCode = line; + loc.prevLineOfCode = curLine; } else if (count == pl + 1) { - loc.errLineOfCode = line; + loc.errLineOfCode = curLine; } else if (count == pl + 2) { - loc.nextLineOfCode = line; + loc.nextLineOfCode = curLine; break; } @@ -124,12 +79,14 @@ std::optional getCodeLines(const ErrPos & errPos) return loc; } + + return std::nullopt; } // print lines of code to the ostream, indicating the error column. void printCodeLines(std::ostream & out, const std::string & prefix, - const ErrPos & errPos, + const AbstractPos & errPos, const LinesOfCode & loc) { // previous line of code. @@ -176,28 +133,6 @@ void printCodeLines(std::ostream & out, } } -void printAtPos(const ErrPos & pos, std::ostream & out) -{ - if (pos) { - switch (pos.origin) { - case foFile: { - out << fmt(ANSI_BLUE "at " ANSI_WARNING "%s:%s" ANSI_NORMAL ":", pos.file, showErrPos(pos)); - break; - } - case foString: { - out << fmt(ANSI_BLUE "at " ANSI_WARNING "«string»:%s" ANSI_NORMAL ":", showErrPos(pos)); - break; - } - case foStdin: { - out << fmt(ANSI_BLUE "at " ANSI_WARNING "«stdin»:%s" ANSI_NORMAL ":", showErrPos(pos)); - break; - } - default: - throw Error("invalid FileOrigin in errPos"); - } - } -} - static std::string indent(std::string_view indentFirst, std::string_view indentRest, std::string_view s) { std::string res; @@ -264,18 +199,18 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s std::ostringstream oss; oss << einfo.msg << "\n"; - if (einfo.errPos.has_value() && *einfo.errPos) { + auto noSource = ANSI_ITALIC " (source not available)" ANSI_NORMAL "\n"; + + if (einfo.errPos) { oss << "\n"; - printAtPos(*einfo.errPos, oss); + einfo.errPos->print(oss); - auto loc = getCodeLines(*einfo.errPos); - - // lines of code. - if (loc.has_value()) { + if (auto loc = einfo.errPos->getCodeLines()) { oss << "\n"; printCodeLines(oss, "", *einfo.errPos, *loc); oss << "\n"; - } + } else + oss << noSource; } auto suggestions = einfo.suggestions.trim(); @@ -290,17 +225,16 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) { oss << "\n" << "… " << iter->hint.str() << "\n"; - if (iter->pos.has_value() && (*iter->pos)) { - auto pos = iter->pos.value(); + if (iter->pos) { oss << "\n"; - printAtPos(pos, oss); + iter->pos->print(oss); - auto loc = getCodeLines(pos); - if (loc.has_value()) { + if (auto loc = iter->pos->getCodeLines()) { oss << "\n"; - printCodeLines(oss, "", pos, *loc); + printCodeLines(oss, "", *iter->pos, *loc); oss << "\n"; - } + } else + oss << noSource; } } } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index a53e9802e..487c0c2ec 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -54,13 +54,6 @@ typedef enum { lvlVomit } Verbosity; -/* adjust Pos::origin bit width when adding stuff here */ -typedef enum { - foFile, - foStdin, - foString -} FileOrigin; - // the lines of code surrounding an error. struct LinesOfCode { std::optional prevLineOfCode; @@ -68,54 +61,37 @@ struct LinesOfCode { std::optional nextLineOfCode; }; -// ErrPos indicates the location of an error in a nix file. -struct ErrPos { - int line = 0; - int column = 0; - std::string file; - FileOrigin origin; +/* An abstract type that represents a location in a source file. */ +struct AbstractPos +{ + uint32_t line = 0; + uint32_t column = 0; - operator bool() const - { - return line != 0; - } + /* Return the contents of the source file. */ + virtual std::optional getSource() const + { return std::nullopt; }; - // convert from the Pos struct, found in libexpr. - template - ErrPos & operator=(const P & pos) - { - origin = pos.origin; - line = pos.line; - column = pos.column; - file = pos.file; - return *this; - } + virtual void print(std::ostream & out) const = 0; - template - ErrPos(const P & p) - { - *this = p; - } + std::string showErrPos() const; + + std::optional getCodeLines() const; }; -std::optional getCodeLines(const ErrPos & errPos); - void printCodeLines(std::ostream & out, const std::string & prefix, - const ErrPos & errPos, + const AbstractPos & errPos, const LinesOfCode & loc); -void printAtPos(const ErrPos & pos, std::ostream & out); - struct Trace { - std::optional pos; + std::shared_ptr pos; hintformat hint; }; struct ErrorInfo { Verbosity level; hintformat msg; - std::optional errPos; + std::shared_ptr errPos; std::list traces; Suggestions suggestions; @@ -177,12 +153,12 @@ public: const ErrorInfo & info() const { calcWhat(); return err; } template - void addTrace(std::optional e, const std::string & fs, const Args & ... args) + void addTrace(std::shared_ptr && e, const std::string & fs, const Args & ... args) { - addTrace(e, hintfmt(fs, args...)); + addTrace(std::move(e), hintfmt(fs, args...)); } - void addTrace(std::optional e, hintformat hint); + void addTrace(std::shared_ptr && e, hintformat hint); bool hasTrace() const { return !err.traces.empty(); } }; diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index cb2b15b41..a6b7da9f5 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -186,10 +186,11 @@ struct JSONLogger : Logger { json["msg"] = oss.str(); json["raw_msg"] = ei.msg.str(); - if (ei.errPos.has_value() && (*ei.errPos)) { + if (ei.errPos) { json["line"] = ei.errPos->line; json["column"] = ei.errPos->column; - json["file"] = ei.errPos->file; + //json["file"] = ei.errPos->file; + json["file"] = nullptr; } else { json["line"] = nullptr; json["column"] = nullptr; @@ -201,10 +202,11 @@ struct JSONLogger : Logger { for (auto iter = ei.traces.rbegin(); iter != ei.traces.rend(); ++iter) { nlohmann::json stackFrame; stackFrame["raw_msg"] = iter->hint.str(); - if (iter->pos.has_value() && (*iter->pos)) { + if (iter->pos) { stackFrame["line"] = iter->pos->line; stackFrame["column"] = iter->pos->column; - stackFrame["file"] = iter->pos->file; + //stackFrame["file"] = iter->pos->file; + stackFrame["file"] = nullptr; } traces.push_back(stackFrame); } diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 6f81b92de..44eb1e3a9 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -82,7 +82,7 @@ public: log(lvlInfo, fs); } - virtual void logEI(const ErrorInfo &ei) = 0; + virtual void logEI(const ErrorInfo & ei) = 0; void logEI(Verbosity lvl, ErrorInfo ei) { diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 8ff904583..d8682f18b 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -353,7 +353,7 @@ Sink & operator << (Sink & sink, const StringSet & s) Sink & operator << (Sink & sink, const Error & ex) { - auto info = ex.info(); + auto & info = ex.info(); sink << "Error" << info.level diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index fca2106c2..a6ac7f799 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -649,7 +649,7 @@ static void upgradeDerivations(Globals & globals, } else newElems.push_back(i); } catch (Error & e) { - e.addTrace(std::nullopt, "while trying to find an upgrade for '%s'", i.queryName()); + e.addTrace(nullptr, "while trying to find an upgrade for '%s'", i.queryName()); throw; } } @@ -956,7 +956,7 @@ static void queryJSON(Globals & globals, std::vector & elems, bool prin } catch (AssertionError & e) { printMsg(lvlTalkative, "skipping derivation named '%1%' which gives an assertion failure", i.queryName()); } catch (Error & e) { - e.addTrace(std::nullopt, "while querying the derivation named '%1%'", i.queryName()); + e.addTrace(nullptr, "while querying the derivation named '%1%'", i.queryName()); throw; } } @@ -1259,7 +1259,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) } catch (AssertionError & e) { printMsg(lvlTalkative, "skipping derivation named '%1%' which gives an assertion failure", i.queryName()); } catch (Error & e) { - e.addTrace(std::nullopt, "while querying the derivation named '%1%'", i.queryName()); + e.addTrace(nullptr, "while querying the derivation named '%1%'", i.queryName()); throw; } } diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index 940923d3b..c527fdb0a 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -257,7 +257,7 @@ static void daemonLoop() } catch (Interrupted & e) { return; } catch (Error & error) { - ErrorInfo ei = error.info(); + auto ei = error.info(); // FIXME: add to trace? ei.msg = hintfmt("error processing connection: %1%", ei.msg.str()); logError(ei);