Introduce AbstractPos

This makes the position object used in exceptions abstract, with a
method getSource() to get the source code of the file in which the
error originated. This is needed for lazy trees because source files
don't necessarily exist in the filesystem, and we don't want to make
libutil depend on the InputAccessor type in libfetcher.

src/libcmd/repl.cc

@@ -215,17 +215,15 @@ 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);
+        out << pos;
-
+        if (auto loc = pos->getCodeLines()) {
-        auto loc = getCodeLines(pos);
-        if (loc.has_value()) {
             out << "\n";
-            printCodeLines(out, "", pos, *loc);
+            printCodeLines(out, "", *pos, *loc);
             out << "\n";
         }
     }
@@ -589,15 +587,17 @@ bool NixRepl::processLine(std::string line)
         Value v;
         evalString(arg, v);
 
-        const auto [file, line] = [&] () -> std::pair<std::string, uint32_t> {
+        const auto [path, line] = [&] () -> std::pair<Path, uint32_t> {
             if (v.type() == nPath || v.type() == nString) {
                 PathSet context;
-                auto filename = state->coerceToString(noPos, v, context).toOwned();
+                auto path = state->coerceToPath(noPos, v, context);
-                state->symbols.create(filename);
+                return {path, 0};
-                return {filename, 0};
             } else if (v.isLambda()) {
                 auto pos = state->positions[v.lambda.fun->pos];
-                return {pos.file, pos.line};
+                if (auto path = std::get_if<Path>(&pos.origin))
+                    return {*path, pos.line};
+                else
+                    throw Error("'%s' cannot be shown in an editor", pos);
             } else {
                 // assume it's a derivation
                 return findPackageFilename(*state, v, arg);
@@ -605,7 +605,7 @@ bool NixRepl::processLine(std::string line)
         }();
 
         // Open in EDITOR
-        auto args = editorFor(file, line);
+        auto args = editorFor(path, line);
         auto editor = args.front();
         args.pop_front();
 

src/libexpr/eval.cc

@@ -823,7 +823,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,
@@ -1012,7 +1012,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
@@ -1024,13 +1024,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),
@@ -1136,9 +1136,9 @@ void EvalState::mkThunk_(Value & v, Expr * expr)
 void EvalState::mkPos(Value & v, PosIdx p)
 {
     auto pos = positions[p];
-    if (!pos.file.empty()) {
+    if (auto path = std::get_if<Path>(&pos.origin)) {
         auto attrs = buildBindings(3);
-        attrs.alloc(sFile).mkString(pos.file);
+        attrs.alloc(sFile).mkString(*path);
         attrs.alloc(sLine).mkInt(pos.line);
         attrs.alloc(sColumn).mkInt(pos.column);
         v.mkAttrs(attrs);
@@ -1246,7 +1246,7 @@ void EvalState::cacheFile(
                 *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)
             : nullptr;
 
@@ -1517,10 +1517,13 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
         state.forceValue(*vAttrs, (pos2 ? pos2 : this->pos ) );
 
     } catch (Error & e) {
-        auto pos2r = state.positions[pos2];
+        if (pos2) {
-        if (pos2 && pos2r.file != state.derivationNixPath)
+            auto pos2r = state.positions[pos2];
-            state.addErrorTrace(e, pos2, "while evaluating the attribute '%1%'",
+            auto origin = std::get_if<Path>(&pos2r.origin);
-                showAttrPath(state, env, attrPath));
+            if (!(origin && *origin == state.derivationNixPath))
+                state.addErrorTrace(e, pos2, "while evaluating the attribute '%1%'",
+                    showAttrPath(state, env, attrPath));
+        }
         throw;
     }
 
@@ -2497,7 +2500,8 @@ void EvalState::printStats()
                     else
                         obj["name"] = nullptr;
                     if (auto pos = positions[fun->pos]) {
-                        obj["file"] = (std::string_view) pos.file;
+                        if (auto path = std::get_if<Path>(&pos.origin))
+                            obj["file"] = *path;
                         obj["line"] = pos.line;
                         obj["column"] = pos.column;
                     }
@@ -2511,7 +2515,8 @@ void EvalState::printStats()
                 for (auto & i : attrSelects) {
                     json obj = json::object();
                     if (auto pos = positions[i.first]) {
-                        obj["file"] = (const std::string &) pos.file;
+                        if (auto path = std::get_if<Path>(&pos.origin))
+                            obj["file"] = *path;
                         obj["line"] = pos.line;
                         obj["column"] = pos.column;
                     }

src/libexpr/eval.hh

@@ -78,7 +78,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;
@@ -457,8 +457,12 @@ private:
     friend struct ExprAttrs;
     friend struct ExprLet;
 
-    Expr * parse(char * text, size_t length, FileOrigin origin, const PathView path,
+    Expr * parse(
-        const PathView basePath, std::shared_ptr<StaticEnv> & staticEnv);
+        char * text,
+        size_t length,
+        Pos::Origin origin,
+        Path basePath,
+        std::shared_ptr<StaticEnv> & staticEnv);
 
 public:
 

src/libexpr/flake/flake.cc

@@ -220,7 +220,7 @@ static Flake getFlake(
     Value vInfo;
     state.evalFile(flakeFile, vInfo, true); // FIXME: symlink attack
 
-    expectType(state, nAttrs, vInfo, state.positions.add({flakeFile, foFile}, 0, 0));
+    expectType(state, nAttrs, vInfo, state.positions.add({flakeFile}, 1, 1));
 
     if (auto description = vInfo.attrs->get(state.sDescription)) {
         expectType(state, nString, *description->value, description->pos);

src/libexpr/nixexpr.cc

@@ -8,6 +8,65 @@
 
 namespace nix {
 
+struct SourcePathAdapter : AbstractPos
+{
+    Path path;
+
+    SourcePathAdapter(Path path)
+        : path(std::move(path))
+    {
+    }
+
+    std::optional<std::string> getSource() const override
+    {
+        try {
+            return readFile(path);
+        } catch (Error &) {
+            return std::nullopt;
+        }
+    }
+
+    void print(std::ostream & out) const override
+    {
+        out << path;
+    }
+};
+
+struct StringPosAdapter : AbstractPos
+{
+    void print(std::ostream & out) const override
+    {
+        out << "«string»";
+    }
+};
+
+struct StdinPosAdapter : AbstractPos
+{
+    void print(std::ostream & out) const override
+    {
+        out << "«stdin»";
+    }
+};
+
+Pos::operator std::shared_ptr<AbstractPos>() const
+{
+    std::shared_ptr<AbstractPos> pos;
+
+    if (auto path = std::get_if<Path>(&origin))
+        pos = std::make_shared<SourcePathAdapter>(*path);
+    else if (std::get_if<stdin_tag>(&origin))
+        pos = std::make_shared<StdinPosAdapter>();
+    else if (std::get_if<string_tag>(&origin))
+        pos = std::make_shared<StringPosAdapter>();
+
+    if (pos) {
+        pos->line = line;
+        pos->column = column;
+    }
+
+    return pos;
+}
+
 /* Displaying abstract syntax trees. */
 
 static void showString(std::ostream & str, std::string_view s)
@@ -248,24 +307,10 @@ void ExprPos::show(const SymbolTable & symbols, std::ostream & str) const
 
 std::ostream & operator << (std::ostream & str, const Pos & pos)
 {
-    if (!pos)
+    if (auto pos2 = (std::shared_ptr<AbstractPos>) pos) {
+        str << *pos2;
+    } else
         str << "undefined position";
-    else
-    {
-        auto f = format(ANSI_BOLD "%1%" ANSI_NORMAL ":%2%:%3%");
-        switch (pos.origin) {
-            case foFile:
-                f % (const std::string &) pos.file;
-                break;
-            case foStdin:
-            case foString:
-                f % "(string)";
-                break;
-            default:
-                throw Error("unhandled Pos origin!");
-        }
-        str << (f % pos.line % pos.column).str();
-    }
 
     return str;
 }

src/libexpr/nixexpr.hh

@@ -23,15 +23,21 @@ MakeError(MissingArgumentError, EvalError);
 MakeError(RestrictedPathError, Error);
 
 /* Position objects. */
-
 struct Pos
 {
-    std::string file;
-    FileOrigin origin;
     uint32_t line;
     uint32_t column;
 
+    struct stdin_tag {};
+    struct string_tag {};
+
+    typedef std::variant<stdin_tag, string_tag, Path> Origin;
+
+    Origin origin;
+
     explicit operator bool() const { return line > 0; }
+
+    operator std::shared_ptr<AbstractPos>() const;
 };
 
 class PosIdx {
@@ -47,7 +53,11 @@ public:
 
     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; }
+
+    bool operator !=(const PosIdx other) const { return id != other.id; }
 };
 
 class PosTable
@@ -61,13 +71,13 @@ public:
         // current origins.back() can be reused or not.
         mutable uint32_t idx = std::numeric_limits<uint32_t>::max();
 
-        explicit Origin(uint32_t idx): idx(idx), file{}, origin{} {}
+        // Used for searching in PosTable::[].
+        explicit Origin(uint32_t idx): idx(idx), origin{Pos::stdin_tag()} {}
 
     public:
-        const std::string file;
+        const Pos::Origin origin;
-        const FileOrigin origin;
 
-        Origin(std::string file, FileOrigin origin): file(std::move(file)), origin(origin) {}
+        Origin(Pos::Origin origin): origin(origin) {}
     };
 
     struct Offset {
@@ -107,7 +117,7 @@ public:
             [] (const auto & a, const auto & b) { return a.idx < b.idx; });
         const auto origin = *std::prev(pastOrigin);
         const auto offset = offsets[idx];
-        return {origin.file, origin.origin, offset.line, offset.column};
+        return {offset.line, offset.column, origin.origin};
     }
 };
 

src/libexpr/parser.y

@@ -34,11 +34,6 @@ namespace nix {
         Path basePath;
         PosTable::Origin origin;
         std::optional<ErrorInfo> error;
-        ParseData(EvalState & state, PosTable::Origin origin)
-            : state(state)
-            , symbols(state.symbols)
-            , origin(std::move(origin))
-            { };
     };
 
     struct ParserFormals {
@@ -649,24 +644,20 @@ formal
 namespace nix {
 
 
-Expr * EvalState::parse(char * text, size_t length, FileOrigin origin,
+Expr * EvalState::parse(
-    const PathView path, const PathView basePath, std::shared_ptr<StaticEnv> & staticEnv)
+    char * text,
+    size_t length,
+    Pos::Origin origin,
+    Path basePath,
+    std::shared_ptr<StaticEnv> & staticEnv)
 {
     yyscan_t scanner;
-    std::string file;
+    ParseData data {
-    switch (origin) {
+        .state = *this,
-        case foFile:
+        .symbols = symbols,
-            file = path;
+        .basePath = std::move(basePath),
-            break;
+        .origin = {origin},
-        case foStdin:
+    };
-        case foString:
-            file = text;
-            break;
-        default:
-            assert(false);
-    }
-    ParseData data(*this, {file, origin});
-    data.basePath = basePath;
 
     yylex_init(&scanner);
     yy_scan_buffer(text, length, scanner);
@@ -718,14 +709,14 @@ Expr * EvalState::parseExprFromFile(const Path & path, std::shared_ptr<StaticEnv
     auto buffer = readFile(path);
     // readFile should have left some extra space for terminators
     buffer.append("\0\0", 2);
-    return parse(buffer.data(), buffer.size(), foFile, path, dirOf(path), staticEnv);
+    return parse(buffer.data(), buffer.size(), path, dirOf(path), staticEnv);
 }
 
 
 Expr * EvalState::parseExprFromString(std::string s, const Path & basePath, std::shared_ptr<StaticEnv> & staticEnv)
 {
     s.append("\0\0", 2);
-    return parse(s.data(), s.size(), foString, "", basePath, staticEnv);
+    return parse(s.data(), s.size(), Pos::string_tag(), basePath, staticEnv);
 }
 
 
@@ -741,7 +732,7 @@ Expr * EvalState::parseStdin()
     auto buffer = drainFD(0);
     // drainFD should have left some extra space for terminators
     buffer.append("\0\0", 2);
-    return parse(buffer.data(), buffer.size(), foStdin, "", absPath("."), staticBaseEnv);
+    return parse(buffer.data(), buffer.size(), Pos::stdin_tag(), absPath("."), staticBaseEnv);
 }
 
 

src/libexpr/primops.cc

@@ -368,8 +368,7 @@ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v)
     auto output = runProgram(program, true, commandArgs);
     Expr * parsed;
     try {
-        auto base = state.positions[pos];
+        parsed = state.parseExprFromString(std::move(output), "/");
-        parsed = state.parseExprFromString(std::move(output), base.file);
     } catch (Error & e) {
         e.addTrace(state.positions[pos], "While parsing the output from '%1%'", program);
         throw;
@@ -798,7 +797,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;
     }
 }
@@ -4018,7 +4017,7 @@ void EvalState::createBaseEnv()
         // the parser needs two NUL bytes as terminators; one of them
         // is implied by being a C string.
         "\0";
-    eval(parse(code, sizeof(code), foFile, derivationNixPath, "/", staticBaseEnv), *vDerivation);
+    eval(parse(code, sizeof(code), derivationNixPath, "/", staticBaseEnv), *vDerivation);
 }
 
 

src/libexpr/tests/libexprtests.hh

@@ -123,7 +123,7 @@ namespace nix {
 
     MATCHER_P(IsAttrsOfSize, n, fmt("Is a set of size [%1%]", n)) {
         if (arg.type() != nAttrs) {
-            *result_listener << "Expexted set got " << arg.type();
+            *result_listener << "Expected set got " << arg.type();
             return false;
         } else if (arg.attrs->size() != (size_t)n) {
             *result_listener << "Expected a set with " << n << " attributes but got " << arg.attrs->size();

src/libexpr/tests/primops.cc

@@ -151,20 +151,7 @@ namespace nix {
         // The `y` attribute is at position
         const char* expr = "builtins.unsafeGetAttrPos \"y\" { y = \"x\"; }";
         auto v = eval(expr);
-        ASSERT_THAT(v, IsAttrsOfSize(3));
+        ASSERT_THAT(v, IsNull());
-
-        auto file = v.attrs->find(createSymbol("file"));
-        ASSERT_NE(file, nullptr);
-        // FIXME: The file when running these tests is the input string?!?
-        ASSERT_THAT(*file->value, IsStringEq(expr));
-
-        auto line = v.attrs->find(createSymbol("line"));
-        ASSERT_NE(line, nullptr);
-        ASSERT_THAT(*line->value, IsIntEq(1));
-
-        auto column = v.attrs->find(createSymbol("column"));
-        ASSERT_NE(column, nullptr);
-        ASSERT_THAT(*column->value, IsIntEq(33));
     }
 
     TEST_F(PrimOpTest, hasAttr) {

src/libexpr/value-to-xml.cc

@@ -24,7 +24,8 @@ static void printValueAsXML(EvalState & state, bool strict, bool location,
 
 static void posToXML(EvalState & state, XMLAttrs & xmlAttrs, const Pos & pos)
 {
-    xmlAttrs["path"] = pos.file;
+    if (auto path = std::get_if<Path>(&pos.origin))
+        xmlAttrs["path"] = *path;
     xmlAttrs["line"] = (format("%1%") % pos.line).str();
     xmlAttrs["column"] = (format("%1%") % pos.column).str();
 }

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.
@@ -30,91 +30,46 @@ const std::string & BaseError::calcWhat() const
 
 std::optional<std::string> ErrorInfo::programName = std::nullopt;
 
-std::ostream & operator<<(std::ostream & os, const hintformat & hf)
+std::ostream & operator <<(std::ostream & os, const hintformat & hf)
 {
     return os << hf.str();
 }
 
-std::string showErrPos(const ErrPos & errPos)
+std::ostream & operator <<(std::ostream & str, const AbstractPos & pos)
 {
-    if (errPos.line > 0) {
+    pos.print(str);
-        if (errPos.column > 0) {
+    str << ":" << pos.line;
-            return fmt("%d:%d", errPos.line, errPos.column);
+    if (pos.column > 0)
-        } else {
+        str << ":" << pos.column;
-            return fmt("%d", errPos.line);
+    return str;
-        }
-    }
-    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;
@@ -263,22 +198,22 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s
 
     std::ostringstream oss;
 
+    auto noSource = ANSI_ITALIC " (source not available)" ANSI_NORMAL "\n";
+
     // traces
     if (showTrace && !einfo.traces.empty()) {
         for (const auto & trace : einfo.traces) {
             oss << "\n" << "… " << trace.hint.str() << "\n";
 
-            if (trace.pos.has_value() && (*trace.pos)) {
+            if (trace.pos) {
-                auto pos = trace.pos.value();
+                oss << "\n" << ANSI_BLUE << "at " ANSI_WARNING << *trace.pos << ANSI_NORMAL << ":";
-                oss << "\n";
-                printAtPos(pos, oss);
 
-                auto loc = getCodeLines(pos);
+                if (auto loc = trace.pos->getCodeLines()) {
-                if (loc.has_value()) {
                     oss << "\n";
-                    printCodeLines(oss, "", pos, *loc);
+                    printCodeLines(oss, "", *trace.pos, *loc);
                     oss << "\n";
-                }
+                } else
+                    oss << noSource;
             }
         }
         oss << "\n" << prefix;
@@ -286,22 +221,19 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s
 
     oss << einfo.msg << "\n";
 
-    if (einfo.errPos.has_value() && *einfo.errPos) {
+    if (einfo.errPos) {
-        oss << "\n";
+        oss << "\n" << ANSI_BLUE << "at " ANSI_WARNING << *einfo.errPos << ANSI_NORMAL << ":";
-        printAtPos(*einfo.errPos, 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();
-    if (! suggestions.suggestions.empty()){
+    if (!suggestions.suggestions.empty()) {
         oss << "Did you mean " <<
             suggestions.trim() <<
             "?" << std::endl;

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::optional<LinesOfCode> getCodeLines() const;
-    ErrPos(const P & p)
-    {
-        *this = p;
-    }
 };
 
-std::optional<LinesOfCode> getCodeLines(const ErrPos & errPos);
+std::ostream & operator << (std::ostream & str, const AbstractPos & pos);
 
 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

@@ -131,6 +131,21 @@ Activity::Activity(Logger & logger, Verbosity lvl, ActivityType type,
     logger.startActivity(id, lvl, type, s, fields, parent);
 }
 
+void to_json(nlohmann::json & json, std::shared_ptr<AbstractPos> pos)
+{
+    if (pos) {
+        json["line"] = pos->line;
+        json["column"] = pos->column;
+        std::ostringstream str;
+        pos->print(str);
+        json["file"] = str.str();
+    } else {
+        json["line"] = nullptr;
+        json["column"] = nullptr;
+        json["file"] = nullptr;
+    }
+}
+
 struct JSONLogger : Logger {
     Logger & prevLogger;
 
@@ -177,27 +192,14 @@ struct JSONLogger : Logger {
         json["level"] = ei.level;
         json["msg"] = oss.str();
         json["raw_msg"] = ei.msg.str();
-
+        to_json(json, ei.errPos);
-        if (ei.errPos.has_value() && (*ei.errPos)) {
-            json["line"] = ei.errPos->line;
-            json["column"] = ei.errPos->column;
-            json["file"] = ei.errPos->file;
-        } else {
-            json["line"] = nullptr;
-            json["column"] = nullptr;
-            json["file"] = nullptr;
-        }
 
         if (loggerSettings.showTrace.get() && !ei.traces.empty()) {
             nlohmann::json traces = nlohmann::json::array();
             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)) {
+                to_json(stackFrame, iter->pos);
-                    stackFrame["line"] = iter->pos->line;
-                    stackFrame["column"] = iter->pos->column;
-                    stackFrame["file"] = iter->pos->file;
-                }
                 traces.push_back(stackFrame);
             }
 

src/nix-env/nix-env.cc

@@ -647,7 +647,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;
             }
         }
@@ -958,7 +958,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;
         }
     }
@@ -1262,7 +1262,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;
         }
     }

tests/function-trace.sh

@@ -32,40 +32,40 @@ expect_trace() {
 
 # failure inside a tryEval
 expect_trace 'builtins.tryEval (throw "example")' "
-function-trace entered (string):1:1 at
+function-trace entered «string»:1:1 at
-function-trace entered (string):1:19 at
+function-trace entered «string»:1:19 at
-function-trace exited (string):1:19 at
+function-trace exited «string»:1:19 at
-function-trace exited (string):1:1 at
+function-trace exited «string»:1:1 at
 "
 
 # Missing argument to a formal function
 expect_trace '({ x }: x) { }' "
-function-trace entered (string):1:1 at
+function-trace entered «string»:1:1 at
-function-trace exited (string):1:1 at
+function-trace exited «string»:1:1 at
 "
 
 # Too many arguments to a formal function
 expect_trace '({ x }: x) { x = "x"; y = "y"; }' "
-function-trace entered (string):1:1 at
+function-trace entered «string»:1:1 at
-function-trace exited (string):1:1 at
+function-trace exited «string»:1:1 at
 "
 
 # Not enough arguments to a lambda
 expect_trace '(x: y: x + y) 1' "
-function-trace entered (string):1:1 at
+function-trace entered «string»:1:1 at
-function-trace exited (string):1:1 at
+function-trace exited «string»:1:1 at
 "
 
 # Too many arguments to a lambda
 expect_trace '(x: x) 1 2' "
-function-trace entered (string):1:1 at
+function-trace entered «string»:1:1 at
-function-trace exited (string):1:1 at
+function-trace exited «string»:1:1 at
 "
 
 # Not a function
 expect_trace '1 2' "
-function-trace entered (string):1:1 at
+function-trace entered «string»:1:1 at
-function-trace exited (string):1:1 at
+function-trace exited «string»:1:1 at
 "
 
 set -e