Add SysError/WinError constructors that take a HintFmt-producing function

src/libexpr/parser.y

@@ -425,7 +425,7 @@ path_start
         /* Absolute paths are always interpreted relative to the
            root filesystem accessor, rather than the accessor of the
            current Nix expression. */
-        Path path(canonPath(literal));
+        auto path = canonPath(literal).string();
         /* add back in the trailing '/' to the first segment */
         if (literal.size() > 1 && literal.back() == '/')
           path += '/';
@@ -442,7 +442,7 @@ path_start
         });
 
         auto basePath = std::filesystem::path(state->basePath.path.abs());
-        Path path(absPath(literal, &basePath).string());
+        auto path = absPath(literal, &basePath).string();
         /* add back in the trailing '/' to the first segment */
         if (literal.size() > 1 && literal.back() == '/')
           path += '/';

src/libutil/include/nix/util/error.hh

@@ -19,6 +19,7 @@
 #include "nix/util/fmt.hh"
 #include "nix/util/config.hh"
 
+#include <concepts>
 #include <cstring>
 #include <list>
 #include <memory>
@@ -287,32 +288,60 @@ protected:
     /**
      * Just here to allow derived classes to use the right constructor
      * (the protected one).
+     *
+     * This one indicates the prebuilt `HintFmt` one with the explicit `errorDetails`
      */
-    struct Disambig
+    struct DisambigHintFmt
     {};
 
+    /**
+     * Just here to allow derived classes to use the right constructor
+     * (the protected one).
+     *
+     * This one indicates the varargs one to build the `HintFmt` with the explicit `errorDetails`
+     */
+    struct DisambigVarArgs
+    {};
+
+    /**
+     * Protected constructor that takes a pre-built HintFmt.
+     * Use this when the error message needs to be constructed before
+     * capturing errno/GetLastError().
+     */
+    SystemError(DisambigHintFmt, std::error_code errorCode, std::string_view errorDetails, const HintFmt & hf)
+        : CloneableError(HintFmt{"%s: %s", Uncolored(hf.str()), errorDetails})
+        , errorCode(errorCode)
+        , errorDetails(errorDetails)
+    {
+    }
+
     /**
      * Protected constructor for subclasses that provide their own error message.
      * The error message is appended to the formatted hint.
      */
     template<typename... Args>
-    SystemError(Disambig, std::error_code errorCode, std::string_view errorDetails, Args &&... args)
-        : CloneableError("")
-        , errorCode(errorCode)
-        , errorDetails(errorDetails)
+    SystemError(DisambigVarArgs, std::error_code errorCode, std::string_view errorDetails, Args &&... args)
+        : SystemError(DisambigHintFmt{}, errorCode, errorDetails, HintFmt{std::forward<Args>(args)...})
     {
-        auto hf = HintFmt(std::forward<Args>(args)...);
-        err.msg = HintFmt("%s: %s", Uncolored(hf.str()), errorDetails);
     }
 
 public:
+    /**
+     * Construct with an error code. The error code's message is automatically
+     * appended to the error message.
+     */
+    SystemError(std::error_code errorCode, const HintFmt & hf)
+        : SystemError(DisambigHintFmt{}, errorCode, errorCode.message(), hf)
+    {
+    }
+
     /**
      * Construct with an error code. The error code's message is automatically
      * appended to the error message.
      */
     template<typename... Args>
     SystemError(std::error_code errorCode, Args &&... args)
-        : SystemError(Disambig{}, errorCode, errorCode.message(), std::forward<Args>(args)...)
+        : SystemError(DisambigVarArgs{}, errorCode, errorCode.message(), std::forward<Args>(args)...)
     {
     }
 
@@ -355,14 +384,27 @@ public:
     template<typename... Args>
     SysError(int errNo, Args &&... args)
         : CloneableError(
-              Disambig{},
+              DisambigVarArgs{},
               std::make_error_code(static_cast<std::errc>(errNo)),
               strerror(errNo),
               std::forward<Args>(args)...)
         , errNo(errNo)
     {
     }
 
+    /**
+     * Construct using the explicitly-provided error number. `strerror`
+     * will be used to try to add additional information to the message.
+     *
+     * Unlike above, the `HintFmt` already exists rather than being made on
+     * the spot.
+     */
+    SysError(int errNo, const HintFmt & hf)
+        : CloneableError(DisambigHintFmt{}, std::make_error_code(static_cast<std::errc>(errNo)), strerror(errNo), hf)
+        , errNo(errNo)
+    {
+    }
+
     /**
      * Construct using the ambient `errno`.
      *
@@ -374,6 +416,34 @@ public:
         : SysError(errno, std::forward<Args>(args)...)
     {
     }
+
+    /**
+     * Construct using the ambient `errno` and a function that produces
+     * a `HintFmt`. errno is read first, then the function is called, so
+     * the function is safe to modify `errno`.
+     */
+    SysError(auto && mkHintFmt)
+        requires std::invocable<decltype(mkHintFmt)> && std::same_as<std::invoke_result_t<decltype(mkHintFmt)>, HintFmt>
+        : SysError(captureErrno(std::forward<decltype(mkHintFmt)>(mkHintFmt)))
+    {
+    }
+
+private:
+    /**
+     * Helper to ensure errno is captured before mkHintFmt is called.
+     * C++ argument evaluation order is unspecified, so we can't rely on
+     * `SysError(errno, mkHintFmt())` evaluating errno first.
+     */
+    static std::pair<int, HintFmt> captureErrno(auto && mkHintFmt)
+    {
+        int e = errno;
+        return {e, mkHintFmt()};
+    }
+
+    SysError(std::pair<int, HintFmt> && p)
+        : SysError(p.first, std::move(p.second))
+    {
+    }
 };
 
 /**
@@ -437,14 +507,29 @@ public:
     template<typename... Args>
     WinError(DWORD lastError, Args &&... args)
         : CloneableError(
-              Disambig{},
+              DisambigVarArgs{},
               std::error_code(lastError, std::system_category()),
               renderError(lastError),
               std::forward<Args>(args)...)
         , lastError(lastError)
     {
     }
 
+    /**
+     * Construct using the explicitly-provided error number.
+     * `FormatMessageA` will be used to try to add additional
+     * information to the message.
+     *
+     * Unlike above, the `HintFmt` already exists rather than being made on
+     * the spot.
+     */
+    WinError(DWORD lastError, const HintFmt & hf)
+        : CloneableError(
+              DisambigHintFmt{}, std::error_code(lastError, std::system_category()), renderError(lastError), hf)
+        , lastError(lastError)
+    {
+    }
+
     /**
      * Construct using `GetLastError()` and the ambient "last error".
      *
@@ -457,7 +542,33 @@ public:
     {
     }
 
+    /**
+     * Construct using `GetLastError()` and a function that produces a
+     * `HintFmt`. `GetLastError()` is called first, then the function is
+     * called, so the function is safe to modify the last error.
+     */
+    WinError(auto && mkHintFmt)
+        requires std::invocable<decltype(mkHintFmt)> && std::same_as<std::invoke_result_t<decltype(mkHintFmt)>, HintFmt>
+        : WinError(captureLastError(std::forward<decltype(mkHintFmt)>(mkHintFmt)))
+    {
+    }
+
 private:
+    /**
+     * Helper to ensure GetLastError() is captured before mkHintFmt is called.
+     * C++ argument evaluation order is unspecified, so we can't rely on
+     * `WinError(GetLastError(), mkHintFmt())` evaluating GetLastError() first.
+     */
+    static std::pair<DWORD, HintFmt> captureLastError(auto && mkHintFmt)
+    {
+        DWORD e = GetLastError();
+        return {e, mkHintFmt()};
+    }
+
+    WinError(std::pair<DWORD, HintFmt> && p)
+        : WinError(p.first, std::move(p.second))
+    {
+    }
 
     static std::string renderError(DWORD lastError);
 };

src/libutil/unix/file-system-at.cc

@@ -72,8 +72,7 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t
             if (errno == ENOSYS)
                 fchmodat2Unsupported.test_and_set();
             else {
-                auto savedErrno = errno;
-                throw SysError(savedErrno, "fchmodat2 %s", PathFmt(descriptorToPath(dirFd) / path.rel()));
+                throw SysError([&] { return HintFmt("fchmodat2 %s", PathFmt(descriptorToPath(dirFd) / path.rel())); });
             }
         } else
             return;
@@ -83,11 +82,11 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t
 #ifdef __linux__
     AutoCloseFD pathFd = ::openat(dirFd, path.rel_c_str(), O_PATH | O_NOFOLLOW | O_CLOEXEC);
     if (!pathFd) {
-        auto savedErrno = errno;
-        throw SysError(
-            savedErrno,
-            "opening %s to get an O_PATH file descriptor (fchmodat2 is unsupported)",
-            PathFmt(descriptorToPath(dirFd) / path.rel()));
+        throw SysError([&] {
+            return HintFmt(
+                "opening %s to get an O_PATH file descriptor (fchmodat2 is unsupported)",
+                PathFmt(descriptorToPath(dirFd) / path.rel()));
+        });
     }
 
     struct ::stat st;
@@ -107,9 +106,9 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t
             if (errno == ENOENT)
                 dontHaveProc.test_and_set();
             else {
-                auto savedErrno = errno;
-                throw SysError(
-                    savedErrno, "chmod %s (%s)", selfProcFdPath, PathFmt(descriptorToPath(dirFd) / path.rel()));
+                throw SysError([&] {
+                    return HintFmt("chmod %s (%s)", selfProcFdPath, PathFmt(descriptorToPath(dirFd) / path.rel()));
+                });
             }
         } else
             return;
@@ -131,8 +130,7 @@ void unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t
     );
 
     if (res == -1) {
-        auto savedErrno = errno;
-        throw SysError(savedErrno, "fchmodat %s", PathFmt(descriptorToPath(dirFd) / path.rel()));
+        throw SysError([&] { return HintFmt("fchmodat %s", PathFmt(descriptorToPath(dirFd) / path.rel())); });
     }
 }
 
@@ -217,8 +215,8 @@ OsString readLinkAt(Descriptor dirFd, const CanonPath & path)
         buf.resize(bufSize);
         ssize_t rlSize = ::readlinkat(dirFd, path.rel_c_str(), buf.data(), bufSize);
         if (rlSize == -1) {
-            auto savedErrno = errno;
-            throw SysError(savedErrno, "reading symbolic link %1%", PathFmt(descriptorToPath(dirFd) / path.rel()));
+            throw SysError(
+                [&] { return HintFmt("reading symbolic link %1%", PathFmt(descriptorToPath(dirFd) / path.rel())); });
         } else if (rlSize < bufSize)
             return {buf.data(), static_cast<std::size_t>(rlSize)};
     }

src/libutil/windows/current-process.cc

@@ -16,8 +16,7 @@ std::chrono::microseconds getCpuUserTime()
     FILETIME userTime;
 
     if (!GetProcessTimes(GetCurrentProcess(), &creationTime, &exitTime, &kernelTime, &userTime)) {
-        auto lastError = GetLastError();
-        throw windows::WinError(lastError, "failed to get CPU time");
+        throw windows::WinError("failed to get CPU time");
     }
 
     ULARGE_INTEGER uLargeInt;

src/libutil/windows/file-descriptor.cc

@@ -20,8 +20,7 @@ std::make_unsigned_t<off_t> getFileSize(Descriptor fd)
 {
     LARGE_INTEGER li;
     if (!GetFileSizeEx(fd, &li)) {
-        auto lastError = GetLastError();
-        throw WinError(lastError, "getting size of file %s", PathFmt(descriptorToPath(fd)));
+        throw WinError([&] { return HintFmt("getting size of file %s", PathFmt(descriptorToPath(fd))); });
     }
     return li.QuadPart;
 }
@@ -49,13 +48,10 @@ size_t readOffset(Descriptor fd, off_t offset, std::span<std::byte> buffer)
         ov.OffsetHigh = static_cast<DWORD>(offset >> 32);
     DWORD n;
     if (!ReadFile(fd, buffer.data(), static_cast<DWORD>(buffer.size()), &n, &ov)) {
-        auto lastError = GetLastError();
-        throw WinError(
-            lastError,
-            "reading %1% bytes at offset %2% from %3%",
-            buffer.size(),
-            offset,
-            PathFmt(descriptorToPath(fd)));
+        throw WinError([&] {
+            return HintFmt(
+                "reading %1% bytes at offset %2% from %3%", buffer.size(), offset, PathFmt(descriptorToPath(fd)));
+        });
     }
     return static_cast<size_t>(n);
 }
@@ -66,8 +62,8 @@ size_t write(Descriptor fd, std::span<const std::byte> buffer, bool allowInterru
         checkInterrupt(); // For consistency with unix
     DWORD n;
     if (!WriteFile(fd, buffer.data(), static_cast<DWORD>(buffer.size()), &n, NULL)) {
-        auto lastError = GetLastError();
-        throw WinError(lastError, "writing %1% bytes to %2%", buffer.size(), PathFmt(descriptorToPath(fd)));
+        throw WinError(
+            [&] { return HintFmt("writing %1% bytes to %2%", buffer.size(), PathFmt(descriptorToPath(fd))); });
     }
     return static_cast<size_t>(n);
 }
@@ -125,8 +121,7 @@ off_t lseek(HANDLE h, off_t offset, int whence)
 void syncDescriptor(Descriptor fd)
 {
     if (!::FlushFileBuffers(fd)) {
-        auto lastError = GetLastError();
-        throw WinError(lastError, "flushing file %s", PathFmt(descriptorToPath(fd)));
+        throw WinError([&] { return HintFmt("flushing file %s", PathFmt(descriptorToPath(fd))); });
     }
 }
 

src/libutil/windows/muxable-pipe.cc

@@ -7,15 +7,17 @@
 
 namespace nix {
 
+using namespace nix::windows;
+
 void MuxablePipePollState::poll(HANDLE ioport, std::optional<unsigned int> timeout)
 {
     /* We are on at least Windows Vista / Server 2008 and can get many
        (countof(oentries)) statuses in one API call. */
     if (!GetQueuedCompletionStatusEx(
             ioport, oentries, sizeof(oentries) / sizeof(*oentries), &removed, timeout ? *timeout : INFINITE, false)) {
-        windows::WinError winError("GetQueuedCompletionStatusEx");
-        if (winError.lastError != WAIT_TIMEOUT)
-            throw winError;
+        auto lastError = GetLastError();
+        if (lastError != WAIT_TIMEOUT)
+            throw WinError(lastError, "GetQueuedCompletionStatusEx");
         assert(removed == 0);
     } else {
         assert(0 < removed && removed <= sizeof(oentries) / sizeof(*oentries));
@@ -52,12 +54,12 @@ void MuxablePipePollState::iterate(
                         // here is possible (but not obligatory) to call
                         // `handleRead` and repeat ReadFile immediately
                     } else {
-                        windows::WinError winError("ReadFile(%s, ..)", (*p)->readSide.get());
-                        if (winError.lastError == ERROR_BROKEN_PIPE) {
+                        auto lastError = GetLastError();
+                        if (lastError == ERROR_BROKEN_PIPE) {
                             handleEOF((*p)->readSide.get());
                             nextp = channels.erase(p); // no need to maintain `channels` ?
-                        } else if (winError.lastError != ERROR_IO_PENDING)
-                            throw winError;
+                        } else if (lastError != ERROR_IO_PENDING)
+                            throw WinError(lastError, "ReadFile(%s, ..)", (*p)->readSide.get());
                     }
                 }
                 break;