Introduce AbstractPos

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
+    auto pos = dt.pos
-        ? *dt.pos
+        ? dt.pos
-        : positions[dt.expr.getPos() ? dt.expr.getPos() : noPos];
+        : (std::shared_ptr<AbstractPos>) positions[dt.expr.getPos() ? dt.expr.getPos() : noPos];
 
     if (pos) {
-        printAtPos(pos, out);
+        pos->print(out);
 
-        auto loc = getCodeLines(pos);
+        if (auto loc = pos->getCodeLines()) {;
-        if (loc.has_value()) {
             out << "\n";
-            printCodeLines(out, "", pos, *loc);
+            printCodeLines(out, "", *pos, *loc);
             out << "\n";
         }
     }

src/libexpr/eval.cc

@@ -775,7 +775,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr &
         ? std::make_unique<DebugTraceStacker>(
             *this,
             DebugTrace {
-                .pos = error->info().errPos ? *error->info().errPos : positions[expr.getPos()],
+                .pos = error->info().errPos ? error->info().errPos : (std::shared_ptr<AbstractPos>) 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<DebugTraceStacker> makeDebugTraceStacker(
     EvalState & state,
     Expr & expr,
     Env & env,
-    std::optional<ErrPos> pos,
+    std::shared_ptr<AbstractPos> && pos,
     const char * s,
     const std::string & s2)
 {
     return std::make_unique<DebugTraceStacker>(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<AbstractPos>) positions[e->getPos()] : nullptr,
                 "while evaluating the file '%1%':", resolvedPath.to_string())
             : nullptr;
 

src/libexpr/eval.hh

@@ -76,7 +76,7 @@ struct RegexCache;
 std::shared_ptr<RegexCache> makeRegexCache();
 
 struct DebugTrace {
-    std::optional<ErrPos> pos;
+    std::shared_ptr<AbstractPos> pos;
     const Expr & expr;
     const Env & env;
     hintformat hint;

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;

src/libexpr/nixexpr.cc

@@ -8,6 +8,64 @@
 
 namespace nix {
 
+struct SourcePathAdapter : AbstractPos
+{
+    std::string file;
+
+    std::optional<std::string> 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<AbstractPos>() const
+{
+    if (!line) return nullptr;
+
+    switch (origin) {
+    case foFile: {
+        auto pos = std::make_shared<SourcePathAdapter>();
+        pos->line = line;
+        pos->column = column;
+        pos->file = file;
+        return pos;
+    }
+    case foStdin: {
+        auto pos = std::make_shared<StdinPosAdapter>();
+        pos->line = line;
+        pos->column = column;
+        return pos;
+    }
+    case foString:
+        auto pos = std::make_shared<StringPosAdapter>();
+        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<const StaticEnv> & env)

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<AbstractPos>() const;
 };
 
 class PosIdx {

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;
     }
 }

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());
 

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();

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));
 }
 
 

src/libstore/build/entry-points.cc

@@ -30,7 +30,7 @@ void Store::buildPaths(const std::vector<DerivedPath> & 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<DerivationGoal *>(i.get())) failed.insert(i2->drvPath);
@@ -40,7 +40,7 @@ void Store::buildPaths(const std::vector<DerivedPath> & 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));
     }

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;
     }
 

src/libstore/filetransfer.cc

@@ -142,9 +142,9 @@ struct curlFileTransfer : public FileTransfer
         }
 
         template<class T>
-        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));
             }
         }
     };

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) {

src/libutil/error.cc

@@ -9,9 +9,9 @@ namespace nix {
 
 const std::string nativeSystem = SYSTEM;
 
-void BaseError::addTrace(std::optional<ErrPos> e, hintformat hint)
+void BaseError::addTrace(std::shared_ptr<AbstractPos> && 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 (column > 0) {
-        if (errPos.column > 0) {
+        return fmt("%d:%d", line, column);
-            return fmt("%d:%d", errPos.line, errPos.column);
+    } else {
-        } else {
+        return fmt("%d", line);
-            return fmt("%d", errPos.line);
-        }
-    }
-    else {
-        return "";
     }
 }
 
-std::optional<LinesOfCode> getCodeLines(const ErrPos & errPos)
+std::optional<LinesOfCode> AbstractPos::getCodeLines() const
 {
-    if (errPos.line <= 0)
+    if (line == 0)
         return std::nullopt;
 
-    if (errPos.origin == foFile) {
+    if (auto source = getSource()) {
-        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 {};
 
-            // count the newlines.
+        std::istringstream iss(*source);
-            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);
         // count the newlines.
         int count = 0;
-        std::string line;
+        std::string curLine;
-        int pl = errPos.line - 1;
+        int pl = line - 1;
 
         LinesOfCode loc;
 
-        do
+        do {
-        {
+            std::getline(iss, curLine);
-            std::getline(iss, line);
             ++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<LinesOfCode> 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);
+        if (auto loc = einfo.errPos->getCodeLines()) {
-
-        // lines of code.
-        if (loc.has_value()) {
             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)) {
+            if (iter->pos) {
-                auto pos = iter->pos.value();
                 oss << "\n";
-                printAtPos(pos, oss);
+                iter->pos->print(oss);
 
-                auto loc = getCodeLines(pos);
+                if (auto loc = iter->pos->getCodeLines()) {
-                if (loc.has_value()) {
                     oss << "\n";
-                    printCodeLines(oss, "", pos, *loc);
+                    printCodeLines(oss, "", *iter->pos, *loc);
                     oss << "\n";
-                }
+                } else
+                    oss << noSource;
             }
         }
     }

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<std::string> prevLineOfCode;
@@ -68,54 +61,37 @@ struct LinesOfCode {
     std::optional<std::string> nextLineOfCode;
 };
 
-// ErrPos indicates the location of an error in a nix file.
+/* An abstract type that represents a location in a source file. */
-struct ErrPos {
+struct AbstractPos
-    int line = 0;
+{
-    int column = 0;
+    uint32_t line = 0;
-    std::string file;
+    uint32_t column = 0;
-    FileOrigin origin;
 
-    operator bool() const
+    /* Return the contents of the source file. */
-    {
+    virtual std::optional<std::string> getSource() const
-        return line != 0;
+    { return std::nullopt; };
-    }
 
-    // convert from the Pos struct, found in libexpr.
+    virtual void print(std::ostream & out) const = 0;
-    template <class P>
-    ErrPos & operator=(const P & pos)
-    {
-        origin = pos.origin;
-        line = pos.line;
-        column = pos.column;
-        file = pos.file;
-        return *this;
-    }
 
-    template <class P>
+    std::string showErrPos() const;
-    ErrPos(const P & p)
+
-    {
+    std::optional<LinesOfCode> getCodeLines() const;
-        *this = p;
-    }
 };
 
-std::optional<LinesOfCode> 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<ErrPos> pos;
+    std::shared_ptr<AbstractPos> pos;
     hintformat hint;
 };
 
 struct ErrorInfo {
     Verbosity level;
     hintformat msg;
-    std::optional<ErrPos> errPos;
+    std::shared_ptr<AbstractPos> errPos;
     std::list<Trace> traces;
 
     Suggestions suggestions;
@@ -177,12 +153,12 @@ public:
     const ErrorInfo & info() const { calcWhat(); return err; }
 
     template<typename... Args>
-    void addTrace(std::optional<ErrPos> e, const std::string & fs, const Args & ... args)
+    void addTrace(std::shared_ptr<AbstractPos> && e, const std::string & fs, const Args & ... args)
     {
-        addTrace(e, hintfmt(fs, args...));
+        addTrace(std::move(e), hintfmt(fs, args...));
     }
 
-    void addTrace(std::optional<ErrPos> e, hintformat hint);
+    void addTrace(std::shared_ptr<AbstractPos> && e, hintformat hint);
 
     bool hasTrace() const { return !err.traces.empty(); }
 };

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);
             }

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)
     {

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

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<DrvInfo> & 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;
         }
     }

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);