Progress towards issue NixOS#7865

src/libexpr/eval.hh

@@ -91,15 +91,18 @@ class ErrorBuilder
     private:
         EvalState & state;
         ErrorInfo info;
+        hintformat message;
 
-        ErrorBuilder(EvalState & s, ErrorInfo && i): state(s), info(i) { }
+        ErrorBuilder(EvalState & s, hintformat && message)
+            : state(s), message(message)
+        { }
 
     public:
         template<typename... Args>
         [[nodiscard, gnu::noinline]]
         static ErrorBuilder * create(EvalState & s, const Args & ... args)
         {
-            return new ErrorBuilder(s, ErrorInfo { .msg = hintfmt(args...) });
+            return new ErrorBuilder(s, hintfmt(args...));
         }
 
         [[nodiscard, gnu::noinline]]
@@ -659,7 +662,7 @@ template<class ErrorType>
 void ErrorBuilder::debugThrow()
 {
     // NOTE: We always use the -LastTrace version as we push the new trace in withFrame()
-    state.debugThrowLastTrace(ErrorType(info));
+    state.debugThrowLastTrace(ErrorType { info, message });
 }
 
 }

src/libexpr/parser.y

@@ -74,19 +74,23 @@ namespace nix {
 
 static void dupAttr(const EvalState & state, const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos)
 {
-    throw ParseError({
-         .msg = hintfmt("attribute '%1%' already defined at %2%",
-             showAttrPath(state.symbols, attrPath), state.positions[prevPos]),
-         .errPos = state.positions[pos]
-    });
+    throw ParseError(
+        {
+             .errPos = state.positions[pos]
+        },
+        hintfmt("attribute '%1%' already defined at %2%",
+            showAttrPath(state.symbols, attrPath), state.positions[prevPos]),
+    );
 }
 
 static void dupAttr(const EvalState & state, Symbol attr, const PosIdx pos, const PosIdx prevPos)
 {
-    throw ParseError({
-        .msg = hintfmt("attribute '%1%' already defined at %2%", state.symbols[attr], state.positions[prevPos]),
-        .errPos = state.positions[pos]
-    });
+    throw ParseError(
+        {
+            .errPos = state.positions[pos]
+        },
+        hintfmt("attribute '%1%' already defined at %2%", state.symbols[attr], state.positions[prevPos]),
+    );
 }
 
 

src/libexpr/primops/fetchTree.cc

@@ -144,10 +144,12 @@ 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.debugThrowLastTrace(EvalError(
+                    {
+                        .errPos = state.positions[pos]
+                    },
+                    hintfmt("attribute 'name' isn’t supported in call to 'fetchTree'"),
+                ));
 
         input = fetchers::Input::fromAttrs(std::move(attrs));
     } else {
@@ -463,10 +465,10 @@ static RegisterPrimOp primop_fetchGit({
           builtins.fetchGit ./work-dir
           ```
 
-	  If the URL points to a local directory, and no `ref` or `rev` is
-	  given, `fetchGit` will use the current content of the checked-out
-	  files, even if they are not committed or added to Git's index. It will
-	  only consider files added to the Git repository, as listed by `git ls-files`.
+      If the URL points to a local directory, and no `ref` or `rev` is
+      given, `fetchGit` will use the current content of the checked-out
+      files, even if they are not committed or added to Git's index. It will
+      only consider files added to the Git repository, as listed by `git ls-files`.
     )",
     .fun = prim_fetchGit,
 });

src/libexpr/value/context.hh

@@ -21,7 +21,7 @@ public:
     {
         raw = raw_;
         auto hf = hintfmt(args...);
-        err.msg = hintfmt("Bad String Context element: %1%: %2%", normaltxt(hf.str()), raw);
+        message = hintfmt("Bad String Context element: %1%: %2%", normaltxt(hf.str()), raw);
     }
 };
 

src/libstore/sqlite.cc

@@ -12,7 +12,7 @@ SQLiteError::SQLiteError(const char *path, const char *errMsg, int errNo, int ex
   : Error(""), path(path), errMsg(errMsg), errNo(errNo), extendedErrNo(extendedErrNo), offset(offset)
 {
     auto offsetStr = (offset == -1) ? "" : "at offset " + std::to_string(offset) + ": ";
-    err.msg = hintfmt("%s: %s%s, %s (in '%s')",
+    message = hintfmt("%s: %s%s, %s (in '%s')",
         normaltxt(hf.str()),
         offsetStr,
         sqlite3_errstr(extendedErrNo),
@@ -31,7 +31,7 @@ SQLiteError::SQLiteError(const char *path, const char *errMsg, int errNo, int ex
 
     if (err == SQLITE_BUSY || err == SQLITE_PROTOCOL) {
         auto exp = SQLiteBusy(path, errMsg, err, exterr, offset, std::move(hf));
-        exp.err.msg = hintfmt(
+        exp.message = hintfmt(
             err == SQLITE_PROTOCOL
                 ? "SQLite database '%s' is busy (SQLITE_PROTOCOL)"
                 : "SQLite database '%s' is busy",
@@ -247,9 +247,7 @@ void handleSQLiteBusy(const SQLiteBusy & e)
 
     if (now > lastWarned + 10) {
         lastWarned = now;
-        logWarning({
-            .msg = hintfmt(e.what())
-        });
+        logWarning(ErrorInfo {}, hintfmt(e.what()));
     }
 
     /* Sleep for a while since retrying the transaction right away

src/libutil/error.cc

@@ -21,13 +21,18 @@ const std::string & BaseError::calcWhat() const
     if (what_.has_value())
         return *what_;
     else {
-        std::ostringstream oss;
-        showErrorInfo(oss, err, loggerSettings.showTrace);
-        what_ = oss.str();
+        what_ = calcWhatUncached();
         return *what_;
     }
 }
 
+std::string Error::calcWhatUncached() const
+{
+    std::ostringstream oss;
+    showErrorInfo(oss, err, message, loggerSettings.showTrace);
+    return oss.str();
+}
+
 std::optional<std::string> ErrorInfo::programName = std::nullopt;
 
 std::ostream & operator <<(std::ostream & os, const hintformat & hf)
@@ -150,7 +155,11 @@ static std::string indent(std::string_view indentFirst, std::string_view indentR
     return res;
 }
 
-std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool showTrace)
+std::ostream & showErrorInfo(
+    std::ostream & out,
+    const ErrorInfo & einfo,
+    std::function<void(std::ostringstream & oss)> msg,
+    bool showTrace)
 {
     std::string prefix;
     switch (einfo.level) {
@@ -331,7 +340,9 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s
         oss << "\n" << prefix;
     }
 
-    oss << einfo.msg << "\n";
+    msg(oss);
+
+    oss << "\n";
 
     if (einfo.errPos) {
         oss << "\n" << ANSI_BLUE << "at " ANSI_WARNING << *einfo.errPos << ANSI_NORMAL << ":";
@@ -355,4 +366,19 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s
 
     return out;
 }
+
+std::ostream & showErrorInfo(
+    std::ostream & out,
+    const ErrorInfo & einfo,
+    hintformat msg,
+    bool showTrace)
+{
+    return showErrorInfo(
+        oss,
+        err,
+        [this](std::ostringstream & oss) {
+            oss << msg;
+        },
+        showTrace);
+}
 }

src/libutil/error.hh

@@ -93,7 +93,6 @@ struct Trace {
 
 struct ErrorInfo {
     Verbosity level;
-    hintformat msg;
     std::shared_ptr<AbstractPos> errPos;
     std::list<Trace> traces;
 
@@ -102,7 +101,17 @@ struct ErrorInfo {
     static std::optional<std::string> programName;
 };
 
-std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool showTrace);
+std::ostream & showErrorInfo(
+    std::ostream & out,
+    const ErrorInfo & einfo,
+    std::function<void(std::ostringstream & oss)> msg,
+    bool showTrace);
+/* Convenience for the common case. */
+std::ostream & showErrorInfo(
+    std::ostream & out,
+    const ErrorInfo & einfo,
+    hintformat msg,
+    bool showTrace);
 
 /* BaseError should generally not be caught, as it has Interrupted as
    a subclass. Catch Error instead. */
@@ -113,32 +122,15 @@ protected:
 
     mutable std::optional<std::string> what_;
     const std::string & calcWhat() const;
-
-public:
-    unsigned int status = 1; // exit status
+    virtual std::string calcWhatUncached() const = 0;
 
     BaseError(const BaseError &) = default;
 
-    template<typename... Args>
-    BaseError(unsigned int status, const Args & ... args)
-        : err { .level = lvlError, .msg = hintfmt(args...) }
+    BaseError(unsigned int status, ErrorInfo && e)
+        : err(std::move(e))
         , status(status)
     { }
 
-    template<typename... Args>
-    explicit BaseError(const std::string & fs, const Args & ... args)
-        : err { .level = lvlError, .msg = hintfmt(fs, args...) }
-    { }
-
-    template<typename... Args>
-    BaseError(const Suggestions & sug, const Args & ... args)
-        : err { .level = lvlError, .msg = hintfmt(args...), .suggestions = sug }
-    { }
-
-    BaseError(hintformat hint)
-        : err { .level = lvlError, .msg = hint }
-    { }
-
     BaseError(ErrorInfo && e)
         : err(std::move(e))
     { }
@@ -147,6 +139,9 @@ public:
         : err(e)
     { }
 
+public:
+    unsigned int status = 1; // exit status
+
 #ifdef EXCEPTION_NEEDS_THROW_SPEC
     ~BaseError() throw () { };
     const char * what() const throw () { return calcWhat().c_str(); }
@@ -175,14 +170,51 @@ public:
     const ErrorInfo & info() { return err; };
 };
 
+struct Error : BaseError
+{
+    hintformat message;
+
+    Error(ErrorInfo && e, hintformat && message)
+        : BaseError(e), message(message)
+    { }
+
+    Error(const ErrorInfo & e, const hintformat & message)
+        : BaseError(e), message(message)
+    { }
+
+    template<typename... Args>
+    Error(unsigned int status, const Args & ... args)
+        : BaseError(status, ErrorInfo { .level = lvlError })
+        , message(hintfmt(args...))
+    { }
+
+    template<typename... Args>
+    explicit Error(const std::string & fs, const Args & ... args)
+        : BaseError(ErrorInfo { .level = lvlError })
+        , message(hintfmt(fs, args...))
+    { }
+
+    template<typename... Args>
+    Error(const Suggestions & sug, const Args & ... args)
+        : BaseError(ErrorInfo { .level = lvlError, .suggestions = sug })
+        , message(hintfmt(args...))
+    { }
+
+    Error(hintformat hint)
+        : BaseError(ErrorInfo { .level = lvlError })
+        , message(hint)
+    { }
+
+    std::string calcWhatUncached() const override;
+};
+
 #define MakeError(newClass, superClass) \
     class newClass : public superClass                  \
     {                                                   \
     public:                                             \
         using superClass::superClass;                   \
     }
 
-MakeError(Error, BaseError);
 MakeError(UsageError, Error);
 MakeError(UnimplementedError, Error);
 
@@ -197,7 +229,7 @@ public:
     {
         errNo = errNo_;
         auto hf = hintfmt(args...);
-        err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo));
+        message = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo));
     }
 
     template<typename... Args>

src/libutil/logging.cc

@@ -74,10 +74,10 @@ class SimpleLogger : public Logger
         writeToStderr(prefix + filterANSIEscapes(fs.s, !tty) + "\n");
     }
 
-    void logEI(const ErrorInfo & ei) override
+    void logEI(const ErrorInfo & ei, hintformat msg) override
     {
         std::stringstream oss;
-        showErrorInfo(oss, ei, loggerSettings.showTrace.get());
+        showErrorInfo(oss, ei, msg, loggerSettings.showTrace.get());
 
         log(ei.level, oss.str());
     }
@@ -182,16 +182,16 @@ struct JSONLogger : Logger {
         write(json);
     }
 
-    void logEI(const ErrorInfo & ei) override
+    void logEI(const ErrorInfo & ei, hintformat message) override
     {
         std::ostringstream oss;
-        showErrorInfo(oss, ei, loggerSettings.showTrace.get());
+        showErrorInfo(oss, ei, message, loggerSettings.showTrace.get());
 
         nlohmann::json json;
         json["action"] = "msg";
         json["level"] = ei.level;
         json["msg"] = oss.str();
-        json["raw_msg"] = ei.msg.str();
+        json["raw_msg"] = message.str();
         to_json(json, ei.errPos);
 
         if (loggerSettings.showTrace.get() && !ei.traces.empty()) {

src/libutil/logging.hh

@@ -82,12 +82,12 @@ public:
         log(lvlInfo, fs);
     }
 
-    virtual void logEI(const ErrorInfo & ei) = 0;
+    virtual void logEI(const ErrorInfo & ei, hintformat msg) = 0;
 
-    void logEI(Verbosity lvl, ErrorInfo ei)
+    void logEI(Verbosity lvl, ErrorInfo ei, hintformat msg)
     {
         ei.level = lvl;
-        logEI(ei);
+        logEI(ei, msg);
     }
 
     virtual void warn(const std::string & msg);